From: David Fowler Date: Sat, 9 Feb 2019 01:40:42 +0000 (-0800) Subject: GetMemory/GetSpan takes a minimum size (dotnet/corefx#35186) X-Git-Tag: submit/tizen/20210909.063632~11031^2~2501 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=95702c8bebe365cd99b30087c578abe1cb2f330e;p=platform%2Fupstream%2Fdotnet%2Fruntime.git GetMemory/GetSpan takes a minimum size (dotnet/corefx#35186) - We changed the contract of IBufferWriter to be more usable by making the sizeHint a minimum size. If the caller requests memory over the max pool size then we allocate. - In the default case where we're using the shared array pool, we let the underlying pool handle this (it does internally). Commit migrated from https://github.com/dotnet/corefx/commit/c45d88d5a1326501e31c02e508efb352a492fb59 --- diff --git a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegment.cs b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegment.cs index f416e26..a0aa453 100644 --- a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegment.cs +++ b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/BufferSegment.cs @@ -51,17 +51,20 @@ namespace System.IO.Pipelines { _memoryOwner = memoryOwner; - AvailableMemory = memoryOwner.Memory; - RunningIndex = 0; - End = 0; - NextSegment = null; + SetUnownedMemory(memoryOwner.Memory); } public void SetMemory(byte[] arrayPoolBuffer) { _memoryOwner = arrayPoolBuffer; - AvailableMemory = arrayPoolBuffer; + SetUnownedMemory(arrayPoolBuffer); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void SetUnownedMemory(Memory memory) + { + AvailableMemory = memory; RunningIndex = 0; End = 0; NextSegment = null; @@ -73,9 +76,9 @@ namespace System.IO.Pipelines { owner.Dispose(); } - else + else if (_memoryOwner is byte[] array) { - ArrayPool.Shared.Return((byte[])_memoryOwner); + ArrayPool.Shared.Return(array); } _memoryOwner = null; diff --git a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs index 7d7de54..d9bb1bc 100644 --- a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs +++ b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs @@ -206,12 +206,19 @@ namespace System.IO.Pipelines if (_pool is null) { + // Use the array pool newSegment.SetMemory(ArrayPool.Shared.Rent(GetSegmentSize(sizeHint))); } - else + else if (sizeHint <= _pool.MaxBufferSize) { + // Use the specified pool if it fits newSegment.SetMemory(_pool.Rent(GetSegmentSize(sizeHint, _pool.MaxBufferSize))); } + else + { + // We can't use the pool so allocate an array + newSegment.SetUnownedMemory(new byte[sizeHint]); + } return newSegment; } diff --git a/src/libraries/System.IO.Pipelines/tests/PipePoolTests.cs b/src/libraries/System.IO.Pipelines/tests/PipePoolTests.cs index 1666dc4..e85d484 100644 --- a/src/libraries/System.IO.Pipelines/tests/PipePoolTests.cs +++ b/src/libraries/System.IO.Pipelines/tests/PipePoolTests.cs @@ -3,12 +3,13 @@ // See the LICENSE file in the project root for more information. using System.Buffers; +using System.Runtime.InteropServices; using System.Threading.Tasks; using Xunit; namespace System.IO.Pipelines.Tests { - public class PipePoolTests + public partial class PipePoolTests { private class DisposeTrackingBufferPool : TestMemoryPool { diff --git a/src/libraries/System.IO.Pipelines/tests/PipePoolTests.nonnetstandard.cs b/src/libraries/System.IO.Pipelines/tests/PipePoolTests.nonnetstandard.cs new file mode 100644 index 0000000..b070161 --- /dev/null +++ b/src/libraries/System.IO.Pipelines/tests/PipePoolTests.nonnetstandard.cs @@ -0,0 +1,125 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Buffers; +using System.Collections.Generic; +using System.Runtime.InteropServices; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace System.IO.Pipelines.Tests +{ + public partial class PipePoolTests + { + [Fact] + public async Task WritesToArrayPoolByDefault() + { + var pipe = new Pipe(); + pipe.Writer.WriteEmpty(10); + await pipe.Writer.FlushAsync(); + pipe.Writer.Complete(); + + ReadResult result = await pipe.Reader.ReadAsync(); + Assert.Equal(10, result.Buffer.Length); + + SequenceMarshal.TryGetReadOnlySequenceSegment( + result.Buffer, + out ReadOnlySequenceSegment start, + out int startIndex, + out ReadOnlySequenceSegment end, + out int endIndex); + + var startSegment = (BufferSegment)start; + var endSegment = (BufferSegment)end; + + Assert.Same(startSegment, endSegment); + Assert.NotNull(startSegment.Memory); + Assert.IsType(startSegment.MemoryOwner); + + pipe.Reader.AdvanceTo(result.Buffer.End); + pipe.Reader.Complete(); + } + + [Fact] + public async Task GetMemoryOverMaxPoolSizeAllocatesArray() + { + using (var pool = new DisposeTrackingBufferPool()) + { + var pipe = new Pipe(new PipeOptions(pool: pool)); + + // Allocate 5 KB + pipe.Writer.WriteEmpty(5 * 1024); + await pipe.Writer.FlushAsync(); + pipe.Writer.Complete(); + + Assert.Equal(0, pool.CurrentlyRentedBlocks); + + ReadResult result = await pipe.Reader.ReadAsync(); + Assert.Equal(5 * 1024, result.Buffer.Length); + + SequenceMarshal.TryGetReadOnlySequenceSegment( + result.Buffer, + out ReadOnlySequenceSegment start, + out int startIndex, + out ReadOnlySequenceSegment end, + out int endIndex); + + var startSegment = (BufferSegment)start; + var endSegment = (BufferSegment)end; + + Assert.Same(startSegment, endSegment); + + // Null owner implies that the buffer is allocated and wasn't rented from the pool + Assert.Null(startSegment.MemoryOwner); + + pipe.Reader.AdvanceTo(result.Buffer.End); + pipe.Reader.Complete(); + + Assert.Equal(0, pool.CurrentlyRentedBlocks); + Assert.Equal(0, pool.DisposedBlocks); + } + } + + [Fact] + public async Task GetMemoryAtMaxPoolSizeAllocatesFromPool() + { + using (var pool = new DisposeTrackingBufferPool()) + { + var pipe = new Pipe(new PipeOptions(pool: pool)); + + pipe.Writer.WriteEmpty(pool.MaxBufferSize); + await pipe.Writer.FlushAsync(); + pipe.Writer.Complete(); + + Assert.Equal(1, pool.CurrentlyRentedBlocks); + + ReadResult result = await pipe.Reader.ReadAsync(); + Assert.Equal(pool.MaxBufferSize, result.Buffer.Length); + + SequenceMarshal.TryGetReadOnlySequenceSegment( + result.Buffer, + out ReadOnlySequenceSegment start, + out int startIndex, + out ReadOnlySequenceSegment end, + out int endIndex); + + var startSegment = (BufferSegment)start; + var endSegment = (BufferSegment)end; + + Assert.Same(startSegment, endSegment); + + // Null owner implies that the buffer is allocated and wasn't rented from the pool + Assert.NotNull(startSegment.MemoryOwner); + + pipe.Reader.AdvanceTo(result.Buffer.End); + pipe.Reader.Complete(); + + Assert.Equal(0, pool.CurrentlyRentedBlocks); + Assert.Equal(1, pool.DisposedBlocks); + } + } + } +} diff --git a/src/libraries/System.IO.Pipelines/tests/PipeWriterTests.cs b/src/libraries/System.IO.Pipelines/tests/PipeWriterTests.cs index 51584cf..60b2aa5 100644 --- a/src/libraries/System.IO.Pipelines/tests/PipeWriterTests.cs +++ b/src/libraries/System.IO.Pipelines/tests/PipeWriterTests.cs @@ -188,13 +188,5 @@ namespace System.IO.Pipelines.Tests var exception = Assert.Throws(() => buffer.Advance(1)); Assert.Equal("No writing operation. Make sure GetMemory() was called.", exception.Message); } - - [Fact] - public void GetMemory_AdjustsToPoolMaxBufferSize() - { - PipeWriter buffer = Pipe.Writer; - var memory = buffer.GetMemory(int.MaxValue); - Assert.True(memory.Length >= 4096); - } } } diff --git a/src/libraries/System.IO.Pipelines/tests/System.IO.Pipelines.Tests.csproj b/src/libraries/System.IO.Pipelines/tests/System.IO.Pipelines.Tests.csproj index 42227d4..ac63118 100644 --- a/src/libraries/System.IO.Pipelines/tests/System.IO.Pipelines.Tests.csproj +++ b/src/libraries/System.IO.Pipelines/tests/System.IO.Pipelines.Tests.csproj @@ -32,5 +32,6 @@ + \ No newline at end of file