-
Notifications
You must be signed in to change notification settings - Fork 0
NOMERGE: apply cgo fix for mobile #1
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
Open
chrisnojima
wants to merge
2
commits into
release-branch.go1.23
Choose a base branch
from
nojima/HOTPOT-go1234-unalign-fix
base: release-branch.go1.23
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
NOMERGE: apply cgo fix for mobile #1
chrisnojima
wants to merge
2
commits into
release-branch.go1.23
from
nojima/HOTPOT-go1234-unalign-fix
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zoom-ua
pushed a commit
that referenced
this pull request
Dec 15, 2025
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: golang#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]>
zoom-ua
pushed a commit
that referenced
this pull request
Dec 15, 2025
This change adds a generated 8-bit bitmask for use in functions shouldEscape and ishex.
Function shouldEscape is now inlineable. Function escape is now much faster;
function unescape is a bit faster. Here are some benchmark results (no change
to allocations):
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 │
QueryEscape/#00-8 58.38n ± 1% 35.98n ± 1% -38.38% (p=0.000 n=20)
QueryEscape/#1-8 303.50n ± 0% 94.77n ± 0% -68.77% (p=0.000 n=20)
QueryEscape/#2-8 202.90n ± 0% 78.66n ± 1% -61.23% (p=0.000 n=20)
QueryEscape/#3-8 444.5n ± 0% 145.9n ± 0% -67.17% (p=0.000 n=20)
QueryEscape/golang#4-8 2678.0n ± 0% 913.7n ± 0% -65.88% (p=0.000 n=20)
PathEscape/#00-8 81.34n ± 0% 44.64n ± 1% -45.12% (p=0.000 n=20)
PathEscape/#1-8 307.65n ± 0% 96.71n ± 1% -68.56% (p=0.000 n=20)
PathEscape/#2-8 200.80n ± 1% 78.25n ± 0% -61.03% (p=0.000 n=20)
PathEscape/#3-8 450.1n ± 1% 145.5n ± 0% -67.67% (p=0.000 n=20)
PathEscape/golang#4-8 2663.5n ± 0% 876.5n ± 0% -67.09% (p=0.000 n=20)
QueryUnescape/#00-8 53.32n ± 1% 51.67n ± 1% -3.09% (p=0.000 n=20)
QueryUnescape/#1-8 161.0n ± 1% 136.2n ± 1% -15.40% (p=0.000 n=20)
QueryUnescape/#2-8 126.1n ± 1% 118.3n ± 1% -6.23% (p=0.000 n=20)
QueryUnescape/#3-8 294.6n ± 0% 273.1n ± 0% -7.30% (p=0.000 n=20)
QueryUnescape/golang#4-8 1.511µ ± 0% 1.411µ ± 0% -6.62% (p=0.000 n=20)
PathUnescape/#00-8 63.84n ± 1% 53.59n ± 1% -16.05% (p=0.000 n=20)
PathUnescape/#1-8 163.6n ± 3% 137.9n ± 1% -15.71% (p=0.000 n=20)
PathUnescape/#2-8 126.4n ± 1% 119.1n ± 1% -5.78% (p=0.000 n=20)
PathUnescape/#3-8 294.2n ± 0% 273.3n ± 0% -7.12% (p=0.000 n=20)
PathUnescape/golang#4-8 1.554µ ± 0% 1.417µ ± 0% -8.78% (p=0.000 n=20)
geomean 277.8n 162.7n -41.44%
This change draws heavy inspiration from CL 174998, which showed promise but stalled years ago.
Updates golang#17860
Change-Id: Idcbb1696608998b9e2fc91e1f2a488d8f1f6028c
GitHub-Last-Rev: ff360c2
GitHub-Pull-Request: golang#75914
Reviewed-on: https://go-review.googlesource.com/c/go/+/712200
Reviewed-by: David Chase <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Jorropo <[email protected]>
Reviewed-by: Takuto Nagami <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
zoom-ua
pushed a commit
that referenced
this pull request
Dec 15, 2025
This change is a follow-up to CL 712200. It further simplifies and speeds up
functions escape and unescape.
Here are some benchmark results (no change to allocations):
goos: darwin
goarch: amd64
pkg: net/url
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
│ go/src/old │ go/src/new │
│ sec/op │ sec/op vs base │
QueryEscape/#00-8 34.58n ± 1% 31.97n ± 1% -7.55% (p=0.000 n=20)
QueryEscape/#1-8 92.92n ± 0% 94.63n ± 0% +1.84% (p=0.000 n=20)
QueryEscape/#2-8 75.44n ± 0% 73.32n ± 0% -2.80% (p=0.000 n=20)
QueryEscape/#3-8 143.4n ± 0% 136.6n ± 0% -4.71% (p=0.000 n=20)
QueryEscape/golang#4-8 918.8n ± 1% 838.3n ± 0% -8.76% (p=0.000 n=20)
PathEscape/#00-8 43.93n ± 0% 42.86n ± 0% -2.44% (p=0.000 n=20)
PathEscape/#1-8 94.99n ± 0% 95.86n ± 0% +0.91% (p=0.000 n=20)
PathEscape/#2-8 75.40n ± 1% 71.50n ± 1% -5.18% (p=0.000 n=20)
PathEscape/#3-8 143.4n ± 0% 136.2n ± 0% -4.99% (p=0.000 n=20)
PathEscape/golang#4-8 871.8n ± 0% 822.7n ± 0% -5.63% (p=0.000 n=20)
QueryUnescape/#00-8 52.64n ± 1% 51.19n ± 0% -2.75% (p=0.000 n=20)
QueryUnescape/#1-8 137.4n ± 1% 137.9n ± 1% ~ (p=0.297 n=20)
QueryUnescape/#2-8 114.0n ± 0% 122.3n ± 1% +7.24% (p=0.000 n=20)
QueryUnescape/#3-8 271.8n ± 0% 260.7n ± 1% -4.08% (p=0.000 n=20)
QueryUnescape/golang#4-8 1.390µ ± 1% 1.355µ ± 0% -2.52% (p=0.000 n=20)
PathUnescape/#00-8 52.45n ± 1% 53.03n ± 1% +1.10% (p=0.008 n=20)
PathUnescape/#1-8 138.5n ± 1% 141.3n ± 0% +2.06% (p=0.000 n=20)
PathUnescape/#2-8 114.0n ± 0% 121.5n ± 0% +6.62% (p=0.000 n=20)
PathUnescape/#3-8 273.1n ± 1% 260.1n ± 0% -4.76% (p=0.000 n=20)
PathUnescape/golang#4-8 1.431µ ± 1% 1.359µ ± 0% -5.07% (p=0.000 n=20)
geomean 160.4n 156.9n -2.14%
Updates golang#17860
Change-Id: If64ac3e9c62c41f672db06cfd7eab7357e934e6d
GitHub-Last-Rev: 1da047a
GitHub-Pull-Request: golang#76048
Reviewed-on: https://go-review.googlesource.com/c/go/+/714900
Reviewed-by: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: David Chase <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Applies fixes from golang#46893. On mobile with v660 there is a rare but reproducible crash on initial provision from clean install