Skip to content

Make GetMemory use MaxBufferSize for MemoryPool #7143

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.

Sign up for GitHub

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 2 commits into from
Feb 4, 2019
Merged

Make GetMemory use MaxBufferSize for MemoryPool #7143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5dc8455
Make GetMemory use MaxBufferSize for MemoryPool
jkotalik Jan 30, 2019
d9b50d7
feedback
jkotalik Feb 1, 2019
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
3 changes: 2 additions & 1 deletion src/Http/Http/src/StreamPipeWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.Clamp(sizeHint, _minimumSegmentSize, _pool.MaxBufferSize));
_currentSegment = _currentSegmentOwner.Memory;
_position = 0;
}
Expand Down
12 changes: 6 additions & 6 deletions src/Http/Http/test/PipeWriterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ public void CanGetSameMemoryWhenNoAdvance()
Assert.Equal(memory, secondMemory);
}

[Fact]
public void CanGetNewSpanWhenNoAdvanceWhenSizeTooLarge()
[Theory]
[InlineData(0)]
[InlineData(2048)]
public void GetSpanWithZeroSizeHintReturnsMaxBufferSizeOfPool(int sizeHint)
{
var span = Writer.GetSpan(0);

var secondSpan = Writer.GetSpan(10000);
var span = Writer.GetSpan(sizeHint);

Assert.False(span.SequenceEqual(secondSpan));
Assert.Equal(4096, span.Length);
Copy link
Member

Choose a reason for hiding this comment

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

Can you verify this same result for when 0 < sizeHint < 4096?

Can you also update CanGetNewMemoryWhenSizeTooLarge to verify that memoryLarge.Length is equal to MemoryPool<byte>.MaxBufferSize?

}

[Fact]
Expand Down
1 change: 1 addition & 0 deletions src/Http/Http/test/StreamPipeWriterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 5 additions & 3 deletions src/Http/Http/test/TestMemoryPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ namespace System.IO.Pipelines.Tests
public class TestMemoryPool : MemoryPool<byte>
{
private MemoryPool<byte> _pool;

private int _maxBufferSize;
private bool _disposed;
private int _rentCount;
public TestMemoryPool()

public TestMemoryPool(int maxBufferSize = 4096)
{
_pool = new CustomMemoryPool<byte>();
_maxBufferSize = maxBufferSize;
}

public override IMemoryOwner<byte> Rent(int minBufferSize = -1)
Expand All @@ -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()
{
Expand Down