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

net/http: Optimize internal Cookie functions #17339

Closed
SchumacherFM opened this issue Oct 4, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@SchumacherFM
Copy link
Contributor

commented Oct 4, 2016

I've send a CL some time ago and wonders what else I need to do to get my first contribution to Go merged into master?

https://go-review.googlesource.com/#/c/27850/

net/http: Optimize internal Cookie functions

- Pre-calculate *Cookie slice in read cookie functions
- readSetCookies: pre-allocs depending on the count of Set-Cookies, avoid
  allocs when no Set-Cookie is present
- readCookies: calculate *Cookie slice length
- Rename success variable to ok; avoid else
- Refactor Cookie.String() to use less allocations
- Remove fmt package and replace with writes to a bytes.Buffer
- Add BenchmarkReadSetCookies() and BenchmarkReadCookies()

Add BenchmarkCookieString()
1000000   2153 ns/op 520 B/op 10 allocs/op <= Original Go 1.7
1000000   1221 ns/op 384 B/op  3 allocs/op <= Modified Go 1.7

That benchmark is from my MacBook Pro 2.4 GHz Intel Core i5 ;-)

If I need to change anything in the CL let me know or just merge (or not) and close this issue.

Thanks!

What version of Go are you using (go version)?

go version devel +79db162 Fri Sep 30 05:15:19 2016 +0000 darwin/amd64

@bradfitz

This comment has been minimized.

Copy link
Member

commented Oct 4, 2016

Sorry, but this doesn't seem important enough to spend time reviewing, at least for me. Is this actually a bottleneck for your application, or did you just want to optimize it for fun? If you have spare time, I have a list of bugs you might be interested in.

Maybe somebody else wants to review.

@bradfitz bradfitz added the Performance label Oct 4, 2016

@bradfitz bradfitz added this to the Unplanned milestone Oct 4, 2016

@bradfitz bradfitz added the help wanted label Oct 4, 2016

@SchumacherFM

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2016

This has been optimized just for fun and is not a bottleneck in my or any other app.

I'm also interested in fixing bugs.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Oct 12, 2016

@bradfitz bradfitz closed this Oct 12, 2016

@SchumacherFM

This comment has been minimized.

Copy link
Contributor Author

commented Oct 12, 2016

Thank you very very much for merging!

@golang golang locked and limited conversation to collaborators Oct 12, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.