-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
http: implement slab allocation for HTTP header parsing #61375
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
base: main
Are you sure you want to change the base?
http: implement slab allocation for HTTP header parsing #61375
Conversation
|
Review requested:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request implements slab allocation for HTTP header parsing to optimize performance by reducing heap allocations. The implementation addresses a TODO comment about O(n) allocations being inefficient and achieves a reported 52% performance improvement in HTTP header parsing benchmarks.
Changes:
- Added a 128-byte fixed-size buffer (slab) to each
StringPtrinstance for caching small strings - Modified
Save()andUpdate()methods to use slab allocation before falling back to heap allocation - Added
using_slab_flag to track slab usage state alongside existingon_heap_flag
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #61375 +/- ##
========================================
Coverage 88.51% 88.51%
========================================
Files 704 704
Lines 208776 209024 +248
Branches 40301 40361 +60
========================================
+ Hits 184803 185028 +225
+ Misses 15966 15963 -3
- Partials 8007 8033 +26
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/node_http_parser.cc
Outdated
| bool on_heap_ = false; | ||
| bool using_slab_ = false; | ||
| size_t size_ = 0; | ||
| char slab_[kSlabSize]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This overall seems like we're re-inventing MaybeStackBuffer here – I think we could just re-use that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! , I've refactored this to use MaybeStackBuffer as the backing store for a allocator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new benchmark result :
asis (main):
./out/Release/node benchmark/http/bench-parser.js
http/bench-parser.js n=100000 len=4: 1,728,678.1850683796
http/bench-parser.js n=100000 len=8: 1,395,311.5272280464
http/bench-parser.js n=100000 len=16: 981,492.7277523204
http/bench-parser.js n=100000 len=32: 641,907.2347759664
./out/Release/node benchmark/http/bench-parser-fragmented.js
http/bench-parser-fragmented.js n=100000 frags=2 len=8: 1,092,175.0170709684
http/bench-parser-fragmented.js n=100000 frags=4 len=8: 883,046.0674095292
http/bench-parser-fragmented.js n=100000 frags=8 len=8: 772,164.5013385202
http/bench-parser-fragmented.js n=100000 frags=2 len=16: 766,964.5365185371
http/bench-parser-fragmented.js n=100000 frags=4 len=16: 640,379.4448008832
http/bench-parser-fragmented.js n=100000 frags=8 len=16: 520,572.0305757982
new:
./out/Release/node benchmark/http/bench-parser.js
http/bench-parser.js n=100000 len=4: 1,764,556.6527588428
http/bench-parser.js n=100000 len=8: 1,500,760.6980788389
http/bench-parser.js n=100000 len=16: 1,095,013.3922327897
http/bench-parser.js n=100000 len=32: 646,285.1635436564
./out/Release/node benchmark/http/bench-parser-fragmented.js
http/bench-parser-fragmented.js n=100000 frags=2 len=8: 1,361,774.7618279774
http/bench-parser-fragmented.js n=100000 frags=4 len=8: 1,263,148.5873261988
http/bench-parser-fragmented.js n=100000 frags=8 len=8: 1,056,199.0301452405
http/bench-parser-fragmented.js n=100000 frags=2 len=16: 1,021,757.915898309
http/bench-parser-fragmented.js n=100000 frags=4 len=16: 923,235.2934387976
http/bench-parser-fragmented.js n=100000 frags=8 len=16: 826,609.3826364499
|
uh sorry, I'm forget send benchmark file |
fix: // TODO(bnoordhuis) Use slab allocation, O(n) allocs is bad.
before:
after:
52% faster HTTP header parsing