From 5dc84550a6e2cf11198bd1c0b21ef76545ca9253 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 30 Jan 2019 12:23:14 -0800 Subject: [PATCH 1/2] Make GetMemory use MaxBufferSize for MemoryPool --- src/Http/Http/src/StreamPipeWriter.cs | 3 ++- src/Http/Http/test/PipeWriterTests.cs | 6 ++---- src/Http/Http/test/StreamPipeWriterTests.cs | 1 + src/Http/Http/test/TestMemoryPool.cs | 8 +++++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Http/Http/src/StreamPipeWriter.cs b/src/Http/Http/src/StreamPipeWriter.cs index 12fba0a93933..e5562c4d345e 100644 --- a/src/Http/Http/src/StreamPipeWriter.cs +++ b/src/Http/Http/src/StreamPipeWriter.cs @@ -278,7 +278,8 @@ private void AddSegment(int sizeHint = 0) } // Get a new buffer using the minimum segment size, unless the size hint is larger than a single segment. - _currentSegmentOwner = _pool.Rent(Math.Max(_minimumSegmentSize, sizeHint)); + // Also, the size cannot be larger than the MaxBufferSize of the MemoryPool + _currentSegmentOwner = _pool.Rent(Math.Min(Math.Max(_minimumSegmentSize, sizeHint), _pool.MaxBufferSize)); _currentSegment = _currentSegmentOwner.Memory; _position = 0; } diff --git a/src/Http/Http/test/PipeWriterTests.cs b/src/Http/Http/test/PipeWriterTests.cs index 3343abd6f3d3..67c3551ca595 100644 --- a/src/Http/Http/test/PipeWriterTests.cs +++ b/src/Http/Http/test/PipeWriterTests.cs @@ -90,13 +90,11 @@ public void CanGetSameMemoryWhenNoAdvance() } [Fact] - public void CanGetNewSpanWhenNoAdvanceWhenSizeTooLarge() + public void GetSpanWithZeroSizeHintReturnsMaxBufferSizeOfPool() { var span = Writer.GetSpan(0); - var secondSpan = Writer.GetSpan(10000); - - Assert.False(span.SequenceEqual(secondSpan)); + Assert.Equal(4096, span.Length); } [Fact] diff --git a/src/Http/Http/test/StreamPipeWriterTests.cs b/src/Http/Http/test/StreamPipeWriterTests.cs index abd7e224542a..e5a0795e6980 100644 --- a/src/Http/Http/test/StreamPipeWriterTests.cs +++ b/src/Http/Http/test/StreamPipeWriterTests.cs @@ -32,6 +32,7 @@ public async Task CanWriteAsyncMultipleTimesIntoSameBlock() [InlineData(8000, 8000)] public async Task CanAdvanceWithPartialConsumptionOfFirstSegment(int firstWriteLength, int secondWriteLength) { + Writer = new StreamPipeWriter(MemoryStream, MinimumSegmentSize, new TestMemoryPool(maxBufferSize: 20000)); await Writer.WriteAsync(Encoding.ASCII.GetBytes("a")); var expectedLength = firstWriteLength + secondWriteLength + 1; diff --git a/src/Http/Http/test/TestMemoryPool.cs b/src/Http/Http/test/TestMemoryPool.cs index dfb6b0477a78..5276c16ad3b6 100644 --- a/src/Http/Http/test/TestMemoryPool.cs +++ b/src/Http/Http/test/TestMemoryPool.cs @@ -14,12 +14,14 @@ namespace System.IO.Pipelines.Tests public class TestMemoryPool : MemoryPool { private MemoryPool _pool; - + private int _maxBufferSize; private bool _disposed; private int _rentCount; - public TestMemoryPool() + + public TestMemoryPool(int maxBufferSize = 4096) { _pool = new CustomMemoryPool(); + _maxBufferSize = maxBufferSize; } public override IMemoryOwner Rent(int minBufferSize = -1) @@ -39,7 +41,7 @@ protected override void Dispose(bool disposing) _disposed = true; } - public override int MaxBufferSize => 4096; + public override int MaxBufferSize => _maxBufferSize; internal void CheckDisposed() { From d9b50d7e8c8766f088c4803705a24608cc4c68af Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 1 Feb 2019 10:07:03 -0800 Subject: [PATCH 2/2] feedback --- src/Http/Http/src/StreamPipeWriter.cs | 2 +- src/Http/Http/test/PipeWriterTests.cs | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Http/Http/src/StreamPipeWriter.cs b/src/Http/Http/src/StreamPipeWriter.cs index e5562c4d345e..1e1b7993befb 100644 --- a/src/Http/Http/src/StreamPipeWriter.cs +++ b/src/Http/Http/src/StreamPipeWriter.cs @@ -279,7 +279,7 @@ private void AddSegment(int sizeHint = 0) // Get a new buffer using the minimum segment size, unless the size hint is larger than a single segment. // Also, the size cannot be larger than the MaxBufferSize of the MemoryPool - _currentSegmentOwner = _pool.Rent(Math.Min(Math.Max(_minimumSegmentSize, sizeHint), _pool.MaxBufferSize)); + _currentSegmentOwner = _pool.Rent(Math.Clamp(sizeHint, _minimumSegmentSize, _pool.MaxBufferSize)); _currentSegment = _currentSegmentOwner.Memory; _position = 0; } diff --git a/src/Http/Http/test/PipeWriterTests.cs b/src/Http/Http/test/PipeWriterTests.cs index 67c3551ca595..714cbbbccbd4 100644 --- a/src/Http/Http/test/PipeWriterTests.cs +++ b/src/Http/Http/test/PipeWriterTests.cs @@ -89,10 +89,12 @@ public void CanGetSameMemoryWhenNoAdvance() Assert.Equal(memory, secondMemory); } - [Fact] - public void GetSpanWithZeroSizeHintReturnsMaxBufferSizeOfPool() + [Theory] + [InlineData(0)] + [InlineData(2048)] + public void GetSpanWithZeroSizeHintReturnsMaxBufferSizeOfPool(int sizeHint) { - var span = Writer.GetSpan(0); + var span = Writer.GetSpan(sizeHint); Assert.Equal(4096, span.Length); }