-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix InboundBuffer::pause & write performance #5144
Conversation
@vietj This change here is not in conflict with #5143 given that it works on different synchronization(s) happening in a inefficient way, so, if you agree on the changes, could be safely merged. Just for reference, it improves by ~5% few benchmarks using plaintext, so, not a game changer; but given that fixing the presence of pausing in Quarkus is NOT possible nor is possible to improve the inbound buffer to not create any array q, in case we end a request while still paused, this is the less destructive and simpler change to improve things I could think about. Said that, sadly profilers can consistently lie and, in an experiment, I've tried removing |
@vietj ping |
@vietj how this look bud? |
this looks good, I need to review it. can you have a quick look at https://github.com/eclipse-vertx/vert.x/pull/5164/files ? |
@franz1981 before proceeding with this, I think we can review and merge https://github.com/eclipse-vertx/vert.x/pull/5143/files which does remove synchronization use in |
ab6f258
to
a789e11
Compare
@vietj I've addressed your comments and added some tests for the additional unsynchronized methods. |
@vietj this is fine than? So we can bring it in the next quarkus release |
Ping @vietj ? This is ok to merge? So I can verify on the next techempower round if helps? |
This patch improve Quarkus techempower plaintext throughput of ~5% in our lab; nothing fancy but seems related to the excessive synchronization which, by using instance field to synchronize with, doesn't allow the JVM to perform the https://shipilev.net/jvm/anatomy-quarks/1-lock-coarsening-for-loops/ (lock coarsening) optimization.
I've applied an optimization to
onEnd
which benefit vertx as well, which often observe non-paused requestsi.e. reducing the synchronization over the connection while completing the request.