Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add more instrumentation to ArrayByteBufferPool and improve its eviction algorithm #11375

Closed
wants to merge 11 commits into from
Closed

Add more instrumentation to ArrayByteBufferPool and improve its eviction algorithm #11375

wants to merge 11 commits into from

Conversation

lorban
Copy link
Contributor

@lorban lorban commented Feb 5, 2024 •
edited
Loading

The current info reported in ArrayByteBufferPool's dump is too limited, this is an attempt at improving that.
This could help with issues like #11326 as we may better understand the state of the pool when it reaches a problematic point.

Also, the eviction algorithm was reworked to fix its most glaring problems.

@lorban
Improve ArrayByteBufferPool instrumentation
b787a96 
instrument ArrayByteBufferPool acquisition sizes

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban lorban added this to the 12.0.x milestone Feb 5, 2024
@lorban lorban requested review from gregw and sbordet February 5, 2024 18:35
@lorban lorban self-assigned this Feb 5, 2024
@lorban
fix checkstyle
312cc80 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
gregw
gregw reviewed Feb 5, 2024
View reviewed changes
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these stats be collected in a ByteBufferPool wrapper?

@lorban
fix test
e632866 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
extract expensive stats into subclass
a4cf35d 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
improve the existing eviction algorithm
d499b9f 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban lorban changed the title Add more instrumentation to ArrayByteBufferPool Add more instrumentation to ArrayByteBufferPool and improve its eviction algorithm Feb 6, 2024
@lorban
Copy link
Contributor Author

lorban commented Feb 6, 2024

@gregw unfortunately, most of those stats are about exposing internal stuff, so it's hard to have them in a wrapper. But I moved the most expensive ones (the requested sizes) to another ABBP subclass so that they're disabled by default.

@lorban lorban requested a review from gregw February 6, 2024 11:28
@lorban lorban marked this pull request as ready for review February 6, 2024 11:28
This was referenced Feb 6, 2024
Closed
Closed
@lorban
make ArrayByteBufferPool resistant to leaks
118d890 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
Copy link
Contributor Author

lorban commented Feb 6, 2024

Note that all the LongAdder I added start to add up as they're on the fast path; they account for 5.78% of the total execution time: https://jenkins.webtide.net/job/load_testing/job/jetty-profiler-12.0.x/116/artifact/core/target/reports/CoreHandlerPerfTest_testNoGzipAsync/server/load-master-2/async-profiler/cpu.html

@lorban
add fence to make test more reliable
944a90b 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
Merge remote-tracking branch 'origin/jetty-12.0.x' into experiment/je…
dd9c707 
…tty-12-more-instrumentation
@lorban lorban mentioned this pull request Feb 7, 2024
Closed
@lorban
replace Pool.stream().toList() with .iterator()
32fc0af 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
properly handle enable returning false
4bef679 
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
stop eviction as soon as the target excess is met and account for swe…
3230e7a 
…pt memory

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
Copy link
Contributor Author

lorban commented Feb 14, 2024

Superseded by #11400

@lorban lorban closed this Feb 14, 2024
@joakime joakime deleted the experiment/jetty-12-more-instrumentation branch February 28, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbordet sbordet Awaiting requested review from sbordet

@gregw gregw Awaiting requested review from gregw

Assignees

@lorban lorban

Labels
Enhancement
Projects
No open projects
Jetty 12.0.7 - FROZEN
Status: ✅ Done
Milestone
12.0.x
Development

Successfully merging this pull request may close these issues.
2 participants
@lorban @gregw