BDN: use request struct to cleanup logic (#895)

* Use struct to simplify BDN logic

* update threshold

* cleanup

* nit

.github/workflows/ci-bdnbenchmark.yml

@@ -88,6 +88,6 @@ jobs:
           gh-pages-branch: 'continuousbenchmark'
           benchmark-data-dir-path: 'website/static/charts'
           max-items-in-chart: 50
-          alert-threshold: '10%'
+          alert-threshold: '110%'
           comment-always: true
           fail-on-alert: false

benchmark/BDN.benchmark/Embedded/EmbeddedNetworkHandler.cs

@@ -24,11 +24,12 @@ public override void Dispose()
 
         public override bool TryClose() => throw new NotImplementedException();
 
-        public unsafe void Send(byte[] buffer, byte* bufferPtr, int length)
+        public unsafe void Send(Request request)
         {
-            networkReceiveBuffer = buffer;
-            networkReceiveBufferPtr = bufferPtr;
-            OnNetworkReceive(length);
+            networkReceiveBuffer = request.buffer;
+            networkReceiveBufferPtr = request.bufferPtr;
+
+            OnNetworkReceive(request.buffer.Length);
 
             // We should have consumed the entire buffer
             Debug.Assert(networkBytesRead == 0);

benchmark/BDN.benchmark/Embedded/Request.cs

@@ -0,0 +1,11 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+namespace Embedded.server
+{
+    public struct Request
+    {
+        public byte[] buffer;
+        public unsafe byte* bufferPtr;
+    }
+}

benchmark/BDN.benchmark/Network/BasicOperations.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license.
 
 using BenchmarkDotNet.Attributes;
+using Embedded.server;
 
 namespace BDN.benchmark.Network
 {
@@ -12,19 +13,18 @@ namespace BDN.benchmark.Network
     public unsafe class BasicOperations : NetworkBase
     {
         static ReadOnlySpan<byte> INLINE_PING => "PING\r\n"u8;
-        byte[] pingRequestBuffer;
-        byte* pingRequestBufferPointer;
+        Request ping;
 
         public override void GlobalSetup()
         {
             base.GlobalSetup();
-            SetupOperation(ref pingRequestBuffer, ref pingRequestBufferPointer, INLINE_PING);
+            SetupOperation(ref ping, INLINE_PING);
         }
 
         [Benchmark]
         public void InlinePing()
         {
-            Send(pingRequestBuffer, pingRequestBufferPointer, pingRequestBuffer.Length);
+            Send(ping);
         }
     }
 }

benchmark/BDN.benchmark/Network/NetworkBase.cs

@@ -69,25 +69,24 @@ public virtual void GlobalCleanup()
             server.Dispose();
         }
 
-        protected void Send(byte[] requestBuffer, byte* requestBufferPointer, int length)
+        protected void Send(Request request)
         {
-            networkHandler.Send(requestBuffer, requestBufferPointer, length);
+            networkHandler.Send(request);
         }
 
-        protected void SetupOperation(ref byte[] requestBuffer, ref byte* requestBufferPointer, ReadOnlySpan<byte> operation)
+        protected unsafe void SetupOperation(ref Request request, ReadOnlySpan<byte> operation, int batchSize = batchSize)
         {
-            requestBuffer = GC.AllocateArray<byte>(operation.Length * batchSize, pinned: true);
-            requestBufferPointer = (byte*)Unsafe.AsPointer(ref requestBuffer[0]);
+            request.buffer = GC.AllocateArray<byte>(operation.Length * batchSize, pinned: true);
+            request.bufferPtr = (byte*)Unsafe.AsPointer(ref request.buffer[0]);
             for (int i = 0; i < batchSize; i++)
-                operation.CopyTo(new Span<byte>(requestBuffer).Slice(i * operation.Length));
+                operation.CopyTo(new Span<byte>(request.buffer).Slice(i * operation.Length));
         }
 
         protected void SlowConsumeMessage(ReadOnlySpan<byte> message)
         {
-            var buffer = GC.AllocateArray<byte>(message.Length, pinned: true);
-            var bufferPointer = (byte*)Unsafe.AsPointer(ref buffer[0]);
-            message.CopyTo(new Span<byte>(buffer));
-            Send(buffer, bufferPointer, buffer.Length);
+            Request request = default;
+            SetupOperation(ref request, message, 1);
+            Send(request);
         }
     }
 }

benchmark/BDN.benchmark/Network/RawStringOperations.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license.
 
 using BenchmarkDotNet.Attributes;
+using Embedded.server;
 
 namespace BDN.benchmark.Network
 {
@@ -12,58 +13,48 @@ namespace BDN.benchmark.Network
     public unsafe class RawStringOperations : NetworkBase
     {
         static ReadOnlySpan<byte> SET => "*3\r\n$3\r\nSET\r\n$1\r\na\r\n$1\r\na\r\n"u8;
-        byte[] setRequestBuffer;
-        byte* setRequestBufferPointer;
+        Request set;
 
         static ReadOnlySpan<byte> SETEX => "*4\r\n$5\r\nSETEX\r\n$1\r\nd\r\n$1\r\n9\r\n$1\r\nd\r\n"u8;
-        byte[] setexRequestBuffer;
-        byte* setexRequestBufferPointer;
+        Request setex;
 
         static ReadOnlySpan<byte> SETNX => "*4\r\n$3\r\nSET\r\n$1\r\na\r\n$1\r\na\r\n$2\r\nNX\r\n"u8;
-        byte[] setnxRequestBuffer;
-        byte* setnxRequestBufferPointer;
+        Request setnx;
 
         static ReadOnlySpan<byte> SETXX => "*4\r\n$3\r\nSET\r\n$1\r\na\r\n$1\r\na\r\n$2\r\nXX\r\n"u8;
-        byte[] setxxRequestBuffer;
-        byte* setxxRequestBufferPointer;
+        Request setxx;
 
         static ReadOnlySpan<byte> GETNF => "*2\r\n$3\r\nGET\r\n$1\r\nb\r\n"u8;
-        byte[] getnfRequestBuffer;
-        byte* getnfRequestBufferPointer;
+        Request getnf;
 
         static ReadOnlySpan<byte> GETF => "*2\r\n$3\r\nGET\r\n$1\r\na\r\n"u8;
-        byte[] getfRequestBuffer;
-        byte* getfRequestBufferPointer;
+        Request getf;
 
         static ReadOnlySpan<byte> INCR => "*2\r\n$4\r\nINCR\r\n$1\r\ni\r\n"u8;
-        byte[] incrRequestBuffer;
-        byte* incrRequestBufferPointer;
+        Request incr;
 
         static ReadOnlySpan<byte> DECR => "*2\r\n$4\r\nDECR\r\n$1\r\nj\r\n"u8;
-        byte[] decrRequestBuffer;
-        byte* decrRequestBufferPointer;
+        Request decr;
 
         static ReadOnlySpan<byte> INCRBY => "*3\r\n$6\r\nINCRBY\r\n$1\r\nk\r\n$10\r\n1234567890\r\n"u8;
-        byte[] incrbyRequestBuffer;
-        byte* incrbyRequestBufferPointer;
+        Request incrby;
 
         static ReadOnlySpan<byte> DECRBY => "*3\r\n$6\r\nDECRBY\r\n$1\r\nl\r\n$10\r\n1234567890\r\n"u8;
-        byte[] decrbyRequestBuffer;
-        byte* decrbyRequestBufferPointer;
+        Request decrby;
 
         public override void GlobalSetup()
         {
             base.GlobalSetup();
-            SetupOperation(ref setRequestBuffer, ref setRequestBufferPointer, SET);
-            SetupOperation(ref setexRequestBuffer, ref setexRequestBufferPointer, SETEX);
-            SetupOperation(ref setnxRequestBuffer, ref setnxRequestBufferPointer, SETNX);
-            SetupOperation(ref setxxRequestBuffer, ref setxxRequestBufferPointer, SETXX);
-            SetupOperation(ref getfRequestBuffer, ref getfRequestBufferPointer, GETF);
-            SetupOperation(ref getnfRequestBuffer, ref getnfRequestBufferPointer, GETNF);
-            SetupOperation(ref incrRequestBuffer, ref incrRequestBufferPointer, INCR);
-            SetupOperation(ref decrRequestBuffer, ref decrRequestBufferPointer, DECR);
-            SetupOperation(ref incrbyRequestBuffer, ref incrbyRequestBufferPointer, INCRBY);
-            SetupOperation(ref decrbyRequestBuffer, ref decrbyRequestBufferPointer, DECRBY);
+            SetupOperation(ref set, SET);
+            SetupOperation(ref setex, SETEX);
+            SetupOperation(ref setnx, SETNX);
+            SetupOperation(ref setxx, SETXX);
+            SetupOperation(ref getf, GETF);
+            SetupOperation(ref getnf, GETNF);
+            SetupOperation(ref incr, INCR);
+            SetupOperation(ref decr, DECR);
+            SetupOperation(ref incrby, INCRBY);
+            SetupOperation(ref decrby, DECRBY);
 
             // Pre-populate data
             SlowConsumeMessage("*3\r\n$3\r\nSET\r\n$1\r\na\r\n$1\r\na\r\n"u8);
@@ -76,61 +67,61 @@ public override void GlobalSetup()
         [Benchmark]
         public void Set()
         {
-            Send(setRequestBuffer, setRequestBufferPointer, setRequestBuffer.Length);
+            Send(set);
         }
 
         [Benchmark]
         public void SetEx()
         {
-            Send(setexRequestBuffer, setexRequestBufferPointer, setexRequestBuffer.Length);
+            Send(setex);
         }
 
         [Benchmark]
         public void SetNx()
         {
-            Send(setnxRequestBuffer, setnxRequestBufferPointer, setnxRequestBuffer.Length);
+            Send(setnx);
         }
 
         [Benchmark]
         public void SetXx()
         {
-            Send(setxxRequestBuffer, setxxRequestBufferPointer, setxxRequestBuffer.Length);
+            Send(setxx);
         }
 
         [Benchmark]
         public void GetFound()
         {
-            Send(getfRequestBuffer, getfRequestBufferPointer, getfRequestBuffer.Length);
+            Send(getf);
         }
 
         [Benchmark]
         public void GetNotFound()
         {
-            Send(getnfRequestBuffer, getnfRequestBufferPointer, getnfRequestBuffer.Length);
+            Send(getnf);
         }
 
         [Benchmark]
         public void Increment()
         {
-            Send(incrRequestBuffer, incrRequestBufferPointer, incrRequestBuffer.Length);
+            Send(incr);
         }
 
         [Benchmark]
         public void Decrement()
         {
-            Send(decrRequestBuffer, decrRequestBufferPointer, decrRequestBuffer.Length);
+            Send(decr);
         }
 
         [Benchmark]
         public void IncrementBy()
         {
-            Send(incrbyRequestBuffer, incrbyRequestBufferPointer, incrbyRequestBuffer.Length);
+            Send(incrby);
         }
 
         [Benchmark]
         public void DecrementBy()
         {
-            Send(decrbyRequestBuffer, decrbyRequestBufferPointer, decrbyRequestBuffer.Length);
+            Send(decrby);
         }
     }
 }

benchmark/BDN.benchmark/Operations/BasicOperations.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license.
 
 using BenchmarkDotNet.Attributes;
+using Embedded.server;
 
 namespace BDN.benchmark.Operations
 {
@@ -12,19 +13,18 @@ namespace BDN.benchmark.Operations
     public unsafe class BasicOperations : OperationsBase
     {
         static ReadOnlySpan<byte> INLINE_PING => "PING\r\n"u8;
-        byte[] pingRequestBuffer;
-        byte* pingRequestBufferPointer;
+        Request ping;
 
         public override void GlobalSetup()
         {
             base.GlobalSetup();
-            SetupOperation(ref pingRequestBuffer, ref pingRequestBufferPointer, INLINE_PING);
+            SetupOperation(ref ping, INLINE_PING);
         }
 
         [Benchmark]
         public void InlinePing()
         {
-            Send(pingRequestBuffer, pingRequestBufferPointer, pingRequestBuffer.Length);
+            Send(ping);
         }
     }
 }

benchmark/BDN.benchmark/Operations/CustomOperations.cs

@@ -3,6 +3,7 @@
 
 using BDN.benchmark.CustomProcs;
 using BenchmarkDotNet.Attributes;
+using Embedded.server;
 using Garnet;
 using Garnet.server;
 
@@ -15,20 +16,16 @@ namespace BDN.benchmark.Operations
     public unsafe class CustomOperations : OperationsBase
     {
         static ReadOnlySpan<byte> SETIFPM => "*4\r\n$7\r\nSETIFPM\r\n$1\r\nk\r\n$3\r\nval\r\n$1\r\nv\r\n"u8;
-        byte[] setIfPmRequestBuffer;
-        byte* setIfPmRequestBufferPointer;
+        Request setIfPm;
 
         static ReadOnlySpan<byte> MYDICTSETGET => "*4\r\n$9\r\nMYDICTSET\r\n$2\r\nck\r\n$1\r\nf\r\n$1\r\nv\r\n*3\r\n$9\r\nMYDICTGET\r\n$2\r\nck\r\n$1\r\nf\r\n"u8;
-        byte[] myDictSetGetRequestBuffer;
-        byte* myDictSetGetRequestBufferPointer;
+        Request myDictSetGet;
 
         static ReadOnlySpan<byte> CTXNSET => "*9\r\n$7\r\nCTXNSET\r\n$6\r\n{0}000\r\n$6\r\n{0}001\r\n$6\r\n{0}002\r\n$6\r\n{0}003\r\n$6\r\n{0}000\r\n$6\r\n{0}001\r\n$6\r\n{0}002\r\n$6\r\n{0}003\r\n"u8;
-        byte[] ctxnsetBuffer;
-        byte* ctxnsetBufferPointer;
+        Request ctxnset;
 
         static ReadOnlySpan<byte> CPROCSET => "*9\r\n$8\r\nCPROCSET\r\n$6\r\n{0}000\r\n$6\r\n{0}001\r\n$6\r\n{0}002\r\n$6\r\n{0}003\r\n$6\r\n{0}000\r\n$6\r\n{0}001\r\n$6\r\n{0}002\r\n$6\r\n{0}003\r\n"u8;
-        byte[] cprocsetBuffer;
-        byte* cprocsetBufferPointer;
+        Request cprocset;
 
         void CreateExtensions()
         {
@@ -55,39 +52,39 @@ public override void GlobalSetup()
             base.GlobalSetup();
             CreateExtensions();
 
-            SetupOperation(ref setIfPmRequestBuffer, ref setIfPmRequestBufferPointer, SETIFPM);
-            SetupOperation(ref myDictSetGetRequestBuffer, ref myDictSetGetRequestBufferPointer, MYDICTSETGET);
-            SetupOperation(ref ctxnsetBuffer, ref ctxnsetBufferPointer, CTXNSET);
-            SetupOperation(ref cprocsetBuffer, ref cprocsetBufferPointer, CPROCSET);
+            SetupOperation(ref setIfPm, SETIFPM);
+            SetupOperation(ref myDictSetGet, MYDICTSETGET);
+            SetupOperation(ref ctxnset, CTXNSET);
+            SetupOperation(ref cprocset, CPROCSET);
 
             SlowConsumeMessage("*4\r\n$7\r\nSETIFPM\r\n$1\r\nk\r\n$3\r\nval\r\n$1\r\nv\r\n"u8);
             SlowConsumeMessage("*4\r\n$9\r\nMYDICTSET\r\n$2\r\nck\r\n$1\r\nf\r\n$1\r\nv\r\n"u8);
-            SlowConsumeMessage(ctxnsetBuffer);
-            SlowConsumeMessage(cprocsetBuffer);
+            SlowConsumeMessage(CTXNSET);
+            SlowConsumeMessage(CPROCSET);
         }
 
         [Benchmark]
         public void CustomRawStringCommand()
         {
-            _ = session.TryConsumeMessages(setIfPmRequestBufferPointer, setIfPmRequestBuffer.Length);
+            Send(setIfPm);
         }
 
         [Benchmark]
         public void CustomObjectCommand()
         {
-            _ = session.TryConsumeMessages(myDictSetGetRequestBufferPointer, myDictSetGetRequestBuffer.Length);
+            Send(myDictSetGet);
         }
 
         [Benchmark]
         public void CustomTransaction()
         {
-            _ = session.TryConsumeMessages(ctxnsetBufferPointer, ctxnsetBuffer.Length);
+            Send(ctxnset);
         }
 
         [Benchmark]
         public void CustomProcedure()
         {
-            _ = session.TryConsumeMessages(cprocsetBufferPointer, cprocsetBuffer.Length);
+            Send(cprocset);
         }
     }
 }