Skip to content

HTTP/3: Http3Stream pooling #34576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project>
<Project>
<Import Project="eng\Common.props" />

<PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ public Http1OutputProducer(
_minResponseDataRateFeature = minResponseDataRateFeature;
_outputAborter = outputAborter;

_flusher = new TimingPipeFlusher(_pipeWriter, timeoutControl, log);
_flusher = new TimingPipeFlusher(timeoutControl, log);
_flusher.Initialize(_pipeWriter);
}

public Task WriteDataAsync(ReadOnlySpan<byte> buffer, CancellationToken cancellationToken = default)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ public Http2FrameWriter(
_log = serviceContext.Log;
_timeoutControl = timeoutControl;
_minResponseDataRate = minResponseDataRate;
_flusher = new TimingPipeFlusher(_outputWriter, timeoutControl, serviceContext.Log);
_flusher = new TimingPipeFlusher(timeoutControl, serviceContext.Log);
_flusher.Initialize(_outputWriter);
_outgoingFrame = new Http2Frame();
_headerEncodingBuffer = new byte[_maxFrameSize];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ public Http2OutputProducer(Http2Stream stream, Http2StreamContext context, Strea

// No need to pass in timeoutControl here, since no minDataRates are passed to the TimingPipeFlusher.
// The minimum output data rate is enforced at the connection level by Http2FrameWriter.
_flusher = new TimingPipeFlusher(_pipeWriter, timeoutControl: null, _log);
_flusher = new TimingPipeFlusher(timeoutControl: null, _log);
_flusher.Initialize(_pipeWriter);

_dataWriteProcessingTask = ProcessDataWrites();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public Http2StreamContext(
Http2PeerSettings serverPeerSettings,
Http2FrameWriter frameWriter,
InputFlowControl connectionInputFlowControl,
OutputFlowControl connectionOutputFlowControl) : base(connectionId, protocols, connectionContext: null!, serviceContext, connectionFeatures, memoryPool, localEndPoint, remoteEndPoint, transport: null!)
OutputFlowControl connectionOutputFlowControl) : base(connectionId, protocols, connectionContext: null!, serviceContext, connectionFeatures, memoryPool, localEndPoint, remoteEndPoint)
{
StreamId = streamId;
StreamLifetimeHandler = streamLifetimeHandler;
Expand Down
62 changes: 43 additions & 19 deletions src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
{
internal class Http3Connection : IHttp3StreamLifetimeHandler, IRequestProcessor
{
private static readonly object StreamPersistentStateKey = new object();

// Internal for unit testing
internal readonly Dictionary<long, IHttp3Stream> _streams = new Dictionary<long, IHttp3Stream>();
internal IHttp3StreamLifetimeHandler _streamLifetimeHandler;
Expand Down Expand Up @@ -257,36 +259,37 @@ public async Task ProcessRequestsAsync<TContext>(IHttpApplication<TContext> appl
Debug.Assert(streamDirectionFeature != null);
Debug.Assert(streamIdFeature != null);

var httpConnectionContext = new Http3StreamContext(
streamContext.ConnectionId,
protocols: default,
connectionContext: null!, // TODO connection context is null here. Should we set it to anything?
_context.ServiceContext,
streamContext.Features,
_context.MemoryPool,
streamContext.LocalEndPoint as IPEndPoint,
streamContext.RemoteEndPoint as IPEndPoint,
streamContext.Transport,
_streamLifetimeHandler,
streamContext,
_clientSettings,
_serverSettings);
httpConnectionContext.TimeoutControl = _context.TimeoutControl;

if (!streamDirectionFeature.CanWrite)
{
// Unidirectional stream
var stream = new Http3ControlStream<TContext>(application, httpConnectionContext);
var stream = new Http3ControlStream<TContext>(application, CreateHttpStreamContext(streamContext));
_streamLifetimeHandler.OnStreamCreated(stream);

ThreadPool.UnsafeQueueUserWorkItem(stream, preferLocal: false);
}
else
{
var persistentStateFeature = streamContext.Features.Get<IPersistentStateFeature>();
Debug.Assert(persistentStateFeature != null, $"Required {nameof(IPersistentStateFeature)} not on stream context.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems too possible to just assert given the transport is pluggable. Let's null-check and throw.


// Request stream
UpdateHighestStreamId(streamIdFeature.StreamId);

var stream = new Http3Stream<TContext>(application, httpConnectionContext);
Http3Stream<TContext> stream;

// Check whether there is an existing HTTP/3 stream on the transport stream.
// A stream will only be cached if the transport stream itself is reused.
if (!persistentStateFeature.State.TryGetValue(StreamPersistentStateKey, out var s))
{
stream = new Http3Stream<TContext>(application, CreateHttpStreamContext(streamContext));
persistentStateFeature.State.Add(StreamPersistentStateKey, stream);
}
else
{
stream = (Http3Stream<TContext>)s!;
stream.InitializeWithExistingContext(streamContext.Transport);
}

_streamLifetimeHandler.OnStreamCreated(stream);

KestrelEventSource.Log.RequestQueuedStart(stream, AspNetCore.Http.HttpProtocol.Http3);
Expand Down Expand Up @@ -371,6 +374,27 @@ public async Task ProcessRequestsAsync<TContext>(IHttpApplication<TContext> appl
}
}

private Http3StreamContext CreateHttpStreamContext(ConnectionContext streamContext)
{
var httpConnectionContext = new Http3StreamContext(
streamContext.ConnectionId,
protocols: default,
connectionContext: null!, // TODO connection context is null here. Should we set it to anything?
_context.ServiceContext,
streamContext.Features,
_context.MemoryPool,
streamContext.LocalEndPoint as IPEndPoint,
streamContext.RemoteEndPoint as IPEndPoint,
_streamLifetimeHandler,
streamContext,
_clientSettings,
_serverSettings);
httpConnectionContext.TimeoutControl = _context.TimeoutControl;
httpConnectionContext.Transport = streamContext.Transport;

return httpConnectionContext;
}

private void UpdateConnectionState()
{
if (_isClosed != 0)
Expand Down Expand Up @@ -443,12 +467,12 @@ private async ValueTask<Http3ControlStream> CreateNewUnidirectionalStreamAsync<T
_context.MemoryPool,
streamContext.LocalEndPoint as IPEndPoint,
streamContext.RemoteEndPoint as IPEndPoint,
streamContext.Transport,
_streamLifetimeHandler,
streamContext,
_clientSettings,
_serverSettings);
httpConnectionContext.TimeoutControl = _context.TimeoutControl;
httpConnectionContext.Transport = streamContext.Transport;

return new Http3ControlStream<TContext>(application, httpConnectionContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,15 @@ public Http3ControlStream(Http3StreamContext context)
_headerType = -1;

_frameWriter = new Http3FrameWriter(
context.Transport.Output,
context.StreamContext,
context.TimeoutControl,
httpLimits.MinResponseDataRate,
context.ConnectionId,
context.MemoryPool,
context.ServiceContext.Log,
_streamIdFeature,
context.ClientPeerSettings,
this);
_frameWriter.Reset(context.Transport.Output, context.ConnectionId);
}

private void OnStreamClosed()
Expand Down
32 changes: 22 additions & 10 deletions src/Servers/Kestrel/Core/src/Internal/Http3/Http3FrameWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,19 @@ internal class Http3FrameWriter
private readonly object _writeLock = new object();

private readonly int _maxTotalHeaderSize;
private readonly PipeWriter _outputWriter;
private readonly ConnectionContext _connectionContext;
private readonly ITimeoutControl _timeoutControl;
private readonly MinDataRate? _minResponseDataRate;
private readonly string _connectionId;
private readonly MemoryPool<byte> _memoryPool;
private readonly IKestrelTrace _log;
private readonly IStreamIdFeature _streamIdFeature;
private readonly IHttp3Stream _http3Stream;
private readonly Http3RawFrame _outgoingFrame;
private readonly TimingPipeFlusher _flusher;

private PipeWriter _outputWriter = default!;
private string _connectionId = default!;

// HTTP/3 doesn't have a max frame size (peer can optionally specify a size).
// Write headers to a buffer that can grow. Possible performance improvement
// by writing directly to output writer (difficult as frame length is prefixed).
Expand All @@ -52,19 +53,17 @@ internal class Http3FrameWriter
private bool _completed;
private bool _aborted;

public Http3FrameWriter(PipeWriter output, ConnectionContext connectionContext, ITimeoutControl timeoutControl, MinDataRate? minResponseDataRate, string connectionId, MemoryPool<byte> memoryPool, IKestrelTrace log, IStreamIdFeature streamIdFeature, Http3PeerSettings clientPeerSettings, IHttp3Stream http3Stream)
public Http3FrameWriter(ConnectionContext connectionContext, ITimeoutControl timeoutControl, MinDataRate? minResponseDataRate, MemoryPool<byte> memoryPool, IKestrelTrace log, IStreamIdFeature streamIdFeature, Http3PeerSettings clientPeerSettings, IHttp3Stream http3Stream)
{
_outputWriter = output;
_connectionContext = connectionContext;
_timeoutControl = timeoutControl;
_minResponseDataRate = minResponseDataRate;
_connectionId = connectionId;
_memoryPool = memoryPool;
_log = log;
_streamIdFeature = streamIdFeature;
_http3Stream = http3Stream;
_outgoingFrame = new Http3RawFrame();
_flusher = new TimingPipeFlusher(_outputWriter, timeoutControl, log);
_flusher = new TimingPipeFlusher(timeoutControl, log);
_headerEncodingBuffer = new ArrayBufferWriter<byte>(HeaderBufferSize);

// Note that max total header size value doesn't react to settings change during a stream.
Expand All @@ -76,6 +75,19 @@ public Http3FrameWriter(PipeWriter output, ConnectionContext connectionContext,
: (int)clientPeerSettings.MaxRequestHeaderFieldSectionSize;
}

public void Reset(PipeWriter output, string connectionId)
{
_outputWriter = output;
_flusher.Initialize(output);
_connectionId = connectionId;

_headersTotalSize = 0;
_headerEncodingBuffer.Clear();
_unflushedBytes = 0;
_completed = false;
_aborted = false;
}

internal Task WriteSettingsAsync(List<Http3PeerSetting> settings)
{
_outgoingFrame.PrepareSettings();
Expand Down Expand Up @@ -238,22 +250,22 @@ internal ValueTask<FlushResult> WriteGoAway(long id)
private void WriteHeaderUnsynchronized()
{
_log.Http3FrameSending(_connectionId, _streamIdFeature.StreamId, _outgoingFrame);
var headerLength = WriteHeader(_outgoingFrame, _outputWriter);
var headerLength = WriteHeader(_outgoingFrame.Type, _outgoingFrame.Length, _outputWriter);

// We assume the payload will be written prior to the next flush.
_unflushedBytes += headerLength + _outgoingFrame.Length;
}

internal static int WriteHeader(Http3RawFrame frame, PipeWriter output)
internal static int WriteHeader(Http3FrameType frameType, long frameLength, PipeWriter output)
{
// max size of the header is 16, most likely it will be smaller.
var buffer = output.GetSpan(16);

var typeLength = VariableLengthIntegerHelper.WriteInteger(buffer, (int)frame.Type);
var typeLength = VariableLengthIntegerHelper.WriteInteger(buffer, (int)frameType);

buffer = buffer.Slice(typeLength);

var lengthLength = VariableLengthIntegerHelper.WriteInteger(buffer, (int)frame.Length);
var lengthLength = VariableLengthIntegerHelper.WriteInteger(buffer, (int)frameLength);

var totalLength = typeLength + lengthLength;
output.Advance(typeLength + lengthLength);
Expand Down
Loading