From 7cd0c2698c38d8fc68258b208f780c3c74865757 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Tue, 25 Dec 2018 20:35:36 -0800 Subject: [PATCH 1/4] Reduce HTTP/2 allocations - Remove per request allocations on the thread pool by implementing IThreadPoolWorkItem on Http2Stream - Made generic version of Http2Stream to store the IHttpApplication instead of using a tuple --- .../src/Internal/Http2/Http2Connection.cs | 10 +++----- .../Core/src/Internal/Http2/Http2Stream.cs | 8 ++++++- .../Core/src/Internal/Http2/Http2StreamOfT.cs | 23 +++++++++++++++++++ .../HttpProtocolFeatureCollectionTests.cs | 15 ++++++++++-- 4 files changed, 46 insertions(+), 10 deletions(-) create mode 100644 src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 9a043b703607..999d4da3ac6e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -605,7 +605,7 @@ private Task ProcessHeadersFrameAsync(IHttpApplication appli } // Start a new stream - _currentHeadersStream = new Http2Stream(new Http2StreamContext + _currentHeadersStream = new Http2Stream(application, new Http2StreamContext { ConnectionId = ConnectionId, StreamId = _incomingFrame.StreamId, @@ -995,13 +995,9 @@ private void StartStream(IHttpApplication application) _activeStreamCount++; _streams[_incomingFrame.StreamId] = _currentHeadersStream; + // Must not allow app code to block the connection handling loop. - ThreadPool.UnsafeQueueUserWorkItem(state => - { - var (app, currentStream) = (Tuple, Http2Stream>)state; - _ = currentStream.ProcessRequestsAsync(app); - }, - new Tuple, Http2Stream>(application, _currentHeadersStream)); + ThreadPool.UnsafeQueueUserWorkItem(_currentHeadersStream, preferLocal: false); } private void ResetRequestHeaderParsingState() diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index da9bbc9dfed8..250f2c4cf88d 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.IO; using System.IO.Pipelines; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; @@ -17,7 +18,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { - public partial class Http2Stream : HttpProtocol + public abstract partial class Http2Stream : HttpProtocol, IThreadPoolWorkItem { private readonly Http2StreamContext _context; private readonly Http2OutputProducer _http2Output; @@ -502,6 +503,11 @@ private Pipe CreateRequestBodyPipe(uint windowSize) } } + /// + /// Used to kick off the request processing loop by derived classes. + /// + public abstract void Execute(); + [Flags] private enum StreamCompletionFlags { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs new file mode 100644 index 000000000000..d026b3a28884 --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs @@ -0,0 +1,23 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Microsoft.AspNetCore.Hosting.Server; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 +{ + public class Http2Stream : Http2Stream + { + private readonly IHttpApplication _application; + + public Http2Stream(IHttpApplication application, Http2StreamContext context) : base(context) + { + _application = application; + } + + public override void Execute() + { + // REVIEW: Should we store this in a field for easy debugging? + _ = ProcessRequestsAsync(_application); + } + } +} diff --git a/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs b/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs index 001d3952b39d..011fa59fe1a6 100644 --- a/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -40,7 +40,7 @@ public HttpProtocolFeatureCollectionTests() _http1Connection.Reset(); _collection = _http1Connection; - var http2Stream = new Http2Stream(context); + var http2Stream = new TestHttp2Stream(context); http2Stream.Reset(); _http2Collection = http2Stream; } @@ -220,5 +220,16 @@ private int SetFeaturesToNonDefault() } private Http1Connection CreateHttp1Connection() => new TestHttp1Connection(_httpConnectionContext); + + private class TestHttp2Stream : Http2Stream + { + public TestHttp2Stream(Http2StreamContext context) : base(context) + { + } + + public override void Execute() + { + } + } } } From adc4e4184c67f261420b69150cd69534725d7016 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Tue, 25 Dec 2018 20:49:38 -0800 Subject: [PATCH 2/4] Remove extra methods that take IHttpApplication --- .../Core/src/Internal/Http2/Http2Connection.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 999d4da3ac6e..4ed606b32c95 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -453,7 +453,7 @@ private Task ProcessFrameAsync(IHttpApplication application, case Http2FrameType.WINDOW_UPDATE: return ProcessWindowUpdateFrameAsync(); case Http2FrameType.CONTINUATION: - return ProcessContinuationFrameAsync(application, payload); + return ProcessContinuationFrameAsync(payload); default: return ProcessUnknownFrameAsync(); } @@ -627,7 +627,7 @@ private Task ProcessHeadersFrameAsync(IHttpApplication appli _headerFlags = _incomingFrame.HeadersFlags; var headersPayload = payload.Slice(0, _incomingFrame.HeadersPayloadLength); // Minus padding - return DecodeHeadersAsync(application, _incomingFrame.HeadersEndHeaders, headersPayload); + return DecodeHeadersAsync(_incomingFrame.HeadersEndHeaders, headersPayload); } } } @@ -878,7 +878,7 @@ private Task ProcessWindowUpdateFrameAsync() return Task.CompletedTask; } - private Task ProcessContinuationFrameAsync(IHttpApplication application, ReadOnlySequence payload) + private Task ProcessContinuationFrameAsync(ReadOnlySequence payload) { if (_currentHeadersStream == null) { @@ -905,7 +905,7 @@ private Task ProcessContinuationFrameAsync(IHttpApplication TimeoutControl.CancelTimeout(); } - return DecodeHeadersAsync(application, _incomingFrame.ContinuationEndHeaders, payload); + return DecodeHeadersAsync(_incomingFrame.ContinuationEndHeaders, payload); } } } @@ -921,7 +921,7 @@ private Task ProcessUnknownFrameAsync() } // This is always called with the _stateLock acquired. - private Task DecodeHeadersAsync(IHttpApplication application, bool endHeaders, ReadOnlySequence payload) + private Task DecodeHeadersAsync(bool endHeaders, ReadOnlySequence payload) { try { @@ -932,7 +932,7 @@ private Task DecodeHeadersAsync(IHttpApplication application { if (_state != Http2ConnectionState.Closed) { - StartStream(application); + StartStream(); } ResetRequestHeaderParsingState(); @@ -969,7 +969,7 @@ private Task DecodeTrailersAsync(bool endHeaders, ReadOnlySequence payload return Task.CompletedTask; } - private void StartStream(IHttpApplication application) + private void StartStream() { if (!_isMethodConnect && (_parsedPseudoHeaderFields & _mandatoryRequestPseudoHeaderFields) != _mandatoryRequestPseudoHeaderFields) { From e75578260ffdefe9b02f3521ee03d13a846bf38c Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 26 Dec 2018 12:53:54 -0800 Subject: [PATCH 3/4] WIP: Pool streams on the Http2Connection - Pool Http2Stream objects based on the MaxConnectionsPerStream configuration. --- .../src/Internal/Http2/Http2Connection.cs | 73 ++++++++++++++----- 1 file changed, 55 insertions(+), 18 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 4ed606b32c95..4d05be558938 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -61,6 +61,10 @@ private enum PseudoHeaderFields private static readonly byte[] _trailersBytes = Encoding.ASCII.GetBytes("trailers"); private static readonly byte[] _connectBytes = Encoding.ASCII.GetBytes("CONNECT"); + // Since the number of streams per connection is user configurable, we need a max just in case that number is too big + // 200 seems reasonable since the default is 100 + private static readonly int _maxPooledStreams = 200; + private readonly HttpConnectionContext _context; private readonly Http2FrameWriter _frameWriter; private readonly HPackDecoder _hpackDecoder; @@ -71,6 +75,8 @@ private enum PseudoHeaderFields private readonly Http2PeerSettings _clientSettings = new Http2PeerSettings(); private readonly Http2Frame _incomingFrame = new Http2Frame(); + private readonly Http2Stream[] _streamPool; + private int _pooledStreamCount; private Http2Stream _currentHeadersStream; private RequestHeaderParsingState _requestHeaderParsingState; @@ -114,6 +120,9 @@ public Http2Connection(HttpConnectionContext context) _serverSettings.HeaderTableSize = (uint)http2Limits.HeaderTableSize; _serverSettings.MaxHeaderListSize = (uint)httpLimits.MaxRequestHeadersTotalSize; _serverSettings.InitialWindowSize = (uint)http2Limits.InitialStreamWindowSize; + + // Pool the set of streams on this connection + _streamPool = new Http2Stream[Math.Min(_maxPooledStreams, http2Limits.MaxStreamsPerConnection)]; } public string ConnectionId => _context.ConnectionId; @@ -194,6 +203,48 @@ public void StopProcessingNextRequest(bool sendGracefulGoAway = false) } } + private Http2Stream CreateStream(IHttpApplication application) + { + lock (_streamPool) + { + if (_pooledStreamCount > 0) + { + _pooledStreamCount--; + return _streamPool[_pooledStreamCount]; + } + } + + return new Http2Stream(application, new Http2StreamContext + { + ConnectionId = ConnectionId, + StreamId = _incomingFrame.StreamId, + ServiceContext = _context.ServiceContext, + ConnectionFeatures = _context.ConnectionFeatures, + MemoryPool = _context.MemoryPool, + LocalEndPoint = _context.LocalEndPoint, + RemoteEndPoint = _context.RemoteEndPoint, + StreamLifetimeHandler = this, + ClientPeerSettings = _clientSettings, + ServerPeerSettings = _serverSettings, + FrameWriter = _frameWriter, + ConnectionInputFlowControl = _inputFlowControl, + ConnectionOutputFlowControl = _outputFlowControl, + TimeoutControl = TimeoutControl, + }); + } + + private void ReturnStream(Http2Stream stream) + { + lock (_streamPool) + { + if (_pooledStreamCount < _streamPool.Length) + { + _streamPool[_pooledStreamCount] = stream; + _pooledStreamCount++; + } + } + } + public async Task ProcessRequestsAsync(IHttpApplication application) { Exception error = null; @@ -605,23 +656,7 @@ private Task ProcessHeadersFrameAsync(IHttpApplication appli } // Start a new stream - _currentHeadersStream = new Http2Stream(application, new Http2StreamContext - { - ConnectionId = ConnectionId, - StreamId = _incomingFrame.StreamId, - ServiceContext = _context.ServiceContext, - ConnectionFeatures = _context.ConnectionFeatures, - MemoryPool = _context.MemoryPool, - LocalEndPoint = _context.LocalEndPoint, - RemoteEndPoint = _context.RemoteEndPoint, - StreamLifetimeHandler = this, - ClientPeerSettings = _clientSettings, - ServerPeerSettings = _serverSettings, - FrameWriter = _frameWriter, - ConnectionInputFlowControl = _inputFlowControl, - ConnectionOutputFlowControl = _outputFlowControl, - TimeoutControl = TimeoutControl, - }); + _currentHeadersStream = CreateStream(application); _currentHeadersStream.Reset(); _headerFlags = _incomingFrame.HeadersFlags; @@ -1055,7 +1090,9 @@ void IHttp2StreamLifetimeHandler.OnStreamCompleted(int streamId) } else { - _streams.TryRemove(streamId, out _); + _streams.TryRemove(streamId, out stream); + + ReturnStream(stream); } } From 02d1b306c56c255a092ba39e4f05080f2967e0ba Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 27 Dec 2018 06:51:34 -0800 Subject: [PATCH 4/4] Properly set the state --- .../Core/src/Internal/Http/Http1Connection.cs | 3 ++- .../Kestrel/Core/src/Internal/Http/HttpProtocol.cs | 6 +++--- .../Core/src/Internal/Http2/Http2Connection.cs | 14 ++++++++++++-- .../Kestrel/Core/src/Internal/Http2/Http2Stream.cs | 13 +++++++------ .../Core/src/Internal/Http2/Http2StreamOfT.cs | 9 ++------- .../test/HttpProtocolFeatureCollectionTests.cs | 7 ++----- 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index 649560ce78d3..dca44693a026 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -37,8 +37,9 @@ public partial class Http1Connection : HttpProtocol, IRequestProcessor private int _remainingRequestHeadersBytesAllowed; public Http1Connection(HttpConnectionContext context) - : base(context) { + Initialize(context); + _context = context; _parser = ServiceContext.HttpParser; _keepAliveTicks = ServerOptions.Limits.KeepAliveTimeout.Ticks; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 72bedf1be684..adb1f2fc1862 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -62,12 +62,12 @@ public abstract partial class HttpProtocol : IHttpResponseControl private long _responseBytesWritten; - private readonly HttpConnectionContext _context; + private HttpConnectionContext _context; protected string _methodText = null; private string _scheme = null; - public HttpProtocol(HttpConnectionContext context) + protected void Initialize(HttpConnectionContext context) { _context = context; @@ -90,7 +90,7 @@ public HttpProtocol(HttpConnectionContext context) protected IKestrelTrace Log => ServiceContext.Log; private DateHeaderValueManager DateHeaderValueManager => ServiceContext.DateHeaderValueManager; // Hold direct reference to ServerOptions since this is used very often in the request processing path - protected KestrelServerOptions ServerOptions { get; } + protected KestrelServerOptions ServerOptions { get; set; } protected string ConnectionId => _context.ConnectionId; public string ConnectionIdFeature { get; set; } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 4d05be558938..95036a99740b 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -205,16 +205,24 @@ public void StopProcessingNextRequest(bool sendGracefulGoAway = false) private Http2Stream CreateStream(IHttpApplication application) { + Http2Stream stream = null; + lock (_streamPool) { if (_pooledStreamCount > 0) { _pooledStreamCount--; - return _streamPool[_pooledStreamCount]; + stream = (Http2Stream)_streamPool[_pooledStreamCount]; } } - return new Http2Stream(application, new Http2StreamContext + if (stream == null) + { + stream = new Http2Stream(); + } + + stream.HttpApplication = application; + stream.Initialize(new Http2StreamContext { ConnectionId = ConnectionId, StreamId = _incomingFrame.StreamId, @@ -231,6 +239,8 @@ private Http2Stream CreateStream(IHttpApplication applicatio ConnectionOutputFlowControl = _outputFlowControl, TimeoutControl = TimeoutControl, }); + + return stream; } private void ReturnStream(Http2Stream stream) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index 250f2c4cf88d..e74e33d237df 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -20,19 +20,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { public abstract partial class Http2Stream : HttpProtocol, IThreadPoolWorkItem { - private readonly Http2StreamContext _context; - private readonly Http2OutputProducer _http2Output; - private readonly StreamInputFlowControl _inputFlowControl; - private readonly StreamOutputFlowControl _outputFlowControl; + private Http2StreamContext _context; + private Http2OutputProducer _http2Output; + private StreamInputFlowControl _inputFlowControl; + private StreamOutputFlowControl _outputFlowControl; internal long DrainExpirationTicks { get; set; } private StreamCompletionFlags _completionState; private readonly object _completionLock = new object(); - public Http2Stream(Http2StreamContext context) - : base(context) + public virtual void Initialize(Http2StreamContext context) { + base.Initialize(context); + _context = context; _inputFlowControl = new StreamInputFlowControl( diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs index d026b3a28884..16be550fa974 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2StreamOfT.cs @@ -7,17 +7,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { public class Http2Stream : Http2Stream { - private readonly IHttpApplication _application; - - public Http2Stream(IHttpApplication application, Http2StreamContext context) : base(context) - { - _application = application; - } + public IHttpApplication HttpApplication { get; set; } public override void Execute() { // REVIEW: Should we store this in a field for easy debugging? - _ = ProcessRequestsAsync(_application); + _ = ProcessRequestsAsync(HttpApplication); } } } diff --git a/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs b/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs index 011fa59fe1a6..ded2eec7af16 100644 --- a/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs @@ -40,7 +40,8 @@ public HttpProtocolFeatureCollectionTests() _http1Connection.Reset(); _collection = _http1Connection; - var http2Stream = new TestHttp2Stream(context); + var http2Stream = new TestHttp2Stream(); + http2Stream.Initialize(context); http2Stream.Reset(); _http2Collection = http2Stream; } @@ -223,10 +224,6 @@ private int SetFeaturesToNonDefault() private class TestHttp2Stream : Http2Stream { - public TestHttp2Stream(Http2StreamContext context) : base(context) - { - } - public override void Execute() { }