Wrap PrintWriter with BufferedWriter - #540
Conversation
brian-brazil
left a comment
There was a problem hiding this comment.
You've failing CI tests.
There was a problem hiding this comment.
Do we actually need to run a HTTP server to test this?
There was a problem hiding this comment.
This hasn't been addressed.
There was a problem hiding this comment.
It's required since bottleneck is container-specific. For example, with Jetty 8.1.7 the bottleneck seems to be the ByteArrayBuffer instances. But with Jetty 9.4.27, ByteArrayBuffer is no longer an issue but another part of the code is consuming CPU cycles. In either case, the wrapper patch improved the performance by approximately the same amount.
There was a problem hiding this comment.
Can we at least bind to 0 so we don't clash with any existing listeners?
Signed-off-by: Takanori Takase <scm@after-sunrise.com>
Signed-off-by: Takanori Takase <scm@after-sunrise.com>
Signed-off-by: Takanori Takase <scm@after-sunrise.com>
52c3624 to
87918da
Compare
Signed-off-by: Takanori Takase <scm@after-sunrise.com>
|
Thanks! |
|
Thanks for the review & merge :) |
Hi @brian-brazil,
Generating servlet output for a large number of metrics seems to be inefficient, since the underlying
PrintWriterimplementation is generating too manyorg.eclipse.jetty.io.ByteArrayBufferinstances.Performance improved by simply wrapping the
PrintWriterwithBufferedWriter. Microbenchmark (ExampleBenchmark, 10k gauges, 100 iterations, laptop machine) shows that it takes around 2.5 sec with the wrapper, and around 5.5 sec without the wrapper.