From a57a0462bb1e23d2851b638757cb49278622b677 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
<41898282+github-actions[bot]@users.noreply.github.com>
Date: Wed, 8 Sep 2021 14:04:23 -0700
Subject: [PATCH] [release/6.0] Use ArrayPool as default pool fallback (#36253)
* Use ArrayPool as SlabMemoryPool fallback
Fixes #30364
* Fix comments
* Typos
[ci skip]
* Support increase buffer size
* Add function tests, rename argument
* Update src/Servers/Kestrel/Core/src/Internal/Http3/Http3OutputProducer.cs
Co-authored-by: Stephen Halter <halter73@gmail.com>
* Update src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs
Co-authored-by: Stephen Halter <halter73@gmail.com>
* Reduce allocations
Co-authored-by: Sebastien Ros <sebastienros@gmail.com>
Co-authored-by: Stephen Halter <halter73@gmail.com>
---
.../src/Internal/Http/Http1OutputProducer.cs | 50 ++++++++++-
.../src/Internal/Http2/Http2OutputProducer.cs | 49 ++++++++++-
.../src/Internal/Http3/Http3OutputProducer.cs | 49 ++++++++++-
.../PipeWriterHelpers/ConcurrentPipeWriter.cs | 8 +-
...erTests.cs => Http1OutputProducerTests.cs} | 87 ++++++++++++++++++-
.../Http2/Http2StreamTests.cs | 48 ++++++++++
.../Http3/Http3StreamTests.cs | 25 ++++++
7 files changed, 298 insertions(+), 18 deletions(-)
rename src/Servers/Kestrel/Core/test/{OutputProducerTests.cs => Http1OutputProducerTests.cs} (56%)
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs
index 1e5d98d6887..41abf1f6c53 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs
@@ -49,6 +49,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
private readonly ConcurrentPipeWriter _pipeWriter;
private IMemoryOwner<byte>? _fakeMemoryOwner;
+ private byte[]? _fakeMemory;
// Chunked responses need to be treated uniquely when using GetMemory + Advance.
// We need to know the size of the data written to the chunk before calling Advance on the
@@ -414,6 +415,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
_fakeMemoryOwner = null;
}
+ if (_fakeMemory != null)
+ {
+ ArrayPool<byte>.Shared.Return(_fakeMemory);
+ _fakeMemory = null;
+ }
+
// Call dispose on any memory that wasn't written.
if (_completedSegments != null)
{
@@ -650,13 +657,48 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
_advancedBytesForChunk = 0;
}
- private Memory<byte> GetFakeMemory(int sizeHint)
+ internal Memory<byte> GetFakeMemory(int minSize)
{
- if (_fakeMemoryOwner == null)
+ // Try to reuse _fakeMemoryOwner
+ if (_fakeMemoryOwner != null)
+ {
+ if (_fakeMemoryOwner.Memory.Length < minSize)
+ {
+ _fakeMemoryOwner.Dispose();
+ _fakeMemoryOwner = null;
+ }
+ else
+ {
+ return _fakeMemoryOwner.Memory;
+ }
+ }
+
+ // Try to reuse _fakeMemory
+ if (_fakeMemory != null)
+ {
+ if (_fakeMemory.Length < minSize)
+ {
+ ArrayPool<byte>.Shared.Return(_fakeMemory);
+ _fakeMemory = null;
+ }
+ else
+ {
+ return _fakeMemory;
+ }
+ }
+
+ // Requesting a bigger buffer could throw.
+ if (minSize <= _memoryPool.MaxBufferSize)
+ {
+ // Use the specified pool as it fits.
+ _fakeMemoryOwner = _memoryPool.Rent(minSize);
+ return _fakeMemoryOwner.Memory;
+ }
+ else
{
- _fakeMemoryOwner = _memoryPool.Rent(sizeHint);
+ // Use the array pool. Its MaxBufferSize is int.MaxValue.
+ return _fakeMemory = ArrayPool<byte>.Shared.Rent(minSize);
}
- return _fakeMemoryOwner.Memory;
}
private Memory<byte> LeasedMemory(int sizeHint)
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs
index 6a17db8934e..7be6cc65209 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs
@@ -36,6 +36,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
private readonly PipeReader _pipeReader;
private readonly ManualResetValueTaskSource<object?> _resetAwaitable = new ManualResetValueTaskSource<object?>();
private IMemoryOwner<byte>? _fakeMemoryOwner;
+ private byte[]? _fakeMemory;
private bool _startedWritingDataFrames;
private bool _streamCompleted;
private bool _suffixSent;
@@ -120,6 +121,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
_fakeMemoryOwner.Dispose();
_fakeMemoryOwner = null;
}
+
+ if (_fakeMemory != null)
+ {
+ ArrayPool<byte>.Shared.Return(_fakeMemory);
+ _fakeMemory = null;
+ }
}
}
@@ -486,14 +493,48 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
}
}
- private Memory<byte> GetFakeMemory(int sizeHint)
+ internal Memory<byte> GetFakeMemory(int minSize)
{
- if (_fakeMemoryOwner == null)
+ // Try to reuse _fakeMemoryOwner
+ if (_fakeMemoryOwner != null)
+ {
+ if (_fakeMemoryOwner.Memory.Length < minSize)
+ {
+ _fakeMemoryOwner.Dispose();
+ _fakeMemoryOwner = null;
+ }
+ else
+ {
+ return _fakeMemoryOwner.Memory;
+ }
+ }
+
+ // Try to reuse _fakeMemory
+ if (_fakeMemory != null)
{
- _fakeMemoryOwner = _memoryPool.Rent(sizeHint);
+ if (_fakeMemory.Length < minSize)
+ {
+ ArrayPool<byte>.Shared.Return(_fakeMemory);
+ _fakeMemory = null;
+ }
+ else
+ {
+ return _fakeMemory;
+ }
}
- return _fakeMemoryOwner.Memory;
+ // Requesting a bigger buffer could throw.
+ if (minSize <= _memoryPool.MaxBufferSize)
+ {
+ // Use the specified pool as it fits.
+ _fakeMemoryOwner = _memoryPool.Rent(minSize);
+ return _fakeMemoryOwner.Memory;
+ }
+ else
+ {
+ // Use the array pool. Its MaxBufferSize is int.MaxValue.
+ return _fakeMemory = ArrayPool<byte>.Shared.Rent(minSize);
+ }
}
[StackTraceHidden]
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3OutputProducer.cs
index 9bc96871f8a..52b05501688 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3OutputProducer.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3OutputProducer.cs
@@ -34,6 +34,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
private bool _disposed;
private bool _suffixSent;
private IMemoryOwner<byte>? _fakeMemoryOwner;
+ private byte[]? _fakeMemory;
public Http3OutputProducer(
Http3FrameWriter frameWriter,
@@ -88,6 +89,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
_fakeMemoryOwner.Dispose();
_fakeMemoryOwner = null;
}
+
+ if (_fakeMemory != null)
+ {
+ ArrayPool<byte>.Shared.Return(_fakeMemory);
+ _fakeMemory = null;
+ }
}
}
@@ -208,14 +215,48 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
}
}
- private Memory<byte> GetFakeMemory(int sizeHint)
+ internal Memory<byte> GetFakeMemory(int minSize)
{
- if (_fakeMemoryOwner == null)
+ // Try to reuse _fakeMemoryOwner
+ if (_fakeMemoryOwner != null)
+ {
+ if (_fakeMemoryOwner.Memory.Length < minSize)
+ {
+ _fakeMemoryOwner.Dispose();
+ _fakeMemoryOwner = null;
+ }
+ else
+ {
+ return _fakeMemoryOwner.Memory;
+ }
+ }
+
+ // Try to reuse _fakeMemory
+ if (_fakeMemory != null)
{
- _fakeMemoryOwner = _memoryPool.Rent(sizeHint);
+ if (_fakeMemory.Length < minSize)
+ {
+ ArrayPool<byte>.Shared.Return(_fakeMemory);
+ _fakeMemory = null;
+ }
+ else
+ {
+ return _fakeMemory;
+ }
}
- return _fakeMemoryOwner.Memory;
+ // Requesting a bigger buffer could throw.
+ if (minSize <= _memoryPool.MaxBufferSize)
+ {
+ // Use the specified pool as it fits.
+ _fakeMemoryOwner = _memoryPool.Rent(minSize);
+ return _fakeMemoryOwner.Memory;
+ }
+ else
+ {
+ // Use the array pool. Its MaxBufferSize is int.MaxValue.
+ return _fakeMemory = ArrayPool<byte>.Shared.Rent(minSize);
+ }
}
[StackTraceHidden]
diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PipeWriterHelpers/ConcurrentPipeWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PipeWriterHelpers/ConcurrentPipeWriter.cs
index c74e5f33975..ea652c1bc3e 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PipeWriterHelpers/ConcurrentPipeWriter.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/PipeWriterHelpers/ConcurrentPipeWriter.cs
@@ -346,19 +346,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure.PipeW
}
}
- private BufferSegment AllocateSegmentUnsynchronized(int sizeHint)
+ private BufferSegment AllocateSegmentUnsynchronized(int minSize)
{
BufferSegment newSegment = CreateSegmentUnsynchronized();
- if (sizeHint <= _pool.MaxBufferSize)
+ if (minSize <= _pool.MaxBufferSize)
{
// Use the specified pool if it fits
- newSegment.SetOwnedMemory(_pool.Rent(GetSegmentSize(sizeHint, _pool.MaxBufferSize)));
+ newSegment.SetOwnedMemory(_pool.Rent(GetSegmentSize(minSize, _pool.MaxBufferSize)));
}
else
{
// We can't use the recommended pool so use the ArrayPool
- newSegment.SetOwnedMemory(ArrayPool<byte>.Shared.Rent(sizeHint));
+ newSegment.SetOwnedMemory(ArrayPool<byte>.Shared.Rent(minSize));
}
_tailMemory = newSegment.AvailableMemory;
diff --git a/src/Servers/Kestrel/Core/test/OutputProducerTests.cs b/src/Servers/Kestrel/Core/test/Http1OutputProducerTests.cs
similarity index 56%
rename from src/Servers/Kestrel/Core/test/OutputProducerTests.cs
rename to src/Servers/Kestrel/Core/test/Http1OutputProducerTests.cs
index 3204cedd340..272fca47786 100644
--- a/src/Servers/Kestrel/Core/test/OutputProducerTests.cs
+++ b/src/Servers/Kestrel/Core/test/Http1OutputProducerTests.cs
@@ -15,11 +15,11 @@ using Xunit;
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
{
- public class OutputProducerTests : IDisposable
+ public class Http1OutputProducerTests : IDisposable
{
private readonly MemoryPool<byte> _memoryPool;
- public OutputProducerTests()
+ public Http1OutputProducerTests()
{
_memoryPool = PinnedBlockMemoryPoolFactory.Create();
}
@@ -74,6 +74,89 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
mockConnectionContext.Verify(f => f.Abort(null), Times.Once());
}
+ [Fact]
+ public void AllocatesFakeMemorySmallerThanMaxBufferSize()
+ {
+ var pipeOptions = new PipeOptions
+ (
+ pool: _memoryPool,
+ readerScheduler: Mock.Of<PipeScheduler>(),
+ writerScheduler: PipeScheduler.Inline,
+ useSynchronizationContext: false
+ );
+
+ using (var socketOutput = CreateOutputProducer(pipeOptions))
+ {
+ var bufferSize = 1;
+ var fakeMemory = socketOutput.GetFakeMemory(bufferSize);
+
+ Assert.True(fakeMemory.Length >= bufferSize);
+ }
+ }
+
+ [Fact]
+ public void AllocatesFakeMemoryBiggerThanMaxBufferSize()
+ {
+ var pipeOptions = new PipeOptions
+ (
+ pool: _memoryPool,
+ readerScheduler: Mock.Of<PipeScheduler>(),
+ writerScheduler: PipeScheduler.Inline,
+ useSynchronizationContext: false
+ );
+
+ using (var socketOutput = CreateOutputProducer(pipeOptions))
+ {
+ var bufferSize = _memoryPool.MaxBufferSize * 2;
+ var fakeMemory = socketOutput.GetFakeMemory(bufferSize);
+
+ Assert.True(fakeMemory.Length >= bufferSize);
+ }
+ }
+
+ [Fact]
+ public void AllocatesIncreasingFakeMemory()
+ {
+ var pipeOptions = new PipeOptions
+ (
+ pool: _memoryPool,
+ readerScheduler: Mock.Of<PipeScheduler>(),
+ writerScheduler: PipeScheduler.Inline,
+ useSynchronizationContext: false
+ );
+
+ using (var socketOutput = CreateOutputProducer(pipeOptions))
+ {
+ var bufferSize1 = 1024;
+ var bufferSize2 = 2048;
+ var fakeMemory = socketOutput.GetFakeMemory(bufferSize1);
+ fakeMemory = socketOutput.GetFakeMemory(bufferSize2);
+
+ Assert.True(fakeMemory.Length >= bufferSize2);
+ }
+ }
+
+ [Fact]
+ public void ReusesFakeMemory()
+ {
+ var pipeOptions = new PipeOptions
+ (
+ pool: _memoryPool,
+ readerScheduler: Mock.Of<PipeScheduler>(),
+ writerScheduler: PipeScheduler.Inline,
+ useSynchronizationContext: false
+ );
+
+ using (var socketOutput = CreateOutputProducer(pipeOptions))
+ {
+ var bufferSize = 1024;
+ var fakeMemory1 = socketOutput.GetFakeMemory(bufferSize);
+ var fakeMemory2 = socketOutput.GetFakeMemory(bufferSize);
+
+ Assert.Equal(fakeMemory1, fakeMemory2);
+ }
+ }
+
private TestHttpOutputProducer CreateOutputProducer(
PipeOptions pipeOptions = null,
ConnectionContext connectionContext = null)
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs
index 284440c1c3b..0fe7184e2e9 100644
--- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs
+++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs
@@ -5101,5 +5101,53 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
Assert.Contains(LogMessages, m => m.Message.Equals("One or more of the following response headers have been removed because they are invalid for HTTP/2 and HTTP/3 responses: 'Connection', 'Transfer-Encoding', 'Keep-Alive', 'Upgrade' and 'Proxy-Connection'."));
}
+
+ [Theory]
+ [InlineData(1000)]
+ [InlineData(4096)]
+ [InlineData(8000)] // Greater than the default max pool size (4096)
+ public async Task GetMemory_AfterAbort_GetsFakeMemory(int sizeHint)
+ {
+ var headers = new[]
+ {
+ new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
+ new KeyValuePair<string, string>(HeaderNames.Path, "/"),
+ new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+ };
+ await InitializeConnectionAsync(async httpContext =>
+ {
+ var response = httpContext.Response;
+
+ await response.BodyWriter.FlushAsync();
+
+ httpContext.Abort();
+
+ var memory = response.BodyWriter.GetMemory(sizeHint);
+ Assert.True(memory.Length >= sizeHint);
+
+ var fisrtPartOfResponse = Encoding.ASCII.GetBytes(new String('a', sizeHint));
+ fisrtPartOfResponse.CopyTo(memory);
+ response.BodyWriter.Advance(sizeHint);
+ });
+
+ await StartStreamAsync(1, headers, endStream: true);
+
+ var headersFrame = await ExpectAsync(Http2FrameType.HEADERS,
+ withLength: 32,
+ withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS,
+ withStreamId: 1);
+ await ExpectAsync(Http2FrameType.RST_STREAM,
+ withLength: 4,
+ withFlags: (byte)Http2DataFrameFlags.NONE,
+ withStreamId: 1);
+
+ await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
+
+ _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this);
+
+ Assert.Equal(2, _decodedHeaders.Count);
+ Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
+ Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
+ }
}
}
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs
index e6f66f75b76..6a82f44102c 100644
--- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs
+++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs
@@ -2927,5 +2927,30 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
var responseHeaders = await requestStream.ExpectHeadersAsync(expectEnd: true);
Assert.Equal("200", responseHeaders[HeaderNames.Status]);
}
+
+ [Theory]
+ [InlineData(1000)]
+ [InlineData(4096)]
+ [InlineData(8000)] // Greater than the default max pool size (4096)
+ public async Task GetMemory_AfterAbort_GetsFakeMemory(int sizeHint)
+ {
+ var tcs = new TaskCompletionSource();
+ var headers = new[]
+ {
+ new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
+ new KeyValuePair<string, string>(HeaderNames.Path, "/"),
+ new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+ };
+ var requestStream = await Http3Api.InitializeConnectionAndStreamsAsync(async context =>
+ {
+ context.Abort();
+
+ var memory = context.Response.BodyWriter.GetMemory(sizeHint);
+
+ Assert.True(memory.Length >= sizeHint);
+ await context.Response.CompleteAsync();
+ context.Response.BodyWriter.Advance(memory.Length);
+ });
+ }
}
}
--
GitLab