PR feedback

Author: JamesNK

src/Servers/Kestrel/Core/src/Internal/Http2/HPack/Http2HPackEncoder.cs

@@ -34,6 +34,8 @@ public Http2HPackEncoder(uint maxHeaderTableSize = Http2PeerSettings.DefaultHead
             _maxHeaderListSize = Http2PeerSettings.DefaultMaxHeaderListSize;
             Head = new HPackHeaderEntry();
             Head.Initialize(-1, string.Empty, string.Empty, int.MaxValue, null);
+            // Bucket count balances memory usage and the expected low number of headers (constrained by the header table size).
+            // Performance with different bucket counts hasn't been measured in detail.
             _headerBuckets = new HPackHeaderEntry[16];
             _hashMask = (byte)(_headerBuckets.Length - 1);
             Head.Before = Head.After = Head;
@@ -52,7 +54,7 @@ public void UpdateMaxHeaderTableSize(uint maxHeaderTableSize)
             }
         }
 
-        public void SetMaxHeaderListSize(uint maxHeaderListSize)
+        public void UpdateMaxHeaderListSize(uint maxHeaderListSize)
         {
             _maxHeaderListSize = maxHeaderListSize;
         }

src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs

@@ -717,9 +717,6 @@ private Task ProcessSettingsFrameAsync(in ReadOnlySequence<byte> payload)
 
                 _clientSettings.Update(Http2FrameReader.ReadSettings(payload));
 
-                // Maximum encoder size is limited by the configured max on the server
-                _frameWriter.UpdateMaxHeaderTableSize(Math.Min(_clientSettings.HeaderTableSize, (uint)Limits.Http2.HeaderTableSize));
-
                 // Ack before we update the windows, they could send data immediately.
                 var ackTask = _frameWriter.WriteSettingsAckAsync();
 
@@ -746,6 +743,15 @@ private Task ProcessSettingsFrameAsync(in ReadOnlySequence<byte> payload)
                     }
                 }
 
+                // Maximum HPack encoder size is limited by Http2Limits.HeaderTableSize, configured max the server.
+                //
+                // Note that the client HPack decoder doesn't care about the ACK so we don't need to lock sending the
+                // ACK and updating the table size on the server together.
+                // The client will wait until a size agreed upon by it (sent in SETTINGS_HEADER_TABLE_SIZE) and the
+                // server (sent as a dynamic table size update in the next HEADERS frame) is received before applying
+                // the new size.
+                _frameWriter.UpdateMaxHeaderTableSize(Math.Min(_clientSettings.HeaderTableSize, (uint)Limits.Http2.HeaderTableSize));
+
                 return ackTask.AsTask();
             }
             catch (Http2SettingsParameterOutOfRangeException ex)