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

Use sync.Pool to reduce lengthReader allocations #5756

Closed
francislavoie opened this issue Aug 16, 2023 · 5 comments · Fixed by #5848
Closed

Use sync.Pool to reduce lengthReader allocations #5756

francislavoie opened this issue Aug 16, 2023 · 5 comments · Fixed by #5848
Labels
good first issue 🐤 Good for newcomers optimization 📉 Performance or cost improvements
Milestone

Comments

@francislavoie
Copy link
Member

In #5461 we introduced lengthReader to count the amount of bytes read from the request body.

We're allocating a new lengthReader on each request though (but only requests with a body, so not GETs). This is inefficient, it could be improved by using sync.Pool to reuse allocated lengthReader structs.

@francislavoie francislavoie added good first issue 🐤 Good for newcomers optimization 📉 Performance or cost improvements labels Aug 16, 2023
@francislavoie francislavoie added this to the 2.x milestone Aug 16, 2023
@perhapsmaple
Copy link
Contributor

Hi @francislavoie, I would like to work on this issue. Are there any benchmarks that I can use to verify the performance impact of the change? Thanks!

@mholt
Copy link
Member

mholt commented Aug 17, 2023

You can write a simple benchmark test to measure allocations and such. 👍

@shashank-priyadarshi
Copy link

shashank-priyadarshi commented Oct 1, 2023

Hi, is this issue being worked upon? I can take it up otherwise.

@francislavoie
Copy link
Member Author

PRs are always welcome 👍

@shashank-priyadarshi
Copy link

@perhapsmaple could you please let me know if you are still working on this? If not, can I please take this up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue 🐤 Good for newcomers optimization 📉 Performance or cost improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants