[#2496] improvement(server): Improve the performance of flushing single buffer #2523
Conversation
There was a problem hiding this comment.
Nick work @xianjingfeng . I observed some similar performance problems in current java based shuffle-server.
| isHugePartition, | ||
| buffer.getEncodedLength(), | ||
| buffer.getBlockCount()); | ||
| synchronized (this) { |
There was a problem hiding this comment.
Do we need to lock all when flushing? From my point, we could just use the tryLock to trigger the flush?
There was a problem hiding this comment.
Maybe we can use buffer as the lock
There was a problem hiding this comment.
Yes. If the buffer’s internal operations are thread-safe, flushing and adding can happen concurrently on the same buffer.
In this case, the external flush can be triggered once, guarded by a Lock — using only tryLock
There was a problem hiding this comment.
But this could be finished in the another PR if you want
There was a problem hiding this comment.
My mistake, this PR is scoped in the flushSingleBuffer . The lock neen't optimize, synchorize is enough and good.
|
Thanks for your review. @zuston |
…uffer flushing (#2524) ### What changes were proposed in this pull request? Use tryLock instead of synchronized when flush buffer ### Why are the changes needed? To followup the #2523 for better performance, this optimization is mentioned in #2523 (comment) ### Does this PR introduce any user-facing change? No. ### How was this patch tested? CI
What changes were proposed in this pull request?
Improve the performance of flushing single buffer
Why are the changes needed?
Fix: #2496
Does this PR introduce any user-facing change?
No.
How was this patch tested?
CI