Re: [tomcat] branch master updated (519f6f8 -> 18e0e3f)
On Mon, Sep 28, 2020 at 11:59 AM Mark Thomas wrote: > On 28/09/2020 09:13, Martin Grigorov wrote: > > > > > Good news: there are no regressions! > > So far, so good. > > > Neutral news: there are no performance improvements according to my > tests. > > I use > > > https://github.com/martin-g/http2-server-perf-tests/blob/128f24e27ef96ee31740db4130855bea2c021793/java/tomcat/src/main/java/info/mgsolutions/tomcat/Main.java > > to test HTTP2, h2c, HTTP and HTTPS > > HTTP: 28K reqs/s > > H2C: 14K reqs/s > > HTTP2: 11K req/s > > > > I still use Vegeta as a test client + my patch to ignore CANCEL errors > > You might want to try without that CANCEL error patch. The Tomcat code > should be more robust against that sort of error now as it retains state > for at least 5x max concurrent streams now. > Confirmed! Now Tomcat does not complain about the late CANCEL! > > I see slightly different figures when testing locally with Vegeta: > > HTTP: 30.5k req/s > HTTPS: 18.0k req/s > h2c:20.7k req/s > h2: 17.2k req/s > > There are a couple of unexpected things there: > - large drop from HTTP to HTTPS > - similar HTTP figures for your test and mine but very different h2/h2c > figures > > Given how unrepresentative local testing is I'm not entirely surprised. > > I'm not planning on spending any time digging into these differences. > Thank you for the improvements ! > > Running load tests with a profiler shows the biggest bottleneck is > around I/O. There might be some small gains to be made with better > buffering to reduce the number of network writes but implementing that > change is more complex for the async HTTP/2 implementation. > Yes, I also see only network IO operations in the profiler now. > > Mark > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
Re: [tomcat] branch master updated (519f6f8 -> 18e0e3f)
On Mon, Sep 28, 2020 at 10:59 AM Mark Thomas wrote: > On 28/09/2020 09:13, Martin Grigorov wrote: > > > > > Good news: there are no regressions! > > So far, so good. > > > Neutral news: there are no performance improvements according to my > tests. > > I use > > > https://github.com/martin-g/http2-server-perf-tests/blob/128f24e27ef96ee31740db4130855bea2c021793/java/tomcat/src/main/java/info/mgsolutions/tomcat/Main.java > > to test HTTP2, h2c, HTTP and HTTPS > > HTTP: 28K reqs/s > > H2C: 14K reqs/s > > HTTP2: 11K req/s > > > > I still use Vegeta as a test client + my patch to ignore CANCEL errors > > You might want to try without that CANCEL error patch. The Tomcat code > should be more robust against that sort of error now as it retains state > for at least 5x max concurrent streams now. > > I see slightly different figures when testing locally with Vegeta: > > HTTP: 30.5k req/s > HTTPS: 18.0k req/s > h2c:20.7k req/s > h2: 17.2k req/s > > There are a couple of unexpected things there: > - large drop from HTTP to HTTPS > - similar HTTP figures for your test and mine but very different h2/h2c > figures > > Given how unrepresentative local testing is I'm not entirely surprised. > > I'm not planning on spending any time digging into these differences. > > Running load tests with a profiler shows the biggest bottleneck is > around I/O. There might be some small gains to be made with better > buffering to reduce the number of network writes but implementing that > change is more complex for the async HTTP/2 implementation. > I was planning to do a buffer queue at one point but then it didn't look so nice and useful in practice. Rémy
Re: [tomcat] branch master updated (519f6f8 -> 18e0e3f)
On 28/09/2020 09:13, Martin Grigorov wrote: > Good news: there are no regressions! So far, so good. > Neutral news: there are no performance improvements according to my tests. > I use > https://github.com/martin-g/http2-server-perf-tests/blob/128f24e27ef96ee31740db4130855bea2c021793/java/tomcat/src/main/java/info/mgsolutions/tomcat/Main.java > to test HTTP2, h2c, HTTP and HTTPS > HTTP: 28K reqs/s > H2C: 14K reqs/s > HTTP2: 11K req/s > > I still use Vegeta as a test client + my patch to ignore CANCEL errors You might want to try without that CANCEL error patch. The Tomcat code should be more robust against that sort of error now as it retains state for at least 5x max concurrent streams now. I see slightly different figures when testing locally with Vegeta: HTTP: 30.5k req/s HTTPS: 18.0k req/s h2c:20.7k req/s h2: 17.2k req/s There are a couple of unexpected things there: - large drop from HTTP to HTTPS - similar HTTP figures for your test and mine but very different h2/h2c figures Given how unrepresentative local testing is I'm not entirely surprised. I'm not planning on spending any time digging into these differences. Running load tests with a profiler shows the biggest bottleneck is around I/O. There might be some small gains to be made with better buffering to reduce the number of network writes but implementing that change is more complex for the async HTTP/2 implementation. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch master updated (519f6f8 -> 18e0e3f)
On Mon, Sep 28, 2020 at 9:28 AM Martin Grigorov wrote: > Hi, > > On Fri, Sep 25, 2020 at 4:40 PM Mark Thomas wrote: > >> On 25/09/2020 14:34, ma...@apache.org wrote: >> > This is an automated email from the ASF dual-hosted git repository. >> > >> > markt pushed a change to branch master >> > in repository https://gitbox.apache.org/repos/asf/tomcat.git. >> > >> > >> > from 519f6f8 Fix a typo in changelog >> > new ee25710 Reduce memory footprint of closed http/2 streams >> > new f6e656e Reduce memory footprint of closed http/2 streams >> > new 0f66705 Reduce memory footprint of closed http/2 streams >> > new fa6df26 Reduce memory footprint of closed http/2 streams >> > new 9ee7d6a Reduce memory footprint of closed http/2 streams >> > new 30df6a0 Reduce memory footprint of closed http/2 streams >> > new 0a78ae9 Fully replace Stream with RecycledStream >> > new 954cbff Refactor the pruning so more stream info is retained >> for longer >> > new 18e0e3f Update changelog >> >> All, >> >> This set of changes provided multiple improvements to the HTTP/2 >> implementation: >> >> 1. The memory used by closed streams is significantly reduced (hopefully >>without the NPEs triggered by my previous attempt). >> 2. We retain information on more streams in the priority tree. This >>enables greater latitude for clients that send frames for streams >>that have been closed (while not increasing memory). >> 3. We no longer aggressively prune the priority tree to just active >>streams (this was causing problems in some usage patterns). >> >> My plan is to let this run on the CI for a few days and then - assuming >> no issues - back-port it early next week. >> > > I will cherry-pick these commits to a branch [1] that I use for testing > and profiling. > I'll report back with the results! > Good news: there are no regressions! Neutral news: there are no performance improvements according to my tests. I use https://github.com/martin-g/http2-server-perf-tests/blob/128f24e27ef96ee31740db4130855bea2c021793/java/tomcat/src/main/java/info/mgsolutions/tomcat/Main.java to test HTTP2, h2c, HTTP and HTTPS HTTP: 28K reqs/s H2C: 14K reqs/s HTTP2: 11K req/s I still use Vegeta as a test client + my patch to ignore CANCEL errors because I haven't found a better load client. I've reported a bug against Golang net package: https://github.com/golang/go/issues/41570 > > 1. > https://github.com/martin-g/tomcat/tree/http2/use-navigateable-map-for-http2-streams > > > >> >> Mark >> >> - >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> >>
Re: [tomcat] branch master updated (519f6f8 -> 18e0e3f)
Hi, On Fri, Sep 25, 2020 at 4:40 PM Mark Thomas wrote: > On 25/09/2020 14:34, ma...@apache.org wrote: > > This is an automated email from the ASF dual-hosted git repository. > > > > markt pushed a change to branch master > > in repository https://gitbox.apache.org/repos/asf/tomcat.git. > > > > > > from 519f6f8 Fix a typo in changelog > > new ee25710 Reduce memory footprint of closed http/2 streams > > new f6e656e Reduce memory footprint of closed http/2 streams > > new 0f66705 Reduce memory footprint of closed http/2 streams > > new fa6df26 Reduce memory footprint of closed http/2 streams > > new 9ee7d6a Reduce memory footprint of closed http/2 streams > > new 30df6a0 Reduce memory footprint of closed http/2 streams > > new 0a78ae9 Fully replace Stream with RecycledStream > > new 954cbff Refactor the pruning so more stream info is retained > for longer > > new 18e0e3f Update changelog > > All, > > This set of changes provided multiple improvements to the HTTP/2 > implementation: > > 1. The memory used by closed streams is significantly reduced (hopefully >without the NPEs triggered by my previous attempt). > 2. We retain information on more streams in the priority tree. This >enables greater latitude for clients that send frames for streams >that have been closed (while not increasing memory). > 3. We no longer aggressively prune the priority tree to just active >streams (this was causing problems in some usage patterns). > > My plan is to let this run on the CI for a few days and then - assuming > no issues - back-port it early next week. > I will cherry-pick these commits to a branch [1] that I use for testing and profiling. I'll report back with the results! 1. https://github.com/martin-g/tomcat/tree/http2/use-navigateable-map-for-http2-streams > > Mark > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
Re: [tomcat] branch master updated (519f6f8 -> 18e0e3f)
On 25/09/2020 14:34, ma...@apache.org wrote: > This is an automated email from the ASF dual-hosted git repository. > > markt pushed a change to branch master > in repository https://gitbox.apache.org/repos/asf/tomcat.git. > > > from 519f6f8 Fix a typo in changelog > new ee25710 Reduce memory footprint of closed http/2 streams > new f6e656e Reduce memory footprint of closed http/2 streams > new 0f66705 Reduce memory footprint of closed http/2 streams > new fa6df26 Reduce memory footprint of closed http/2 streams > new 9ee7d6a Reduce memory footprint of closed http/2 streams > new 30df6a0 Reduce memory footprint of closed http/2 streams > new 0a78ae9 Fully replace Stream with RecycledStream > new 954cbff Refactor the pruning so more stream info is retained for > longer > new 18e0e3f Update changelog All, This set of changes provided multiple improvements to the HTTP/2 implementation: 1. The memory used by closed streams is significantly reduced (hopefully without the NPEs triggered by my previous attempt). 2. We retain information on more streams in the priority tree. This enables greater latitude for clients that send frames for streams that have been closed (while not increasing memory). 3. We no longer aggressively prune the priority tree to just active streams (this was causing problems in some usage patterns). My plan is to let this run on the CI for a few days and then - assuming no issues - back-port it early next week. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch master updated (519f6f8 -> 18e0e3f)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 519f6f8 Fix a typo in changelog
new ee25710 Reduce memory footprint of closed http/2 streams
new f6e656e Reduce memory footprint of closed http/2 streams
new 0f66705 Reduce memory footprint of closed http/2 streams
new fa6df26 Reduce memory footprint of closed http/2 streams
new 9ee7d6a Reduce memory footprint of closed http/2 streams
new 30df6a0 Reduce memory footprint of closed http/2 streams
new 0a78ae9 Fully replace Stream with RecycledStream
new 954cbff Refactor the pruning so more stream info is retained for
longer
new 18e0e3f Update changelog
The 9 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/coyote/http2/AbstractNonZeroStream.java | 143 +++
java/org/apache/coyote/http2/AbstractStream.java | 9 +-
.../apache/coyote/http2/Http2UpgradeHandler.java | 264 +
...tionSettingsRemote.java => RecycledStream.java} | 23 +-
java/org/apache/coyote/http2/Stream.java | 90 +--
.../apache/coyote/http2/StreamStateMachine.java| 21 +-
webapps/docs/changelog.xml | 5 +
7 files changed, 348 insertions(+), 207 deletions(-)
create mode 100644 java/org/apache/coyote/http2/AbstractNonZeroStream.java
copy java/org/apache/coyote/http2/{ConnectionSettingsRemote.java =>
RecycledStream.java} (61%)
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
