-
Notifications
You must be signed in to change notification settings - Fork 186
feat: JMH Benchmark for FileBlockItemWriter and optimized performance #22287
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?
feat: JMH Benchmark for FileBlockItemWriter and optimized performance #22287
Conversation
Signed-off-by: Alex Kehayov <[email protected]>
Signed-off-by: Alex Kehayov <[email protected]>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Could we make a new implementation in a class like FileBlockItemWriterV2? Then could we have the benchmark run against both implementations? And lets make sure the benchmark covers both small blocks and blocks up to say like 500mb |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #22287 +/- ##
============================================
- Coverage 70.97% 70.71% -0.27%
+ Complexity 24632 24631 -1
============================================
Files 2698 2700 +2
Lines 105072 105475 +403
Branches 11043 11088 +45
============================================
+ Hits 74577 74582 +5
- Misses 26472 26870 +398
Partials 4023 4023
... and 13 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Signed-off-by: Alex Kehayov <[email protected]>
|
@derektriley I made some changes to compare both implementations side by side. Also added param for running the bench with 500mb blocks. Attaching the output here. Summary of the results:
Buffer performance:
|
Signed-off-by: Alex Kehayov <[email protected]>
|
After further revising the async approach, I found it causes incompatible behavior that would compromise other system components. Since blocks must be persisted to disk in order and be available on disk immediately when marked CLOSED, I switched to a different async approach that guarantees these conditions, but the performance gains weren't sufficient for the added complexity. |
Signed-off-by: Alex Kehayov <[email protected]>
|
V3 results: Throughput (ops/ms) — V3 is faster Compression ratio — V3 is better Performance: V3 is 8–16x faster across all block sizes. |
|
FWIW there is a ticket for PBJ to explore using Zstd. While that ticket focuses on the gRPC side of things, it may be nice to explore the Zstd implementations mentioned there so everything uses the same implementation. Just a thought. |
Description:
Summarized results:
Overall Performance (Block: 500 items × 2000 bytes)
(1,019 blocks/sec)
(2,166 blocks/sec)
Original bench results:
Bench original.rtf
Optimized bench results:
Bench optimized.rtf
Related issue(s):
Closes #22128
Notes for reviewer:
Unit tests covering the changes and documentation will be added if the approach is considered useful.
Checklist