Several allocation reductions in SocketsHttpHandler (#34724)

* Remove state machine allocation for SendWithNtConnectionAuthAsync Just manually inline the tiny helper that's only used from one call site. * Remove unnecessary HeaderStoreItemInfo allocations For the common case of a raw string, let the dictionary store the raw string directly rather than always wrapping it in a HeaderStoreItemInfo. * Cache Date and Server response header values For Server, we expect it to be the same for every request on the same connection. For Date, if we're making lots of requests in a high-throughput fashion, we expect many responses on the same connection to have the same value. In both cases, we compare the received value against the last one received, and if it's the same, reuse the same string.
dotnet · Apr 11, 2020 · c06810b · c06810b
1 parent 54f34bb
commit c06810b
Show file tree

Hide file tree

Showing 11 changed files with 321 additions and 224 deletions.
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/ByteArrayHelpers.cs b/src/libraries/System.Net.Http/src/System/Net/Http/ByteArrayHelpers.cs
@@ -8,6 +8,9 @@ namespace System
 {
     internal static class ByteArrayHelpers
     {
+        // TODO: https://github.com/dotnet/runtime/issues/28230
+        // Use Ascii.Equals* when it's available.
+
         internal static bool EqualsOrdinalAsciiIgnoreCase(string left, ReadOnlySpan<byte> right)
         {
             Debug.Assert(left != null, "Expected non-null string");
@@ -22,16 +25,33 @@ internal static bool EqualsOrdinalAsciiIgnoreCase(string left, ReadOnlySpan<byte
                 uint charA = left[i];
                 uint charB = right[i];
 
-                unchecked
+                // We're only interested in ASCII characters here.
+                if ((charA - 'a') <= ('z' - 'a'))
+                    charA -= ('a' - 'A');
+                if ((charB - 'a') <= ('z' - 'a'))
+                    charB -= ('a' - 'A');
+
+                if (charA != charB)
                 {
-                    // We're only interested in ASCII characters here.
-                    if ((charA - 'a') <= ('z' - 'a'))
-                        charA -= ('a' - 'A');
-                    if ((charB - 'a') <= ('z' - 'a'))
-                        charB -= ('a' - 'A');
+                    return false;
                 }
+            }
 
-                if (charA != charB)
+            return true;
+        }
+
+        internal static bool EqualsOrdinalAscii(string left, ReadOnlySpan<byte> right)
+        {
+            Debug.Assert(left != null, "Expected non-null string");
+
+            if (left.Length != right.Length)
+            {
+                return false;
+            }
+
+            for (int i = 0; i < left.Length; i++)
+            {
+                if (left[i] != right[i])
                 {
                     return false;
                 }

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs
@@ -266,6 +266,11 @@ internal HttpRequestHeaders()
         {
         }
 
+        internal HttpRequestHeaders(bool forceHeaderStoreItems)
+            : base(HttpHeaderType.General | HttpHeaderType.Request | HttpHeaderType.Custom, HttpHeaderType.Response, forceHeaderStoreItems)
+        {
+        }
+
         internal override void AddHeaders(HttpHeaders sourceHeaders)
         {
             base.AddHeaders(sourceHeaders);

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs
@@ -45,17 +45,8 @@ public static IWebProxy DefaultProxy
             }
         }
 
-        public HttpRequestHeaders DefaultRequestHeaders
-        {
-            get
-            {
-                if (_defaultRequestHeaders == null)
-                {
-                    _defaultRequestHeaders = new HttpRequestHeaders();
-                }
-                return _defaultRequestHeaders;
-            }
-        }
+        public HttpRequestHeaders DefaultRequestHeaders =>
+            _defaultRequestHeaders ??= new HttpRequestHeaders(forceHeaderStoreItems: true);
 
         public Version DefaultRequestVersion
         {

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
@@ -1027,7 +1027,7 @@ private void WriteHeaderCollection(HttpHeaders headers)
                 return;
             }
 
-            foreach (KeyValuePair<HeaderDescriptor, HttpHeaders.HeaderStoreItemInfo> header in headers.HeaderStore)
+            foreach (KeyValuePair<HeaderDescriptor, object> header in headers.HeaderStore)
             {
                 int headerValuesCount = HttpHeaders.GetValuesAsStrings(header.Key, header.Value, ref _headerValues);
                 Debug.Assert(headerValuesCount > 0, "No values for header??");

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs
@@ -532,23 +532,24 @@ public void OnHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
                             throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_header_name, Encoding.ASCII.GetString(name)));
                         }
 
-                        string headerValue = descriptor.GetHeaderValue(value);
-
                         // Note we ignore the return value from TryAddWithoutValidation;
                         // if the header can't be added, we silently drop it.
                         if (_responseProtocolState == ResponseProtocolState.ExpectingTrailingHeaders)
                         {
                             Debug.Assert(_trailers != null);
+                            string headerValue = descriptor.GetHeaderValue(value);
                             _trailers.Add(KeyValuePair.Create((descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor, headerValue));
                         }
                         else if ((descriptor.HeaderType & HttpHeaderType.Content) == HttpHeaderType.Content)
                         {
                             Debug.Assert(_response != null && _response.Content != null);
+                            string headerValue = descriptor.GetHeaderValue(value);
                             _response.Content.Headers.TryAddWithoutValidation(descriptor, headerValue);
                         }
                         else
                         {
                             Debug.Assert(_response != null);
+                            string headerValue = _connection.GetResponseHeaderValueWithCaching(descriptor, value);
                             _response.Headers.TryAddWithoutValidation((descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor, headerValue);
                         }
                     }

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs
@@ -593,7 +593,7 @@ private void BufferHeaderCollection(HttpHeaders headers)
                 return;
             }
 
-            foreach (KeyValuePair<HeaderDescriptor, HttpHeaders.HeaderStoreItemInfo> header in headers.HeaderStore)
+            foreach (KeyValuePair<HeaderDescriptor, object> header in headers.HeaderStore)
             {
                 int headerValuesCount = HttpHeaders.GetValuesAsStrings(header.Key, header.Value, ref _headerValues);
                 Debug.Assert(headerValuesCount > 0, "No values for header??");
@@ -920,7 +920,7 @@ private void OnHeader(int? staticIndex, HeaderDescriptor descriptor, string? sta
             }
             else
             {
-                string headerValue = staticValue ?? descriptor.GetHeaderValue(literalValue);
+                string headerValue = staticValue ?? _connection.GetResponseHeaderValueWithCaching(descriptor, literalValue);
 
                 switch (_headerState)
                 {

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
@@ -198,7 +198,7 @@ private async ValueTask WriteHeadersAsync(HttpHeaders headers, string? cookiesFr
         {
             if (headers.HeaderStore != null)
             {
-                foreach (KeyValuePair<HeaderDescriptor, HttpHeaders.HeaderStoreItemInfo> header in headers.HeaderStore)
+                foreach (KeyValuePair<HeaderDescriptor, object> header in headers.HeaderStore)
                 {
                     if (header.Key.KnownHeader != null)
                     {
@@ -934,21 +934,25 @@ private static void ParseHeaderNameValue(HttpConnection connection, ReadOnlySpan
                 pos++;
             }
 
-            string headerValue = descriptor.GetHeaderValue(line.Slice(pos));
-
             // Note we ignore the return value from TryAddWithoutValidation. If the header can't be added, we silently drop it.
-            // Request headers returned on the response must be treated as custom headers.
+            ReadOnlySpan<byte> value = line.Slice(pos);
             if (isFromTrailer)
             {
+                string headerValue = descriptor.GetHeaderValue(value);
                 response.TrailingHeaders.TryAddWithoutValidation((descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor, headerValue);
             }
             else if ((descriptor.HeaderType & HttpHeaderType.Content) == HttpHeaderType.Content)
             {
+                string headerValue = descriptor.GetHeaderValue(value);
                 response.Content!.Headers.TryAddWithoutValidation(descriptor, headerValue);
             }
             else
             {
-                response.Headers.TryAddWithoutValidation((descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor, headerValue);
+                // Request headers returned on the response must be treated as custom headers.
+                string headerValue = connection.GetResponseHeaderValueWithCaching(descriptor, value);
+                response.Headers.TryAddWithoutValidation(
+                    (descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor,
+                    headerValue);
             }
         }
 

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs
@@ -2,7 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Diagnostics.CodeAnalysis;
 using System.IO;
+using System.Net.Http.Headers;
 using System.Net.Security;
 using System.Runtime.CompilerServices;
 using System.Threading;
@@ -12,6 +14,30 @@ namespace System.Net.Http
 {
     internal abstract class HttpConnectionBase : IHttpTrace
     {
+        /// <summary>Cached string for the last Date header received on this connection.</summary>
+        private string? _lastDateHeaderValue;
+        /// <summary>Cached string for the last Server header received on this connection.</summary>
+        private string? _lastServerHeaderValue;
+
+        /// <summary>Uses <see cref="HeaderDescriptor.GetHeaderValue"/>, but first special-cases several known headers for which we can use caching.</summary>
+        public string GetResponseHeaderValueWithCaching(HeaderDescriptor descriptor, ReadOnlySpan<byte> value)
+        {
+            return
+                ReferenceEquals(descriptor.KnownHeader, KnownHeaders.Date) ? GetOrAddCachedValue(ref _lastDateHeaderValue, descriptor, value) :
+                ReferenceEquals(descriptor.KnownHeader, KnownHeaders.Server) ? GetOrAddCachedValue(ref _lastServerHeaderValue, descriptor, value) :
+                descriptor.GetHeaderValue(value);
+
+            static string GetOrAddCachedValue([NotNull] ref string? cache, HeaderDescriptor descriptor, ReadOnlySpan<byte> value)
+            {
+                string? lastValue = cache;
+                if (lastValue is null || !ByteArrayHelpers.EqualsOrdinalAscii(lastValue, value))
+                {
+                    cache = lastValue = descriptor.GetHeaderValue(value);
+                }
+                return lastValue;
+            }
+        }
+
         public abstract Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken);
         public abstract void Trace(string message, [CallerMemberName] string? memberName = null);
 

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
@@ -732,7 +732,17 @@ public async Task<HttpResponseMessage> SendWithRetryAsync(HttpRequestMessage req
                 {
                     if (connection is HttpConnection)
                     {
-                        response = await SendWithNtConnectionAuthAsync((HttpConnection)connection, request, doRequestAuth, cancellationToken).ConfigureAwait(false);
+                        ((HttpConnection)connection).Acquire();
+                        try
+                        {
+                            response = await (doRequestAuth && Settings._credentials != null ?
+                                AuthenticationHelper.SendWithNtConnectionAuthAsync(request, Settings._credentials, (HttpConnection)connection, this, cancellationToken) :
+                                SendWithNtProxyAuthAsync((HttpConnection)connection, request, cancellationToken)).ConfigureAwait(false);
+                        }
+                        finally
+                        {
+                            ((HttpConnection)connection).Release();
+                        }
                     }
                     else
                     {

diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs b/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs
@@ -51,7 +51,7 @@ private static Memory<byte> HPackEncode(HttpHeaders headers)
             FillAvailableSpaceWithOnes(buffer);
             string[] headerValues = Array.Empty<string>();
 
-            foreach (KeyValuePair<HeaderDescriptor, HttpHeaders.HeaderStoreItemInfo> header in headers.HeaderStore)
+            foreach (KeyValuePair<HeaderDescriptor, object> header in headers.HeaderStore)
             {
                 int headerValuesCount = HttpHeaders.GetValuesAsStrings(header.Key, header.Value, ref headerValues);
                 Assert.InRange(headerValuesCount, 0, int.MaxValue);