slice buffer improvements

src/csharp/Grpc.Core.Tests/Internal/SliceBufferSafeHandleTest.cs

@@ -27,12 +27,99 @@ namespace Grpc.Core.Internal.Tests
     public class SliceBufferSafeHandleTest
     {
         [TestCase]
-        public void BasicTest()
+        public void Complete_EmptyBuffer()
         {
-            
+            using (var sliceBuffer = SliceBufferSafeHandle.Create())
+            {
+                sliceBuffer.Complete();
+                CollectionAssert.AreEqual(new byte[0], sliceBuffer.ToByteArray());
+            }
         }
 
-        // other ideas:
+        [TestCase]
-        // AdjustTailSpace(0) if previous tail size is 0... (better for SliceBufferSafeHandle)
+        public void Complete_TailSizeZero()
+        {
+            using (var sliceBuffer = SliceBufferSafeHandle.Create())
+            {
+                var origPayload = GetTestBuffer(10);
+                origPayload.AsSpan().CopyTo(sliceBuffer.GetSpan(origPayload.Length));
+                sliceBuffer.Advance(origPayload.Length);
+                // call complete where tail space size == 0
+                sliceBuffer.Complete();
+                CollectionAssert.AreEqual(origPayload, sliceBuffer.ToByteArray());
+            }
+        }
+
+        [TestCase]
+        public void Complete_TruncateTailSpace()
+        {
+            using (var sliceBuffer = SliceBufferSafeHandle.Create())
+            {
+                var origPayload = GetTestBuffer(10);
+                var dest = sliceBuffer.GetSpan(origPayload.Length + 10);
+                origPayload.AsSpan().CopyTo(dest);
+                sliceBuffer.Advance(origPayload.Length);
+                // call complete where tail space needs to be truncated
+                sliceBuffer.Complete();
+                CollectionAssert.AreEqual(origPayload, sliceBuffer.ToByteArray());
+            }
+        }
+
+        [TestCase]
+        public void SliceBufferIsReusable()
+        {
+            using (var sliceBuffer = SliceBufferSafeHandle.Create())
+            {
+                var origPayload = GetTestBuffer(10);
+                origPayload.AsSpan().CopyTo(sliceBuffer.GetSpan(origPayload.Length));
+                sliceBuffer.Advance(origPayload.Length);
+                sliceBuffer.Complete();
+                CollectionAssert.AreEqual(origPayload, sliceBuffer.ToByteArray());
+
+                sliceBuffer.Reset();
+
+                var origPayload2 = GetTestBuffer(20);
+                origPayload2.AsSpan().CopyTo(sliceBuffer.GetSpan(origPayload2.Length));
+                sliceBuffer.Advance(origPayload2.Length);
+                sliceBuffer.Complete();
+                CollectionAssert.AreEqual(origPayload2, sliceBuffer.ToByteArray());
+
+                sliceBuffer.Reset();
+
+                CollectionAssert.AreEqual(new byte[0], sliceBuffer.ToByteArray());
+            }
+        }
+
+        [TestCase]
+        public void SliceBuffer_SizeHintZero()
+        {
+            using (var sliceBuffer = SliceBufferSafeHandle.Create())
+            {
+                var destSpan = sliceBuffer.GetSpan(0);
+                Assert.IsTrue(destSpan.Length > 0);  // some non-zero size memory is made available
+
+                sliceBuffer.Reset();
+
+                var destMemory = sliceBuffer.GetMemory(0);
+                Assert.IsTrue(destMemory.Length > 0);
+            }
+        }
+
+        // Advance() - multiple chunks...
+
+        // other TODOS:
+
+        // -- provide a null instance of SliceBufferSafeHandle
+        //-- order of UnsafeSerialize in AsyncCall methods...
+
+        private byte[] GetTestBuffer(int length)
+        {
+            var testBuffer = new byte[length];
+            for (int i = 0; i < testBuffer.Length; i++)
+            {
+                testBuffer[i] = (byte) i;
+            }
+            return testBuffer;
+        }
     }
 }

src/csharp/Grpc.Core/Internal/SliceBufferSafeHandle.cs

@@ -31,6 +31,7 @@ namespace Grpc.Core.Internal
     /// </summary>
     internal class SliceBufferSafeHandle : SafeHandleZeroIsInvalid, IBufferWriter<byte>
     {
+        const int DefaultTailSpaceSize = 4096;  // default buffer to allocate if no size hint is provided
         static readonly NativeMethods Native = NativeMethods.Get();
         static readonly ILogger Logger = GrpcEnvironment.Logger.ForType<SliceBufferSafeHandle>();
 
@@ -72,7 +73,7 @@ namespace Grpc.Core.Internal
         // Use GetSpan when possible for better efficiency.
         public Memory<byte> GetMemory(int sizeHint = 0)
         {
-            GetSpan(sizeHint);
+            EnsureBufferSpace(sizeHint);
             if (memoryManagerLazy == null)
             {
                 memoryManagerLazy = new SliceMemoryManager();
@@ -84,13 +85,7 @@ namespace Grpc.Core.Internal
         // provides access to the "tail space" of this buffer.
         public unsafe Span<byte> GetSpan(int sizeHint = 0)
         {
-            GrpcPreconditions.CheckArgument(sizeHint >= 0);
+            EnsureBufferSpace(sizeHint);
-            if (tailSpaceLen < sizeHint)
-            {
-                // TODO: should we ignore the hint sometimes when
-                // available tail space is close enough to the sizeHint?
-                AdjustTailSpace(sizeHint);
-            }
             return new Span<byte>(tailSpacePtr.ToPointer(), tailSpaceLen);
         }
 
@@ -135,7 +130,27 @@ namespace Grpc.Core.Internal
             return result;
         }
 
-        // Gets data of server_rpc_new completion.
+        private void EnsureBufferSpace(int sizeHint)
+        {
+            GrpcPreconditions.CheckArgument(sizeHint >= 0);
+            if (sizeHint == 0)
+            {
+                // if no hint is provided, keep the available space within some "reasonable" boundaries.
+                // This is quite a naive approach which could use some fine-tuning, but currently in most case we know
+                // the required buffer size in advance anyway, so this approach seems good enough for now.
+                if (tailSpaceLen < DefaultTailSpaceSize /2 )
+                {
+                    AdjustTailSpace(DefaultTailSpaceSize);
+                }
+            }
+            else if (tailSpaceLen < sizeHint)
+            {
+                // if hint is provided, always make sure we provide at least that much space
+                AdjustTailSpace(sizeHint);
+            }
+        }
+
+        // make sure there's exactly requestedSize bytes of continguous buffer space at the end of this slice buffer
         private void AdjustTailSpace(int requestedSize)
         {
             GrpcPreconditions.CheckArgument(requestedSize >= 0);