If the array is quite large, it's more efficient to use copy() instead of append()

so for example, if len(optionalSet) > 100 and if optionalSet is a slice

var missing []string
for _, feature := range optionalSet {
	missing = append(missing, feature)
}

should become

missing := make([]string, len(optionalSet)
copy(missing, optionalSet)

instead of

missing := make([]string, 0, len(optionalSet))
for _, feature := range optionalSet {
	missing = append(missing, feature)
}

Benchmark:

For example, when len(existing) = 500, with this new test:

func BenchmarkPreallocateCopy(b *testing.B) {
	existing := make([]int64, 200, 200)
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		// Preallocate our initial slice
		init := make([]int64, len(existing))
		copy(init, existing)
	}
}

the results clearly shows that copy is faster:

BenchmarkNoPreallocate-2     	  300000	      4039 ns/op
BenchmarkPreallocate-2       	 1000000	      1664 ns/op
BenchmarkPreallocateCopy-2   	 1000000	      1303 ns/op

Maybe it should be mention somewhere in the README ?

The motivation here is to remove the need for forking prealloc for use in linter aggregation tools such as golangci-lint. I attempted to make as few changes as possible to the code itself to keep the existing structure, while adding the interface necessary for golangci-lint to use it.

Those changes are based largely on the commit here: https://github.com/golangci/prealloc/commit/215b22d4de21190b80ce05e7d8466677c1aa3223

If there are other approaches that might suit this repo better, I'd be happy to make the change. I just figured a proposal in the form of a quick refactor would be the easiest place to start the discussion.

Related: https://github.com/golangci/golangci-lint/issues/991#issuecomment-617468077

This is a fix for the issue

I have done manual testing to test the code changes and it works as expected.

type MySlice3 []string
func main() {
	type MySlice1 []string
	type MySlice2 = []string

	var var1 MySlice1
	var var2 []string
	var3 := make([]string, 10, 10)
	var var4 MySlice2
    var var5 MySlice3
	for feature := range optionalSet {
		var1 = append(var1, feature)
		var2 = append(var2, feature)
		var3 = append(var3, feature)
		var4 = append(var4, feature)
		var5 = append(var5, feature)
	}
}

../../bin/test.go:9 Consider preallocating var1 ../../bin/test.go:10 Consider preallocating var2 ../../bin/test.go:12 Consider preallocating var4 ../../bin/test.go:13 Consider preallocating var5

var jobs []job
for job := range jobc {
    jobs = append(jobs, job)
}

In the code above, jobc is an unbuffered channel. I don't know the number of jobs that will be pulled from it. So I can't preallocate it.

Given this sample program:

package main

import "fmt"

func main() {
	var buf []byte

	strings := []string{"foo", "bar", "baz"}
	for i, s := range strings {
		buf = append(buf, s...)
		buf = append(buf, '\n')
	}

	fmt.Println(buf)
}

prealloc will output:

main.go:6 Consider preallocating buf
main.go:6 Consider preallocating buf

It appears to be outputting a line for each append in the loop, i.e. if I add one more buf = append(buf, x) there will be one more "main.go:6 Consider preallocating buf" line output.

There should be at most one warning regardless of multiple append calls.

Hi, i have case when i need to define array that supposed to received result from database. Previously i have done this:

var models []Model
result := db.Where("namespace = ?", namespace).Find(&clients)

where the result length is arbitrary. When i run prealloc, it suggests me to preallocating the array so i revised the code like this

models := make([]Model, 0) // i put 0 since i don't know the length yet
result := db.Where("namespace = ?", namespace).Find(&clients)

So, am i doing this correctly ? is preallocating the array with zero length because the length can be arbitrary still gaining the performance benefit rather than defining it with var?

Thanks!

a := []int{}
b := []int{1}
c := []int{1,2}
s := []int{}
for _, x := range [][]int{a, b, c} {
    s = append(s, x...)
} 
// s cannot be preallocated

But prealloc warns about this code?

Thanks.

This is not detected at the moment:

type MySlice []string

var missing MySlice
for feature := range optionalSet {
	missing = append(missing, feature)
}

And this as well:

type MySlice = []string

var missing MySlice
for feature := range optionalSet {
	missing = append(missing, feature)
}

Long time no see. 😁

See https://github.com/golangci/golangci-lint/issues/991. Trying to pre-allocate when ... is in play in a for loop gets messy. Also revised the logic to better handle multiple assignments, and added a test.

Setting the exit status of prealloc to something other than 0 would make it possible to use in CI builds as a means of verification.

If compatibility is a concern, a flag could be used to enable this behavior. In that case, the flag would ideally be -set_exit_status to be consistent with golint.

Preallocation of maps can be useful, as shown by PR #23, would be nice to have prealloc suggest that too.

Just wondering how smart it can be made as things are not as straightforward as with slices because there's key uniqueness at play. Cases where the optimal preallocated capacity can be easily determined beforehand are more rare with maps, and in that sense suggesting it could have a worse signal to noise ratio. Maybe it should be made optional if it can't be made "smart enough" :thinking:

Results from system in front of me:

BenchmarkMap/Size10NoPreallocate-4  	 1954923	       603.7 ns/op	     484 B/op	       3 allocs/op
BenchmarkMap/Size10Preallocate-4    	 2951250	       393.8 ns/op	     340 B/op	       2 allocs/op
BenchmarkMap/Size200NoPreallocate-4 	   71530	     15337 ns/op	   11270 B/op	      25 allocs/op
BenchmarkMap/Size200Preallocate-4   	  126673	      8216 ns/op	    5719 B/op	       8 allocs/op

I currently work in a team and we had a code issue a few times that made it beyond code review. It is pre-allocating a slice with size instead of cap, then use append on it. Example

ar := make([]int, 10)
for i:=0; i<10; i++ {
  ar = append(ar, i)
  }

The correct pre-alloc would be ar := make([]int, 0, 10). Because of the missing 0,, the faulty code produces an array of size 20.

Would it be possible to add a check for this into your linter? I could not find any linter that has this check.

When ranging over a list that contains conditional break/continue directives and a slice = append(slice, element) at the end, prealloc suggests preallocation.

No lint triggered

var a []int
for i := range []struct{}{{}, {}, {}} {
        if i < 1 {
                a = append(a, i)
        } 
}

Lint triggered

var a []int
for i := range []struct{}{{}, {}, {}} {
        if i < 1 {
              continue
        } 
        a = append(a, i)
}

In the following function, prealloc is confused by buf being redeclared:

func copySort(w io.Writer, r io.Reader, sep rune, keys []int) error {
	// Copy the header line.
	var buf [1]byte // THATS LINE 275 WHERE THE ERROR MESSAGES POINT TO
	for {
		n, err := r.Read(buf[:])
		if n != 0 {
			if _, err := w.Write(buf[:]); err != nil {
				return err
			}
		}
		if err != nil {
			if err == io.EOF {
				return nil
			}
			return err
		}
		if buf[0] == '\n' {
			break
		}
	}
	...
	for i, k := range keys {
		buf := make([]byte, 0, 16)
		buf = append(buf, "-k"...)
		n := int64(1 + k)
		buf = strconv.AppendInt(buf, n, 10)
		buf = append(buf, ',')
		buf = strconv.AppendInt(buf, n, 10)
		args[2+i] = alloc.DirectString(buf)
	}
	...
}

preallocreports these 2 error messages:

...:275 Consider preallocating buf
...:275 Consider preallocating buf

If I rename the first buf declaration with any other variable name (e.g., cell) the error messages disappear.

It seems a variable redeclaration is not always managed correctly by prealloc.