HTTP/3: Http3Stream pooling #34576
HTTP/3: Http3Stream pooling #34576
Conversation
JamesNK
requested review from
BrennanConroy,
Pilchie,
Tratcher and
halter73
as code owners
Tratcher
left a comment
Tratcher
left a comment
There was a problem hiding this comment.
Seems OK in general once the build is fixed. I'm curious to see the benchmarks.
JamesNK
force-pushed
the
jamesnk/http3-http3streapooling3
branch
from
4a31edd to
899e200
Compare
| else | ||
| { | ||
| var persistentStateFeature = streamContext.Features.Get<IPersistentStateFeature>(); | ||
| Debug.Assert(persistentStateFeature != null, $"Required {nameof(IPersistentStateFeature)} not on stream context."); |
There was a problem hiding this comment.
This seems too possible to just assert given the transport is pluggable. Let's null-check and throw.
halter73
left a comment
halter73
left a comment
There was a problem hiding this comment.
I also look forward to the benchmarks.
JamesNK
force-pushed
the
jamesnk/http3-http3streapooling3
branch
from
899e200 to
858d3ee
Compare
dougbu
left a comment
dougbu
left a comment
There was a problem hiding this comment.
New and renamed files need MIT license headers
|
I thought I had already updated old files. Which ones still need to be updated? |
dougbu
left a comment
dougbu
left a comment
There was a problem hiding this comment.
I thought I had already updated old files. Which ones still need to be updated?
I must have looked at this and still had an older copy of the changes cached. Sorry.
I don't see another way to remove my changes request and am signing off only on the headings.
|
Before - 300mb allocated for 10,000 requests After - 50mb allocated for 10,000 requests |
ghost
added this to the
amcasey
added
area-networking
Fixes #33680
This builds on top of the new
IPersistentStateFeature.Http3Stream and many of its associated types (pipes, output producer, decoder) are now reused between requests.
In this PR I also added some basic microbenchmarks that do HTTP/3. Refactoring the test code so it is usable in microbenchmarks is the major of line changes in this PR.
I will get benchmarks comparing allocations before and after.