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 (file)
--- 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<byte> 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<byte>.Shared.Return((byte[])_memoryOwner);
+                ArrayPool<byte>.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 (file)
--- 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<byte>.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 (file)
--- 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 (file)
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<byte> start,
+               out int startIndex,
+               out ReadOnlySequenceSegment<byte> end,
+               out int endIndex);
+
+            var startSegment = (BufferSegment)start;
+            var endSegment = (BufferSegment)end;
+
+            Assert.Same(startSegment, endSegment);
+            Assert.NotNull(startSegment.Memory);
+            Assert.IsType<byte[]>(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<byte> start,
+                   out int startIndex,
+                   out ReadOnlySequenceSegment<byte> 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<byte> start,
+                   out int startIndex,
+                   out ReadOnlySequenceSegment<byte> 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 (file)
--- 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<InvalidOperationException>(() => 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 (file)
--- 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 @@
     <Compile Include="PipeLengthTests.cs" />
     <Compile Include="PipeReaderWriterFacts.nonnetstandard.cs" />
     <Compile Include="PipeResetTests.nonnetstandard.cs" />
+    <Compile Include="PipePoolTests.nonnetstandard.cs" />
   </ItemGroup>
 </Project>
\ No newline at end of file