-
Notifications
You must be signed in to change notification settings - Fork 18.4k
net/url: reduce allocs in Encode #75874
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
Conversation
This PR (HEAD: 38a1551) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/711280. Important tips:
|
Message from Gopher Robot: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from jub0bs: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Jes Cok: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from jub0bs: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Sean Liao: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
38a1551
to
569f3ff
Compare
This PR (HEAD: 569f3ff) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/711280. Important tips:
|
Message from jub0bs: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from jub0bs: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Sean Liao: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from jub0bs: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Sean Liao: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from t hepudds: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Damien Neil: Patch Set 2: Code-Review+2 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from jub0bs: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
569f3ff
to
3dc5182
Compare
This PR (HEAD: 3dc5182) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/711280. Important tips:
|
Message from jub0bs: Patch Set 3: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from t hepudds: Patch Set 3: Commit-Queue+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Go LUCI: Patch Set 3: Dry run: CV is trying the patch. Bot data: {"action":"start","triggered_at":"2025-10-17T14:42:42Z","revision":"b024213d10bda1ac4e9ee045ef67fc68b1afde89"} Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from t hepudds: Patch Set 3: Code-Review+2 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Sean Liao: Patch Set 3: Auto-Submit+1 Code-Review+2 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from t hepudds: Patch Set 3: -Commit-Queue Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Go LUCI: Patch Set 3: This CL has passed the run Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Go LUCI: Patch Set 3: LUCI-TryBot-Result+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Emmanuel Odeke: Patch Set 3: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
3dc5182
to
f6442a6
Compare
This PR (HEAD: f6442a6) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/711280. Important tips:
|
Message from t hepudds: Patch Set 4: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
f6442a6
to
3475f2e
Compare
This PR (HEAD: 3475f2e) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/711280. Important tips:
|
Message from jub0bs: Patch Set 4: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from t hepudds: Patch Set 5: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Jes Cok: Patch Set 5: Commit-Queue+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Go LUCI: Patch Set 5: Dry run: CV is trying the patch. Bot data: {"action":"start","triggered_at":"2025-10-18T19:07:47Z","revision":"a7060b5e7afd4594212d32bccfa8c193a143638d"} Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Jes Cok: Patch Set 5: -Commit-Queue Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Go LUCI: Patch Set 5: This CL has failed the run. Reason: Tryjob golang/try/gotip-linux-amd64-race has failed with summary (view all results):
To reproduce, try Additional links for debugging: Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Go LUCI: Patch Set 5: LUCI-TryBot-Result-1 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
3475f2e
to
f25be71
Compare
This PR (HEAD: f25be71) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/711280. Important tips:
|
Message from t hepudds: Patch Set 6: Commit-Queue+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Go LUCI: Patch Set 6: Dry run: CV is trying the patch. Bot data: {"action":"start","triggered_at":"2025-10-18T19:34:18Z","revision":"2208e55f14ff63636e48210901e2684465c02566"} Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from t hepudds: Patch Set 6: -Commit-Queue Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Go LUCI: Patch Set 6: This CL has passed the run Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Go LUCI: Patch Set 6: LUCI-TryBot-Result+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from t hepudds: Patch Set 6: Code-Review+2 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from jub0bs: Patch Set 6: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Florian Lehner: Patch Set 6: Code-Review+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Emmanuel Odeke: Patch Set 6: Code-Review+2 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Emmanuel Odeke: Patch Set 6: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Keith Randall: Patch Set 6: Code-Review+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
Message from Cherry Mui: Patch Set 6: Code-Review+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/711280. |
This change adds benchmarks for Encode and reverts what CL 617356 did in this package. At the moment, using maps.Keys in conjunction with slices.Sorted indeed causes a bunch of closures to escape to heap. Moreover, all other things being equal, pre-sizing the slice in which we collect the keys is beneficial to performance when they are "many" (>8) keys because it results in fewer allocations than if we don't pre-size the slice. Here are some benchmark results: goos: darwin goarch: amd64 pkg: net/url cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz │ old │ new │ │ sec/op │ sec/op vs base │ EncodeQuery/#00-8 2.051n ± 1% 2.343n ± 1% +14.24% (p=0.000 n=20) EncodeQuery/#1-8 2.337n ± 1% 2.458n ± 4% +5.16% (p=0.000 n=20) EncodeQuery/oe=utf8&q=puppies-8 489.6n ± 0% 284.5n ± 0% -41.88% (p=0.000 n=20) EncodeQuery/q=dogs&q=%26&q=7-8 397.2n ± 1% 231.7n ± 1% -41.66% (p=0.000 n=20) EncodeQuery/a=a1&a=a2&a=a3&b=b1&b=b2&b=b3&c=c1&c=c2&c=c3-8 743.1n ± 0% 519.0n ± 0% -30.16% (p=0.000 n=20) EncodeQuery/a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i-8 1324.0n ± 0% 931.0n ± 0% -29.68% (p=0.000 n=20) geomean 98.57n 75.38n -23.53% │ old │ new │ │ B/op │ B/op vs base │ EncodeQuery/#00-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹ EncodeQuery/#1-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹ EncodeQuery/oe=utf8&q=puppies-8 168.00 ± 0% 56.00 ± 0% -66.67% (p=0.000 n=20) EncodeQuery/q=dogs&q=%26&q=7-8 112.00 ± 0% 32.00 ± 0% -71.43% (p=0.000 n=20) EncodeQuery/a=a1&a=a2&a=a3&b=b1&b=b2&b=b3&c=c1&c=c2&c=c3-8 296.0 ± 0% 168.0 ± 0% -43.24% (p=0.000 n=20) EncodeQuery/a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i-8 680.0 ± 0% 264.0 ± 0% -61.18% (p=0.000 n=20) geomean ² -47.48% ² ¹ all samples are equal ² summaries must be >0 to compute geomean │ old │ new │ │ allocs/op │ allocs/op vs base │ EncodeQuery/#00-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹ EncodeQuery/#1-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹ EncodeQuery/oe=utf8&q=puppies-8 8.000 ± 0% 3.000 ± 0% -62.50% (p=0.000 n=20) EncodeQuery/q=dogs&q=%26&q=7-8 7.000 ± 0% 3.000 ± 0% -57.14% (p=0.000 n=20) EncodeQuery/a=a1&a=a2&a=a3&b=b1&b=b2&b=b3&c=c1&c=c2&c=c3-8 10.000 ± 0% 5.000 ± 0% -50.00% (p=0.000 n=20) EncodeQuery/a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i-8 12.000 ± 0% 5.000 ± 0% -58.33% (p=0.000 n=20) geomean ² -43.23% ² ¹ all samples are equal ² summaries must be >0 to compute geomean Change-Id: Ia0d7579f90434f0546d93b680ab18b47a1ffbdac GitHub-Last-Rev: f25be71 GitHub-Pull-Request: #75874 Reviewed-on: https://go-review.googlesource.com/c/go/+/711280 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Florian Lehner <[email protected]> Reviewed-by: Emmanuel Odeke <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Reviewed-by: Sean Liao <[email protected]> Reviewed-by: t hepudds <[email protected]>
This PR is being closed because golang.org/cl/711280 has been merged. |
This change adds benchmarks for Encode and reverts what CL 617356 did in
this package. At the moment, using maps.Keys in conjunction with
slices.Sorted indeed causes a bunch of closures to escape to heap.
Moreover, all other things being equal, pre-sizing the slice in which
we collect the keys is beneficial to performance when they are "many" (>8)
keys because it results in fewer allocations than if we don't pre-size the
slice.
Here are some benchmark results:
goos: darwin
goarch: amd64
pkg: net/url
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
│ old │ new │
│ sec/op │ sec/op vs base │
EncodeQuery/#00-8 2.051n ± 1% 2.343n ± 1% +14.24% (p=0.000 n=20)
EncodeQuery/#1-8 2.337n ± 1% 2.458n ± 4% +5.16% (p=0.000 n=20)
EncodeQuery/oe=utf8&q=puppies-8 489.6n ± 0% 284.5n ± 0% -41.88% (p=0.000 n=20)
EncodeQuery/q=dogs&q=%26&q=7-8 397.2n ± 1% 231.7n ± 1% -41.66% (p=0.000 n=20)
EncodeQuery/a=a1&a=a2&a=a3&b=b1&b=b2&b=b3&c=c1&c=c2&c=c3-8 743.1n ± 0% 519.0n ± 0% -30.16% (p=0.000 n=20)
EncodeQuery/a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i-8 1324.0n ± 0% 931.0n ± 0% -29.68% (p=0.000 n=20)
geomean 98.57n 75.38n -23.53%
EncodeQuery/#00-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹
EncodeQuery/#1-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹
EncodeQuery/oe=utf8&q=puppies-8 168.00 ± 0% 56.00 ± 0% -66.67% (p=0.000 n=20)
EncodeQuery/q=dogs&q=%26&q=7-8 112.00 ± 0% 32.00 ± 0% -71.43% (p=0.000 n=20)
EncodeQuery/a=a1&a=a2&a=a3&b=b1&b=b2&b=b3&c=c1&c=c2&c=c3-8 296.0 ± 0% 168.0 ± 0% -43.24% (p=0.000 n=20)
EncodeQuery/a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i-8 680.0 ± 0% 264.0 ± 0% -61.18% (p=0.000 n=20)
geomean ² -47.48% ²
¹ all samples are equal
² summaries must be >0 to compute geomean
EncodeQuery/#00-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹
EncodeQuery/#1-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=20) ¹
EncodeQuery/oe=utf8&q=puppies-8 8.000 ± 0% 3.000 ± 0% -62.50% (p=0.000 n=20)
EncodeQuery/q=dogs&q=%26&q=7-8 7.000 ± 0% 3.000 ± 0% -57.14% (p=0.000 n=20)
EncodeQuery/a=a1&a=a2&a=a3&b=b1&b=b2&b=b3&c=c1&c=c2&c=c3-8 10.000 ± 0% 5.000 ± 0% -50.00% (p=0.000 n=20)
EncodeQuery/a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i-8 12.000 ± 0% 5.000 ± 0% -58.33% (p=0.000 n=20)
geomean ² -43.23% ²
¹ all samples are equal
² summaries must be >0 to compute geomean