Open ainar-g opened 2 years ago
I believe we use the append
version so that we let the runtime round up to the next size class. That will allow the caller to append some items to the result without immediately triggering a reallocation+copy.
The append
version is also knows that it doesn't need to zero the allocation before overwriting it with the copy.
With both those said, it would be nice if the append
version wasn't slower. It looks like it isn't for large arrays, but there's some overhead for small arrays.
If there is anything to be fixed here, it is in the compiler, so retitled.
"The append version is also knows that it doesn't need to zero the allocation before overwriting it with the copy." The same seems true for make+copy too: https://go-review.googlesource.com/c/go/+/146719
I think what we should do is detect cases of append(X, s...)
where X is known to have len and cap 0 in the compiler and rewrite this into makeextended+copy where makeextended is a make variant that calculates the new slice size like append does but can assume more constraints then the general append case. This should then be minimaly slower then make+copy. Whether the optimization is alot faster to warrant the added complexity benchmarks need to show.
my 2 cents is to do write the library code so it's readable and not "cool one liner", ie option 2 (make+copy) is imo a better implementation to read - and happen to be faster, so ... great!
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes with
go version devel go1.19-d3ffff2790 Tue Jun 28 13:01:41 2022 +0000 linux/amd64
.What operating system and processor architecture are you using (
go env
)?go env
OutputDiscoveries / Proposal
slices.Clone
is currently defined as:append
here seems to be optimized to output what is essentiallymake
+copy
. But if we do that explicitly:and benchmark it (see https://go.dev/play/p/WHvD0zJ33S1) then the new function generally performs better:
Looking at the assembly, it seems that the current version uses
runtime.growslice
while the new one,runtime.mallocgc
. There are other changes in the assembly as well.