-
Notifications
You must be signed in to change notification settings - Fork 464
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
optimize: pooled HeaderScanner and HeaderValueScanner #284
Conversation
Idk if the performance optimization is worth the additional complexity introduced by pooling. We need benchmark to measure and justify performance improvement. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #284 +/- ##
===========================================
- Coverage 75.62% 67.92% -7.71%
===========================================
Files 96 89 -7
Lines 9451 8635 -816
===========================================
- Hits 7147 5865 -1282
- Misses 1857 2397 +540
+ Partials 447 373 -74
... and 45 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
CI need to solve |
Some unit tests in
|
Good job and Is it possible to add a benchmark to see the effect? @Haswf |
Sure, I will run benchmark against current develop branch. |
Update this PR if there is anything new to the bench test 🙏 |
benchmark: old:
new:
It doesn't seem to be optimised, but we'll have to work out why later and not merge it for now. |
What type of PR is this?
optimize: pooled HeaderScanner and HeaderValueScanner
Check the PR title.
(Optional) Translate the PR title into Chinese.
optimize: 池化 HeaderScanner and HeaderValueScanner
(Optional) More detail description for this PR(en: English/zh: Chinese).
en: use sync.Pool to recycle HeaderScanner and HeaderValueScanner. It also contains unit tests to ensure scanner has been properly reset.
zh(optional):
Which issue(s) this PR fixes:
implements #227