From e727101957754556375dd93c13404e744241e8c5 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 13 Jun 2019 20:03:29 -0700 Subject: [PATCH] Work around potential race in PipeWriter Redux (#11065) - Make sure we always await the last flush task before calling FlushAsync again instead of preemptively calling FlushAsync and checking to see if the ValueTask is incomplete before bothering to acquire the _flushLock - This now acquires the _flushLock fore every call to Response.Body.Write whereas this only happened for truly async writes before. - I don't think this is a big concern since this should normally be uncontested, and DefaultPipeWriter.FlushAsync/GetResult already acquire a lock. --- .../Infrastructure/TimingPipeFlusher.cs | 58 ++++++--- .../Kestrel/Core/test/Http1ConnectionTests.cs | 2 +- .../Core/test/HttpResponseHeadersTests.cs | 2 +- .../Kestrel/Core/test/OutputProducerTests.cs | 2 +- .../Core/test/PipelineExtensionTests.cs | 2 +- .../Kestrel/Core/test/StartLineTests.cs | 2 +- .../Core/test/TestHelpers/TestInput.cs | 2 +- .../Core/test/TimingPipeFlusherTests.cs | 67 ++++++++++ .../src/LibuvTransportOptions.cs | 2 +- .../test/LibuvOutputConsumerTests.cs | 34 ++--- .../src/SocketTransportOptions.cs | 2 +- .../ChunkWriterBenchmark.cs | 2 +- .../Http1ConnectionBenchmark.cs | 2 +- ...Http1ConnectionParsingOverheadBenchmark.cs | 2 +- .../Http1LargeWritingBenchmark.cs | 117 ++++++++++++++++++ .../Http1ReadingBenchmark.cs | 2 +- .../Http1WritingBenchmark.cs | 16 ++- .../HttpProtocolFeatureCollection.cs | 2 +- .../PipeThroughputBenchmark.cs | 2 +- .../RequestParsingBenchmark.cs | 2 +- .../ResponseHeaderCollectionBenchmark.cs | 2 +- .../Kestrel/shared/test/TestServiceContext.cs | 2 +- .../Http2/Http2TestBase.cs | 2 +- .../Buffers.MemoryPool/MemoryPoolFactory.cs | 2 +- 24 files changed, 270 insertions(+), 60 deletions(-) create mode 100644 src/Servers/Kestrel/Core/test/TimingPipeFlusherTests.cs create mode 100644 src/Servers/Kestrel/perf/Kestrel.Performance/Http1LargeWritingBenchmark.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TimingPipeFlusher.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TimingPipeFlusher.cs index e9c66b40b6..22568e0bd0 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TimingPipeFlusher.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TimingPipeFlusher.cs @@ -22,7 +22,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure private readonly IKestrelTrace _log; private readonly object _flushLock = new object(); - private Task _lastFlushTask = null; + + // This field should only be get or set under the _flushLock. This is a ValueTask that was either: + // 1. The default value where "IsCompleted" is true + // 2. Created by an async method + // 3. Constructed explicitely from a completed result + // This means it should be safe to await a single _lastFlushTask instance multiple times. + private ValueTask _lastFlushTask; public TimingPipeFlusher( PipeWriter writer, @@ -51,35 +57,41 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure public ValueTask FlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) { - var flushValueTask = _writer.FlushAsync(cancellationToken); + // https://github.com/dotnet/corefxlab/issues/1334 + // Pipelines don't support multiple awaiters on flush. + lock (_flushLock) + { + if (_lastFlushTask.IsCompleted) + { + _lastFlushTask = TimeFlushAsync(minRate, count, outputAborter, cancellationToken); + } + else + { + _lastFlushTask = AwaitLastFlushAndTimeFlushAsync(_lastFlushTask, minRate, count, outputAborter, cancellationToken); + } + + return _lastFlushTask; + } + } + + private ValueTask TimeFlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) + { + var pipeFlushTask = _writer.FlushAsync(cancellationToken); if (minRate != null) { _timeoutControl.BytesWrittenToBuffer(minRate, count); } - if (flushValueTask.IsCompletedSuccessfully) + if (pipeFlushTask.IsCompletedSuccessfully) { - return new ValueTask(flushValueTask.Result); + return new ValueTask(pipeFlushTask.Result); } - // https://github.com/dotnet/corefxlab/issues/1334 - // Pipelines don't support multiple awaiters on flush. - // While it's acceptable to call PipeWriter.FlushAsync again before the last FlushAsync completes, - // it is not acceptable to attach a new continuation (via await, AsTask(), etc..). In this case, - // we find previous flush Task which still accounts for any newly committed bytes and await that. - lock (_flushLock) - { - if (_lastFlushTask == null || _lastFlushTask.IsCompleted) - { - _lastFlushTask = flushValueTask.AsTask(); - } - - return TimeFlushAsync(minRate, count, outputAborter, cancellationToken); - } + return TimeFlushAsyncAwaited(pipeFlushTask, minRate, count, outputAborter, cancellationToken); } - private async ValueTask TimeFlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) + private async ValueTask TimeFlushAsyncAwaited(ValueTask pipeFlushTask, MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) { if (minRate != null) { @@ -88,7 +100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure try { - return await _lastFlushTask; + return await pipeFlushTask; } catch (OperationCanceledException ex) when (outputAborter != null) { @@ -111,5 +123,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure return default; } + + private async ValueTask AwaitLastFlushAndTimeFlushAsync(ValueTask lastFlushTask, MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) + { + await lastFlushTask; + return await TimeFlushAsync(minRate, count, outputAborter, cancellationToken); + } } } diff --git a/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs b/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs index 042a136b3e..f602d09798 100644 --- a/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs +++ b/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs @@ -42,7 +42,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public Http1ConnectionTests() { - _pipelineFactory = MemoryPoolFactory.Create(); + _pipelineFactory = SlabMemoryPoolFactory.Create(); var options = new PipeOptions(_pipelineFactory, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); diff --git a/src/Servers/Kestrel/Core/test/HttpResponseHeadersTests.cs b/src/Servers/Kestrel/Core/test/HttpResponseHeadersTests.cs index 6a4e1c2352..ed3c492e3c 100644 --- a/src/Servers/Kestrel/Core/test/HttpResponseHeadersTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpResponseHeadersTests.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [Fact] public void InitialDictionaryIsEmpty() { - using (var memoryPool = MemoryPoolFactory.Create()) + using (var memoryPool = SlabMemoryPoolFactory.Create()) { var options = new PipeOptions(memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); diff --git a/src/Servers/Kestrel/Core/test/OutputProducerTests.cs b/src/Servers/Kestrel/Core/test/OutputProducerTests.cs index df162d8322..ca7c20e7d6 100644 --- a/src/Servers/Kestrel/Core/test/OutputProducerTests.cs +++ b/src/Servers/Kestrel/Core/test/OutputProducerTests.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public OutputProducerTests() { - _memoryPool = MemoryPoolFactory.Create(); + _memoryPool = SlabMemoryPoolFactory.Create(); } public void Dispose() diff --git a/src/Servers/Kestrel/Core/test/PipelineExtensionTests.cs b/src/Servers/Kestrel/Core/test/PipelineExtensionTests.cs index e2dfc2efd4..ebf57aebeb 100644 --- a/src/Servers/Kestrel/Core/test/PipelineExtensionTests.cs +++ b/src/Servers/Kestrel/Core/test/PipelineExtensionTests.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests private const int _ulongMaxValueLength = 20; private readonly Pipe _pipe; - private readonly MemoryPool _memoryPool = MemoryPoolFactory.Create(); + private readonly MemoryPool _memoryPool = SlabMemoryPoolFactory.Create(); public PipelineExtensionTests() { diff --git a/src/Servers/Kestrel/Core/test/StartLineTests.cs b/src/Servers/Kestrel/Core/test/StartLineTests.cs index bd4d5f4d5b..c53e9f3980 100644 --- a/src/Servers/Kestrel/Core/test/StartLineTests.cs +++ b/src/Servers/Kestrel/Core/test/StartLineTests.cs @@ -499,7 +499,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public StartLineTests() { - MemoryPool = MemoryPoolFactory.Create(); + MemoryPool = SlabMemoryPoolFactory.Create(); var options = new PipeOptions(MemoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); Transport = pair.Transport; diff --git a/src/Servers/Kestrel/Core/test/TestHelpers/TestInput.cs b/src/Servers/Kestrel/Core/test/TestHelpers/TestInput.cs index 3a8ce70e67..d77c68a163 100644 --- a/src/Servers/Kestrel/Core/test/TestHelpers/TestInput.cs +++ b/src/Servers/Kestrel/Core/test/TestHelpers/TestInput.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public TestInput() { - _memoryPool = MemoryPoolFactory.Create(); + _memoryPool = SlabMemoryPoolFactory.Create(); var options = new PipeOptions(pool: _memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); Transport = pair.Transport; diff --git a/src/Servers/Kestrel/Core/test/TimingPipeFlusherTests.cs b/src/Servers/Kestrel/Core/test/TimingPipeFlusherTests.cs new file mode 100644 index 0000000000..400d390039 --- /dev/null +++ b/src/Servers/Kestrel/Core/test/TimingPipeFlusherTests.cs @@ -0,0 +1,67 @@ +// 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.IO.Pipelines; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests +{ + public class TimingPipeFlusherTests + { + [Fact] + public async Task IfFlushIsCalledAgainBeforeTheLastFlushCompletedItWaitsForTheLastCall() + { + var mockPipeWriter = new Mock(); + var pipeWriterFlushTcsArray = new[] { + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously), + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously), + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously), + }; + var pipeWriterFlushCallCount = 0; + + mockPipeWriter.Setup(p => p.FlushAsync(CancellationToken.None)).Returns(() => + { + return new ValueTask(pipeWriterFlushTcsArray[pipeWriterFlushCallCount++].Task); + }); + + var timingPipeFlusher = new TimingPipeFlusher(mockPipeWriter.Object, null, null); + + var flushTask0 = timingPipeFlusher.FlushAsync(); + var flushTask1 = timingPipeFlusher.FlushAsync(); + var flushTask2 = timingPipeFlusher.FlushAsync(); + + Assert.False(flushTask0.IsCompleted); + Assert.False(flushTask1.IsCompleted); + Assert.False(flushTask2.IsCompleted); + Assert.Equal(1, pipeWriterFlushCallCount); + + pipeWriterFlushTcsArray[0].SetResult(default); + await flushTask0.AsTask().DefaultTimeout(); + + Assert.True(flushTask0.IsCompleted); + Assert.False(flushTask1.IsCompleted); + Assert.False(flushTask2.IsCompleted); + Assert.True(pipeWriterFlushCallCount <= 2); + + pipeWriterFlushTcsArray[1].SetResult(default); + await flushTask1.AsTask().DefaultTimeout(); + + Assert.True(flushTask0.IsCompleted); + Assert.True(flushTask1.IsCompleted); + Assert.False(flushTask2.IsCompleted); + Assert.True(pipeWriterFlushCallCount <= 3); + + pipeWriterFlushTcsArray[2].SetResult(default); + await flushTask2.AsTask().DefaultTimeout(); + + Assert.True(flushTask0.IsCompleted); + Assert.True(flushTask1.IsCompleted); + Assert.True(flushTask2.IsCompleted); + Assert.Equal(3, pipeWriterFlushCallCount); + } + } +} diff --git a/src/Servers/Kestrel/Transport.Libuv/src/LibuvTransportOptions.cs b/src/Servers/Kestrel/Transport.Libuv/src/LibuvTransportOptions.cs index 4ba48aa9ad..06c8aec796 100644 --- a/src/Servers/Kestrel/Transport.Libuv/src/LibuvTransportOptions.cs +++ b/src/Servers/Kestrel/Transport.Libuv/src/LibuvTransportOptions.cs @@ -31,7 +31,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv public long? MaxWriteBufferSize { get; set; } = 64 * 1024; - internal Func> MemoryPoolFactory { get; set; } = System.Buffers.MemoryPoolFactory.Create; + internal Func> MemoryPoolFactory { get; set; } = System.Buffers.SlabMemoryPoolFactory.Create; private static int ProcessorThreadCount { diff --git a/src/Servers/Kestrel/Transport.Libuv/test/LibuvOutputConsumerTests.cs b/src/Servers/Kestrel/Transport.Libuv/test/LibuvOutputConsumerTests.cs index 5647ce0dcf..c295258b2a 100644 --- a/src/Servers/Kestrel/Transport.Libuv/test/LibuvOutputConsumerTests.cs +++ b/src/Servers/Kestrel/Transport.Libuv/test/LibuvOutputConsumerTests.cs @@ -41,7 +41,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests public LibuvOutputConsumerTests() { - _memoryPool = MemoryPoolFactory.Create(); + _memoryPool = SlabMemoryPoolFactory.Create(); _mockLibuv = new MockLibuv(); var context = new TestLibuvTransportContext(); @@ -560,15 +560,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests Assert.False(task1Waits.IsFaulted); // following tasks should wait. - var task3Canceled = outputProducer.WriteDataAsync(fullBuffer, cancellationToken: abortedSource.Token); + var task2Canceled = outputProducer.WriteDataAsync(fullBuffer, cancellationToken: abortedSource.Token); // Give time for tasks to percolate await _mockLibuv.OnPostTask; - // Third task is not completed - Assert.False(task3Canceled.IsCompleted); - Assert.False(task3Canceled.IsCanceled); - Assert.False(task3Canceled.IsFaulted); + // Second task is not completed + Assert.False(task2Canceled.IsCompleted); + Assert.False(task2Canceled.IsCanceled); + Assert.False(task2Canceled.IsFaulted); abortedSource.Cancel(); @@ -578,29 +578,29 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests await _libuvThread.PostAsync(cb => cb(0), triggerNextCompleted); } - // First task is completed + // First task is completed Assert.True(task1Waits.IsCompleted); Assert.False(task1Waits.IsCanceled); Assert.False(task1Waits.IsFaulted); - // A final write guarantees that the error is observed by OutputProducer, - // but doesn't return a canceled/faulted task. - var task4Success = outputProducer.WriteDataAsync(fullBuffer); - Assert.True(task4Success.IsCompleted); - Assert.False(task4Success.IsCanceled); - Assert.False(task4Success.IsFaulted); + // Second task is now canceled + await Assert.ThrowsAsync(() => task2Canceled); + Assert.True(task2Canceled.IsCanceled); - // Third task is now canceled - await Assert.ThrowsAsync(() => task3Canceled); - Assert.True(task3Canceled.IsCanceled); + // A final write can still succeed. + var task3Success = outputProducer.WriteDataAsync(fullBuffer); await _mockLibuv.OnPostTask; - // Complete the 4th write + // Complete the 3rd write while (completeQueue.TryDequeue(out var triggerNextCompleted)) { await _libuvThread.PostAsync(cb => cb(0), triggerNextCompleted); } + + Assert.True(task3Success.IsCompleted); + Assert.False(task3Success.IsCanceled); + Assert.False(task3Success.IsFaulted);; } }); } diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs index 7bd8e6e937..4adb16ebfb 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs @@ -28,6 +28,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets public long? MaxWriteBufferSize { get; set; } = 64 * 1024; - internal Func> MemoryPoolFactory { get; set; } = System.Buffers.MemoryPoolFactory.Create; + internal Func> MemoryPoolFactory { get; set; } = System.Buffers.SlabMemoryPoolFactory.Create; } } diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/ChunkWriterBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/ChunkWriterBenchmark.cs index c327d8d51f..c1aeaf8efa 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/ChunkWriterBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/ChunkWriterBenchmark.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance [GlobalSetup] public void Setup() { - _memoryPool = MemoryPoolFactory.Create(); + _memoryPool = SlabMemoryPoolFactory.Create(); var pipe = new Pipe(new PipeOptions(_memoryPool)); _reader = pipe.Reader; _writer = pipe.Writer; diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ConnectionBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ConnectionBenchmark.cs index 290c63f2f2..887aad3939 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ConnectionBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ConnectionBenchmark.cs @@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance [GlobalSetup] public void Setup() { - var memoryPool = MemoryPoolFactory.Create(); + var memoryPool = SlabMemoryPoolFactory.Create(); var options = new PipeOptions(memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs index 8ffe0a0059..bb103df34e 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance [IterationSetup] public void Setup() { - var memoryPool = MemoryPoolFactory.Create(); + var memoryPool = SlabMemoryPoolFactory.Create(); var options = new PipeOptions(memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/Http1LargeWritingBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1LargeWritingBenchmark.cs new file mode 100644 index 0000000000..165b4369a7 --- /dev/null +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1LargeWritingBenchmark.cs @@ -0,0 +1,117 @@ +// 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; +using System.Buffers; +using System.IO.Pipelines; +using System.Threading.Tasks; +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.AspNetCore.Testing; + +namespace Microsoft.AspNetCore.Server.Kestrel.Performance +{ + public class Http1LargeWritingBenchmark + { + private TestHttp1Connection _http1Connection; + private DuplexPipe.DuplexPipePair _pair; + private MemoryPool _memoryPool; + private Task _consumeResponseBodyTask; + + // Keep this divisable by 10 so it can be evenly segmented. + private readonly byte[] _writeData = new byte[10 * 1024 * 1024]; + + [GlobalSetup] + public void GlobalSetup() + { + _memoryPool = SlabMemoryPoolFactory.Create(); + _http1Connection = MakeHttp1Connection(); + _consumeResponseBodyTask = ConsumeResponseBody(); + } + + [IterationSetup] + public void Setup() + { + _http1Connection.Reset(); + _http1Connection.RequestHeaders.ContentLength = _writeData.Length; + _http1Connection.FlushAsync().GetAwaiter().GetResult(); + } + + [Benchmark] + public Task WriteAsync() + { + return _http1Connection.ResponseBody.WriteAsync(_writeData, 0, _writeData.Length, default); + } + + [Benchmark] + public Task WriteSegmentsUnawaitedAsync() + { + // Write a 10th the of the data at a time + var segmentSize = _writeData.Length / 10; + + for (int i = 0; i < 9; i++) + { + // Ignore the first nine tasks. + _ = _http1Connection.ResponseBody.WriteAsync(_writeData, i * segmentSize, segmentSize, default); + } + + return _http1Connection.ResponseBody.WriteAsync(_writeData, 9 * segmentSize, segmentSize, default); + } + + private TestHttp1Connection MakeHttp1Connection() + { + var options = new PipeOptions(_memoryPool, useSynchronizationContext: false); + var pair = DuplexPipe.CreateConnectionPair(options, options); + _pair = pair; + + var serviceContext = new ServiceContext + { + DateHeaderValueManager = new DateHeaderValueManager(), + ServerOptions = new KestrelServerOptions(), + Log = new MockTrace(), + HttpParser = new HttpParser() + }; + + var http1Connection = new TestHttp1Connection(new HttpConnectionContext + { + ServiceContext = serviceContext, + ConnectionFeatures = new FeatureCollection(), + MemoryPool = _memoryPool, + TimeoutControl = new TimeoutControl(timeoutHandler: null), + Transport = pair.Transport + }); + + http1Connection.Reset(); + http1Connection.InitializeBodyControl(MessageBody.ZeroContentLengthKeepAlive); + serviceContext.DateHeaderValueManager.OnHeartbeat(DateTimeOffset.UtcNow); + + return http1Connection; + } + + private async Task ConsumeResponseBody() + { + var reader = _pair.Application.Input; + var readResult = await reader.ReadAsync(); + + while (!readResult.IsCompleted) + { + reader.AdvanceTo(readResult.Buffer.End); + readResult = await reader.ReadAsync(); + } + + reader.Complete(); + } + + [GlobalCleanup] + public void Dispose() + { + _pair.Transport.Output.Complete(); + _consumeResponseBodyTask.GetAwaiter().GetResult(); + _memoryPool?.Dispose(); + } + } +} diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ReadingBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ReadingBenchmark.cs index 2faca73d6a..6fb788115a 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ReadingBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1ReadingBenchmark.cs @@ -34,7 +34,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance [GlobalSetup] public void GlobalSetup() { - _memoryPool = MemoryPoolFactory.Create(); + _memoryPool = SlabMemoryPoolFactory.Create(); _http1Connection = MakeHttp1Connection(); } diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/Http1WritingBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1WritingBenchmark.cs index 94dc4e66c5..2a3585bc7c 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/Http1WritingBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/Http1WritingBenchmark.cs @@ -28,14 +28,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance private TestHttp1Connection _http1Connection; private DuplexPipe.DuplexPipePair _pair; private MemoryPool _memoryPool; + private Task _consumeResponseBodyTask; private readonly byte[] _writeData = Encoding.ASCII.GetBytes("Hello, World!"); [GlobalSetup] public void GlobalSetup() { - _memoryPool = MemoryPoolFactory.Create(); + _memoryPool = SlabMemoryPoolFactory.Create(); _http1Connection = MakeHttp1Connection(); + _consumeResponseBodyTask = ConsumeResponseBody(); } [Params(true, false)] @@ -124,14 +126,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance return http1Connection; } - [IterationCleanup] - public void Cleanup() + private async Task ConsumeResponseBody() { var reader = _pair.Application.Input; - if (reader.TryRead(out var readResult)) + var readResult = await reader.ReadAsync(); + + while (!readResult.IsCompleted) { reader.AdvanceTo(readResult.Buffer.End); + readResult = await reader.ReadAsync(); } + + reader.Complete(); } public enum Startup @@ -144,6 +150,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance [GlobalCleanup] public void Dispose() { + _pair.Transport.Output.Complete(); + _consumeResponseBodyTask.GetAwaiter().GetResult(); _memoryPool?.Dispose(); } } diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/HttpProtocolFeatureCollection.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/HttpProtocolFeatureCollection.cs index 7f948c5a4b..2db6ce7100 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/HttpProtocolFeatureCollection.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/HttpProtocolFeatureCollection.cs @@ -78,7 +78,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public HttpProtocolFeatureCollection() { - var memoryPool = MemoryPoolFactory.Create(); + var memoryPool = SlabMemoryPoolFactory.Create(); var options = new PipeOptions(memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/PipeThroughputBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/PipeThroughputBenchmark.cs index 03b3eb2f1d..c6b0da1cd0 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/PipeThroughputBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/PipeThroughputBenchmark.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance [IterationSetup] public void Setup() { - _memoryPool = MemoryPoolFactory.Create(); + _memoryPool = SlabMemoryPoolFactory.Create(); _pipe = new Pipe(new PipeOptions(_memoryPool)); } diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/RequestParsingBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/RequestParsingBenchmark.cs index a6cba51178..9e0f842fc1 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/RequestParsingBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/RequestParsingBenchmark.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance [IterationSetup] public void Setup() { - _memoryPool = MemoryPoolFactory.Create(); + _memoryPool = SlabMemoryPoolFactory.Create(); var options = new PipeOptions(_memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs index 417c5a8bbe..8430ed5900 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs @@ -172,7 +172,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance [IterationSetup] public void Setup() { - var memoryPool = MemoryPoolFactory.Create(); + var memoryPool = SlabMemoryPoolFactory.Create(); var options = new PipeOptions(memoryPool, readerScheduler: PipeScheduler.Inline, writerScheduler: PipeScheduler.Inline, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(options, options); diff --git a/src/Servers/Kestrel/shared/test/TestServiceContext.cs b/src/Servers/Kestrel/shared/test/TestServiceContext.cs index e5332963aa..9732bf1c8f 100644 --- a/src/Servers/Kestrel/shared/test/TestServiceContext.cs +++ b/src/Servers/Kestrel/shared/test/TestServiceContext.cs @@ -74,7 +74,7 @@ namespace Microsoft.AspNetCore.Testing public MockSystemClock MockSystemClock { get; set; } - public Func> MemoryPoolFactory { get; set; } = System.Buffers.MemoryPoolFactory.Create; + public Func> MemoryPoolFactory { get; set; } = System.Buffers.SlabMemoryPoolFactory.Create; public int ExpectedConnectionMiddlewareCount { get; set; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 424307ac26..7d09560508 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -114,7 +114,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests protected static readonly byte[] _noData = new byte[0]; protected static readonly byte[] _maxData = Encoding.ASCII.GetBytes(new string('a', Http2PeerSettings.MinAllowedMaxFrameSize)); - private readonly MemoryPool _memoryPool = MemoryPoolFactory.Create(); + private readonly MemoryPool _memoryPool = SlabMemoryPoolFactory.Create(); internal readonly Http2PeerSettings _clientSettings = new Http2PeerSettings(); internal readonly HPackEncoder _hpackEncoder = new HPackEncoder(); diff --git a/src/Shared/Buffers.MemoryPool/MemoryPoolFactory.cs b/src/Shared/Buffers.MemoryPool/MemoryPoolFactory.cs index c7ee26ca25..655b640c45 100644 --- a/src/Shared/Buffers.MemoryPool/MemoryPoolFactory.cs +++ b/src/Shared/Buffers.MemoryPool/MemoryPoolFactory.cs @@ -3,7 +3,7 @@ namespace System.Buffers { - internal static class MemoryPoolFactory + internal static class SlabMemoryPoolFactory { public static MemoryPool Create() {