golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.88k stars 17.65k forks source link

cmd/compile: avoid unnecessary allocations for consecutive appends #44628

Open rogpeppe opened 3 years ago

rogpeppe commented 3 years ago

go1.16

The following function will often incur two allocations, one for each append call. However the compiler could potentially know the length of the extra data in advance (len(f) + 4) and hence allocate at most once.

func appendFoo(x []byte, f string) []byte {
    x = append(x, "aggf"...)
    x = append(x, f...)
    return x
}

Working around this is awkward because to do that properly requires duplication of the append capacity-growing algorithm so that the slice will grow by a multiplicative rather than a constant factor. When generics arrive, I'd love to see a function like this in the standard library (that would be useful for #14325 amongst others), but the above optimization would still be useful to avoid using that.

// Grow returns ts with its capacity grown enough to accommodate n more items.
func Grow[T any](ts []T, n int) []T

Related issues:

pierrec commented 3 years ago

I may be mistaken, but doesnt the following allow growing the capacity accordingly and is optimized to not allocate the make?

s = append(s, make([]T, n)...)

tmthrgd commented 3 years ago

I don’t think the compiler will ever be able to optimise this because each append has effects that are observable outside the function. Compare the following:

https://play.golang.org/p/WvBXW8u1SWk

https://play.golang.org/p/nfqTnI8SrnV

https://play.golang.org/p/N2dtswu2dLC

randall77 commented 3 years ago

It is still possible, but tricky. What @rogpeppe is worried about is when both appends cause a grow. In that case, the intermediate growth is not observable and can be skipped. But yes, you do have to write all the appends you can to the original slice.

Nevertheless, @pierrec 's workaround should handle this. I'm not sure if specialized code in the compiler would be worth it.

quasilyte commented 2 years ago

A very similar case is #25828 We can probably move the missing info from one issue into another (any of the two would suffice) and close the other one. See https://github.com/golang/go/issues/25828#issuecomment-407497073 for some insights.