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
Closed

net/http: Optimize internal Cookie functions #17339

SchumacherFM opened this issue Oct 4, 2016 · 4 comments

Comments

@SchumacherFM
Copy link
Contributor

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
Copy link
Contributor

bradfitz 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 this to the Unplanned milestone Oct 4, 2016
@SchumacherFM
Copy link
Contributor Author

SchumacherFM 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
Copy link
Contributor

@SchumacherFM
Copy link
Contributor Author

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.
Projects
None yet
Development

No branches or pull requests

3 participants