Closed alexanderkiel closed 3 years ago
Commit messages of this repository should follow the seven rules of a great Git commit message, as mentioned in the project's contributing guidelines. It looks like there's a few issues with the commit messages in this pull request:
Thanks for the PR, and apologies for taking a little while to look at this in detail.
The code looks good, and the fix is very much appreciated. Can you change the commit message to:
Fix performance issue with async responses
For async responses, the function rest.util.servlet/make-output-stream
creates a subclass of java.io.FilterOutputStream in order to complete
the async context after the output stream has been closed.
Unfortunately for the performance, FilterOutputStream implements the
write(byte[], int, int) method by looping through the byte array and
calling the single byte write(int) method.
The solution is simple: add an appropriate write method to the proxy,
that calls write(byte[], int, int) on the underlying servlet output
stream.
The details of the full investigation can be left to the PR if anyone's interested in looking it up. The subject line of commits by convention don't capitalise every word.
I've changed the commit message. Thanks for accepting my PR.
For async responses, the function
rest.util.servlet/make-output-stream
creates a subclass ofjava.io.FilterOutputStream
in order to complete the async context after the output stream has been closed.Unfortunately for the performance, FilterOutputStream implements the
write(byte[], int, int)
method by looping through the byte array and calling the single bytewrite(int)
method.The solution is simple. I just added an appropriate write method to the proxy, that calls
write(byte[], int, int)
on the underlying servlet output stream.I found this issue while profiling a real application which uses
ring-jetty-adapter
in async mode. However, I extendedring-bench
to be able show the performance issue even in micro benchmarks. Please note that the difference is even higher in the real world, because the output stream of Jetty is way more costly for writing single bytes as the ByteArrayOutputStream used in the benchmark.The following output of my extended implementation of
ring-bench
shows the situation before my fix:$ lein bench Reading project from ring-bench Benchmarking... :build - n/a - 207278.78 ops/s (σ=385.14) :update - 128 - 258979.64 ops/s (σ=221.42) :update - 1024 - 217854.74 ops/s (σ=818.57) :update - 8192 - 99921.47 ops/s (σ=171.63) :update - 65536 - 14295.33 ops/s (σ=126.82) :update-async - 128 - 101890.34 ops/s (σ=101.51) :update-async - 1024 - 18157.46 ops/s (σ=75.54) :update-async - 8192 - 1611.05 ops/s (σ=6.62) :update-async - 65536 - 304.54 ops/s (σ=1.55) :handler - 128 - 113733.90 ops/s (σ=199.07) :handler - 1024 - 105209.47 ops/s (σ=115.30) :handler - 8192 - 52326.65 ops/s (σ=109.66) :handler - 65536 - 13280.15 ops/s (σ=47.32)
Here I use different body sizes as parameter for the benchmark shown in the second column. For 128 byte bodies the difference is 2.5-fold while for 64kb bodies it is about 50-fold.
After my fix, the numbers are nearly even:
$ lein bench Reading project from ring-bench Benchmarking... :build - n/a - 189232.43 ops/s (σ=168.91) :update - 128 - 266048.82 ops/s (σ=396.09) :update - 1024 - 221138.68 ops/s (σ=679.52) :update - 8192 - 100312.33 ops/s (σ=226.14) :update - 65536 - 14250.45 ops/s (σ=37.64) :update-async - 128 - 256945.70 ops/s (σ=499.25) :update-async - 1024 - 212027.74 ops/s (σ=539.64) :update-async - 8192 - 70950.87 ops/s (σ=93.68) :update-async - 65536 - 14227.39 ops/s (σ=17.58) :handler - 128 - 113094.90 ops/s (σ=192.89) :handler - 1024 - 105370.23 ops/s (σ=627.62) :handler - 8192 - 66245.49 ops/s (σ=74.81) :handler - 65536 - 13142.11 ops/s (σ=70.18)