-
Notifications
You must be signed in to change notification settings - Fork 0
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
Lots of integer variables in func stable and func merge #1
Comments
Can I just get feedback on the idea to separate code from func stable into a different function, before other things? |
Of course your code mus be seperated into much much smaller functions and these function do need tests. |
nsajko
pushed a commit
that referenced
this issue
Mar 25, 2019
Most chars in URLs are lowercase, so check that first. Performance change: String-4 7.62µs ± 1% 7.27µs ± 3% -4.61% (p=0.008 n=5+5) QueryEscape/#00-4 92.6ns ± 3% 90.3ns ± 1% -2.48% (p=0.016 n=5+5) QueryEscape/#1-4 515ns ± 4% 510ns ± 2% ~ (p=0.683 n=5+5) QueryEscape/#2-4 375ns ± 1% 343ns ± 1% -8.52% (p=0.008 n=5+5) QueryEscape/#3-4 758ns ± 1% 699ns ± 1% -7.83% (p=0.008 n=5+5) QueryEscape/#04-4 6.06µs ± 1% 5.74µs ± 1% -5.38% (p=0.008 n=5+5) PathEscape/#00-4 140ns ± 1% 135ns ± 2% -3.85% (p=0.008 n=5+5) PathEscape/#1-4 511ns ± 3% 507ns ± 3% ~ (p=0.587 n=5+5) PathEscape/#2-4 372ns ± 1% 342ns ± 2% -8.22% (p=0.008 n=5+5) PathEscape/#3-4 747ns ± 1% 685ns ± 1% -8.30% (p=0.008 n=5+5) PathEscape/#04-4 5.94µs ± 1% 5.64µs ± 3% -4.98% (p=0.008 n=5+5) QueryUnescape/#00-4 111ns ± 4% 110ns ± 2% ~ (p=0.952 n=5+5) QueryUnescape/#1-4 390ns ± 0% 391ns ± 2% ~ (p=0.714 n=5+5) QueryUnescape/#2-4 297ns ± 5% 295ns ± 3% ~ (p=0.524 n=5+5) QueryUnescape/#3-4 543ns ± 3% 556ns ± 2% +2.39% (p=0.032 n=5+5) QueryUnescape/#04-4 3.23µs ± 3% 3.22µs ± 2% ~ (p=1.000 n=5+5) PathUnescape/#00-4 111ns ± 1% 110ns ± 3% ~ (p=0.881 n=5+5) PathUnescape/#1-4 389ns ± 2% 386ns ± 2% ~ (p=0.444 n=5+5) PathUnescape/#2-4 297ns ± 1% 295ns ± 3% ~ (p=0.738 n=5+5) PathUnescape/#3-4 557ns ± 3% 553ns ± 2% ~ (p=0.810 n=5+5) PathUnescape/#04-4 2.94µs ± 2% 2.97µs ± 2% ~ (p=0.222 n=5+5) Change-Id: I7e6d64cd5f8f5218cb40f52f0015168a8674aabb Reviewed-on: https://go-review.googlesource.com/c/go/+/168883 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
@vdobler
I still did not quite get what exactly do you want regarding the "opaque struct" (previous discussion here: [0], CL: [1]). What variables would go in the struct and what code from func stable should go in its methods? I think you may have imagined a struct with very many methods, and I fear that that would increase code size substantially. For example, do you know that even before calling func merge in func stable we have to check if block indices coincide with BDS (block distribution storage) or MIB (movement imitation buffer) indices (so as to restrict the blocks to their proper range before merging, without including BDS or MIB) [2]?
I do see that it would probably be better to pass references to structs to func merge instead of all those integer variables.
On the other hand there is a related change possible that I only did not include before because of Go doing little inlining: the code in func stable from [3] to [4] could be separated out into a function. That would eliminate the identifiers/variables bds0, bds1, backupBDS0, backupBDS1 from func stable; that is why I think you would like it ...?
Sidenote: the future changes to my code should probably happen here on branches other than master instead of on the Go Gerrit instance because of rsc being annoyed by the amount of patchsets.
[0] golang/go#25137
[1] https://go-review.googlesource.com/c/go/+/101415
[2] https://go-review.googlesource.com/c/go/+/101415/41/src/sort/sort.go#1246
[3] https://go-review.googlesource.com/c/go/+/101415/41/src/sort/sort.go#1009
[4] https://go-review.googlesource.com/c/go/+/101415/41/src/sort/sort.go#1170
The text was updated successfully, but these errors were encountered: