Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/exp/slices: Grow should not mutate bytes between length and capacity #53853

Closed
dsnet opened this issue Jul 13, 2022 · 1 comment
Closed

x/exp/slices: Grow should not mutate bytes between length and capacity #53853

dsnet opened this issue Jul 13, 2022 · 1 comment
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@dsnet
Copy link
Member

dsnet commented Jul 13, 2022

The documentation of Grow says:

Grow may modify elements of the slice between the length and the capacity.

While this is just a "may", in practice it does, which leads to poor performance.

Suppose I had some logic that did something like:

for _, v := range manyValues {
    n := v.EncodedLen()
    dst = slices.Grow(dst, n)
    v.EncodeInto(dst)
    dst = dst[:len(dst)+n]
}

In the event that dst is large enough, I would expect that every call to slices.Grow to effectively be a noop.
However, that is not what happens. Instead, it always zeroes out the space between dst[len(dst):len(dst)+n].

The performance cost of this is evident with this simple benchmark:

func Benchmark(b *testing.B) {
	buf := make([]byte, 0, 1<<20)
	for i := 0; i < b.N; i++ {
		buf = slices.Grow(buf, 1<<20)
	}
}

which prints:

Benchmark-24    	    5581	    230992 ns/op

I would expect it to be something like:

Benchmark-24    	1000000000	         0.2222 ns/op

\cc @ianlancetaylor @eliben

@gopherbot gopherbot added this to the Unreleased milestone Jul 13, 2022
@gopherbot
Copy link

Change https://go.dev/cl/417494 mentions this issue: slices: avoid mutating elements between length and capacity in Grow

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 14, 2022
@golang golang locked and limited conversation to collaborators Oct 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Development

No branches or pull requests

3 participants