More HTTP/2 performance (and a few functional) improvements (#36246)
authorStephen Toub <stoub@microsoft.com>
Fri, 15 May 2020 21:30:17 +0000 (17:30 -0400)
committerGitHub <noreply@github.com>
Fri, 15 May 2020 21:30:17 +0000 (17:30 -0400)
commit5b112c310f9ea28ca7a06941787f2bea7eb315eb
tree373f40a5df6dbbb19bbc02f7e23699a9c8f82a9f
parent1d4ef57892187ed753e519b91f2b3da2afc6e130
More HTTP/2 performance (and a few functional) improvements (#36246)

* Use span instead of array for StatusHeaderName

* Fix potential leak into CancellationToken

We need to dispose of the linked token source we create.

Also cleaned up some unnecessarily complicated code nearby.

* Fix HttpConnectionBase.LogExceptions

My previous changes here were flawed for the sync-completing case, and also accidentally introduced a closure.

* Clean up protocol state if/else cascades into switches

* Consolidate a bunch of exception throws into helpers

* Fix cancellation handling of WaitFor100ContinueAsync

* Change AsyncMutex's linked list to be circular

* Remove linked token sources

Rather than creating temporary linked token sources with the request body source and the supplied cancellation token, we can instead just register with the supplied token to cancel the request body source.  This is valid because canceling any part of sending a request cancels any further sending of that request, not just that one constituent operation.

* Avoid registering for linked cancellation until absolutely necessary

We can avoid registering with the cancellation token until after we know that our send is completing asynchronously.

* Remove closure/delegate allocation from WaitForDataAsync

`this` was being closed over accidentally.  I can't wait for static lambdas.

* Avoid a temporary list for storing trailers

Since it only exists to be defensive but we don't expect response.TrailingHeaders to be accessed until after the whole response has been received, we can store the headers into an HttpResponseHeaders instance and swap that instance in at the end.  Best and common case, we avoid the list. Worst and uncommon case, we pay the overhead of the extra HttpResponseHeaders instead of the List.

* Delete dead AcquireWriteLockAsync method

* Reduce header frame overhead

Minor optimizations to improve the asm

* Remove unnecessary throws with GetShutdownException

* Avoid extra lock in SendHeadersAsync

* Move Http2Stream construction out of lock

Makes a significant impact on reducing lock contention.

* Streamline RemoveStream

Including moving credit adjustment out of the lock

* Move response message allocation to ctor

Remove it from within the lock

* Reorder interfaces on Http2Stream

IHttpTrace doesn't need to be prioritized.

* Address PR feedback
src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cancellation.cs
src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpResponseHeaders.cs
src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs
src/libraries/System.Net.Http/src/System/Threading/AsyncMutex.cs