perf(internal): add small buffer tiers and route put by capacity - #221
Closed
darakanoit wants to merge 3 commits into
Closed
darakanoit wants to merge 3 commits into
darakanoit wants to merge 3 commits into
Conversation
The buffer pool served every request up to 1 MB from the 1 MB tier, so a 200-byte payload held a 1 MB buffer while in flight. With 64 workers receiving at once that is 64 MB of resident memory for a few kilobytes of data, and every pool miss zeroes a full megabyte. put routed a buffer by the requested size, not by its capacity. A buffer above 10 MB is allocated with make and was stored in the 10 MB tier, where it was handed out for every later request in that tier and never released while the pool stayed warm. Add 4 KB, 16 KB, 64 KB and 256 KB tiers below the existing 1 MB, 5 MB and 10 MB ones, hold them in an array instead of a sync.Map, and route put by the buffer's capacity: a buffer goes back only to the tier whose size it has, anything else is dropped. Signed-off-by: darakanoit <dara.kamaliev@gmail.com>
… over the tiers Review note on roadrunner-server#222, which carries this file. The loop cost grew with the tier: get plus put took 7.7 ns on the 4 KB tier, 10.0 ns on 64 KB and 11.7 ns on 5 MB. With one pool per tier and a switch in both directions the cost is flat at about 7.7 ns. Signed-off-by: darakanoit <dara.kamaliev@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #221 +/- ##
==========================================
- Coverage 78.56% 78.37% -0.19%
==========================================
Files 9 9
Lines 695 689 -6
==========================================
- Hits 546 540 -6
Misses 149 149 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
Hey @darakanoit 👋🏻 |
Author
|
Makes sense, thanks. Closing this one; #222 already carries this change and its description now covers both. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for This PR
Follow-up to roadrunner-server/roadrunner#2395 and #220. Problem statement: roadrunner-server/roadrunner#2401. Two problems in the tiered buffer pool in
internal/bpool.go, which since #220 serves both the send and the receive path.The smallest tier is 1 MB. Every payload up to 1 MB, and every options read of 4 to 40 bytes, takes a 1 MB buffer while in flight. Resident memory grows with the number of workers, not with the size of the data. Measured with N goroutines each holding a buffer for a 200-byte payload, which is what N workers do while their responses are in flight:
The memory survives one GC in the victim cache, so under steady load it is permanent. A pool miss also zeroes the whole tier: 37 µs for 1 MB on an M3.
putroutes by the requested size, not by the buffer's capacity. A buffer above 10 MB is allocated withmakeinget, butputstores it in the 10 MB tier. From then on it is handed out for every 5 to 10 MB request and never released while the pool is warm. The defaultmax_request_sizein the http plugin is 1 GB, so a single large upload or response reaches this path. Reproduction on master:get(50 << 20),put(50 << 20, buf), thenget(6 << 20)returns a buffer withcap == 50 MB.Description of Changes
tierSizesarray instead of async.Mapkeyed by size.getis a scan over sevenuint32comparisons.put(data)routes bycap(*data)and returns a buffer only to the tier whose size equals its capacity. Anything else, including an oversizedmakeresult, is dropped. This is the same rule asputDataBufferChunkinx/net/http2andBinaryTieredBufferPool.Putin grpc-go, which both key on the buffer rather than on the request.receive_test.gointobpool_test.go: every tier boundary, an oversized buffer never entering the 10 MB tier, a foreign slice never entering a tier, a 4 KB buffer returning to the 4 KB tier. The last three pinGOMAXPROCS(1)so thesync.Poolprivate slot makes the outcome deterministic. Ported onto master they fail on every run without the race detector.Preallocatecontract are unchanged. Onlyinternalis touched.Benchmarks,
BenchmarkReceivePathandBenchmarkSendPath, master and this branch interleaved over 5 rounds, benchstat over 10 samples each, M3:Allocations per op are identical in every case. The gain on small frames is the
sync.Maplookup and the two type assertions that are gone fromgetandput.closes: roadrunner-server/roadrunner#2401
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.
PR Checklist
git commit -s) or (git commit -S).CHANGELOG.md.