SNIPacket performance improvement (#27187)

Gene Lee · saurabh500 · commit cb00beea1104 · 2018-02-26T10:20:58.000-08:00
* SNIPacket Perf Improvement

* comment fix
diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsConnection.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsConnection.cs
@@ -207,7 +207,7 @@ public void HandleReceiveComplete(SNIPacket packet, uint sniErrorCode)
                         };
 
                         _dataBytesLeft = (int)_currentHeader.length;
-                        _currentPacket = new SNIPacket(null);
+                        _currentPacket = new SNIPacket();
                         _currentPacket.Allocate((int)_currentHeader.length);
                     }
 
diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs
@@ -100,7 +100,7 @@ private void SendControlPacket(SNISMUXFlags flags)
                 GetSMUXHeaderBytes(0, (byte)flags, ref headerBytes);
             }
 
-            SNIPacket packet = new SNIPacket(null);
+            SNIPacket packet = new SNIPacket();
             packet.SetData(headerBytes, SNISMUXHeader.HEADER_LENGTH);
             
             _connection.Send(packet);
@@ -143,7 +143,7 @@ private SNIPacket GetSMUXEncapsulatedPacket(SNIPacket packet)
             byte[] headerBytes = null;
             GetSMUXHeaderBytes(packet.Length, (byte)SNISMUXFlags.SMUX_DATA, ref headerBytes);
 
-            SNIPacket smuxPacket = new SNIPacket(null);
+            SNIPacket smuxPacket = new SNIPacket();
             smuxPacket.Description = string.Format("({0}) SMUX packet {1}", packet.Description == null ? "" : packet.Description, xSequenceNumber);
             smuxPacket.Allocate(16 + packet.Length);
             smuxPacket.AppendData(headerBytes, 16);
diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNINpHandle.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNINpHandle.cs
@@ -154,7 +154,7 @@ public override uint Receive(out SNIPacket packet, int timeout)
                 packet = null;
                 try
                 {
-                    packet = new SNIPacket(null);
+                    packet = new SNIPacket();
                     packet.Allocate(_bufferSize);
                     packet.ReadFromStream(_stream);
 
@@ -181,7 +181,7 @@ public override uint ReceiveAsync(ref SNIPacket packet, bool isMars = false)
         {
             lock (this)
             {
-                packet = new SNIPacket(null);
+                packet = new SNIPacket();
                 packet.Allocate(_bufferSize);
 
                 try
diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs
@@ -15,20 +15,10 @@ internal class SNIPacket : IDisposable, IEquatable<SNIPacket>
     {
         private byte[] _data;
         private int _length;
-        private int _capacity;
         private int _offset;
         private string _description;
         private SNIAsyncCallback _completionCallback;
 
-        /// <summary>
-        /// Constructor
-        /// </summary>
-        /// <param name="handle">Owning SNI handle</param>
-        public SNIPacket(SNIHandle handle)
-        {
-            _offset = 0;
-        }
-
         /// <summary>
         /// Packet description (used for debugging)
         /// </summary>
@@ -46,7 +36,7 @@ public string Description
         }
 
         /// <summary>
-        /// Data left to process
+        /// Length of data left to process
         /// </summary>
         public int DataLeft
         {
@@ -67,9 +57,6 @@ public int Length
             }
         }
 
-        /// <summary>
-        /// Packet validity
-        /// </summary>
         public bool IsInvalid
         {
             get
@@ -78,14 +65,10 @@ public bool IsInvalid
             }
         }
 
-        /// <summary>
-        /// Packet data
-        /// </summary>
         public void Dispose()
         {
             _data = null;
-            _length = 0;
-            _capacity = 0;
+            Release();
         }
 
         /// <summary>
@@ -107,13 +90,18 @@ public void InvokeCompletionCallback(uint sniErrorCode)
         }
 
         /// <summary>
-        /// Allocate space for data
+        /// Allocate byte array for data.
         /// </summary>
-        /// <param name="capacity">Bytes to allocate</param>
-        public void Allocate(int capacity)
+        /// <param name="bufferSize">Minimum length of byte array to be allocated</param>
+        public void Allocate(int bufferSize)
         {
-            _capacity = capacity;
-            _data = new Byte[capacity];
+            if (_data == null || _data.Length != bufferSize)
+            {
+                _data = new byte[bufferSize];
+            }
+
+            _length = 0;
+            _offset = 0;
         }
 
         /// <summary>
@@ -122,18 +110,20 @@ public void Allocate(int capacity)
         /// <returns>Cloned packet</returns>
         public SNIPacket Clone()
         {
-            SNIPacket packet = new SNIPacket(null);
-            packet._data = new byte[_length];
-            Buffer.BlockCopy(_data, 0, packet._data, 0, _length);
+            SNIPacket packet = new SNIPacket();
+            packet._data = new byte[_data.Length];
+            Buffer.BlockCopy(_data, 0, packet._data, 0, _data.Length);
             packet._length = _length;
+            packet._description = _description;
+            packet._completionCallback = _completionCallback;
 
             return packet;
         }
 
         /// <summary>
         /// Get packet data
         /// </summary>
-        /// <param name="inBuff">Buffer</param>
+        /// <param name="buffer">Buffer</param>
         /// <param name="dataSize">Data in packet</param>
         public void GetData(byte[] buffer, ref int dataSize)
         {
@@ -150,7 +140,6 @@ public void SetData(byte[] data, int length)
         {
             _data = data;
             _length = length;
-            _capacity = length;
             _offset = 0;
         }
 
@@ -208,7 +197,8 @@ public int TakeData(byte[] buffer, int dataOffset, int size)
             }
 
             Buffer.BlockCopy(_data, _offset, buffer, dataOffset, size);
-            _offset = _offset + size;
+            _offset += size;
+
             return size;
         }
 
@@ -217,9 +207,7 @@ public int TakeData(byte[] buffer, int dataOffset, int size)
         /// </summary>
         public void Release()
         {
-            _length = 0;
-            _capacity = 0;
-            _data = null;
+            Reset();
         }
 
         /// <summary>
@@ -228,7 +216,9 @@ public void Release()
         public void Reset()
         {
             _length = 0;
-            _data = new byte[_capacity];
+            _offset = 0;
+            _description = null;
+            _completionCallback = null;
         }
 
         /// <summary>
@@ -249,7 +239,7 @@ public void ReadFromStreamAsync(Stream stream, SNIAsyncCallback callback, bool i
                 options |= TaskContinuationOptions.LongRunning;
             }
 
-            stream.ReadAsync(_data, 0, _capacity).ContinueWith(t =>
+            stream.ReadAsync(_data, 0, _data.Length).ContinueWith(t =>
             {
                 Exception e = t.Exception != null ? t.Exception.InnerException : null;
                 if (e != null)
@@ -270,7 +260,7 @@ public void ReadFromStreamAsync(Stream stream, SNIAsyncCallback callback, bool i
 
                 if (error)
                 {
-                    this.Release();
+                    Release();
                 }
 
                 callback(this, error ? TdsEnums.SNI_ERROR : TdsEnums.SNI_SUCCESS);
@@ -286,7 +276,7 @@ public void ReadFromStreamAsync(Stream stream, SNIAsyncCallback callback, bool i
         /// <param name="stream">Stream to read from</param>
         public void ReadFromStream(Stream stream)
         {
-            _length = stream.Read(_data, 0, _capacity);
+            _length = stream.Read(_data, 0, _data.Length);
         }
 
         /// <summary>
@@ -298,6 +288,11 @@ public void WriteToStream(Stream stream)
             stream.Write(_data, 0, _length);
         }
 
+        public Task WriteToStreamAsync(Stream stream)
+        {
+            return stream.WriteAsync(_data, 0, _length);
+        }
+
         /// <summary>
         /// Get hash code
         /// </summary>
diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIProxy.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIProxy.cs
@@ -189,11 +189,11 @@ public uint SetConnectionBufferSize(SNIHandle handle, uint bufferSize)
         }
 
         /// <summary>
-        /// Get packet data
+        /// Copies data packets in SNIPacket object to given byte array parameter
         /// </summary>
-        /// <param name="packet">SNI packet</param>
-        /// <param name="inBuff">Buffer</param>
-        /// <param name="dataSize">Data size</param>
+        /// <param name="packet">SNIPacket object containing data packets</param>
+        /// <param name="inBuff">Destination byte array where data packets are copied to</param>
+        /// <param name="dataSize">Length of data packets</param>
         /// <returns>SNI error status</returns>
         public uint PacketGetData(SNIPacket packet, byte[] inBuff, ref uint dataSize)
         {
@@ -240,11 +240,11 @@ public uint WritePacket(SNIHandle handle, SNIPacket packet, bool sync)
         {
             if (sync)
             {
-                return handle.Send(packet.Clone());
+                return handle.Send(packet);
             }
             else
             {
-                return handle.SendAsync(packet.Clone());
+                return handle.SendAsync(packet);
             }
         }
 
@@ -426,8 +426,7 @@ private SNINpHandle CreateNpHandle(DataSource details, long timerExpire, object
         /// <returns>SNI error status</returns>
         public uint ReadAsync(SNIHandle handle, out SNIPacket packet, bool isMars = false)
         {
-            packet = new SNIPacket(null);
-
+            packet = null;
             return handle.ReceiveAsync(ref packet);
         }
 
diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNITcpHandle.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNITcpHandle.cs
diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs

Original file line number	Diff line number	Diff line change
`@@ -207,7 +207,7 @@ public void HandleReceiveComplete(SNIPacket packet, uint sniErrorCode)`
`207`	`207`	`};`
`208`	`208`
`209`	`209`	`_dataBytesLeft = (int)_currentHeader.length;`
`210`		`- _currentPacket = new SNIPacket(null);`
	`210`	`+ _currentPacket = new SNIPacket();`
`211`	`211`	`_currentPacket.Allocate((int)_currentHeader.length);`
`212`	`212`	`}`
`213`	`213`
Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,7 @@ public override uint Receive(out SNIPacket packet, int timeout)`
`154`	`154`	`packet = null;`
`155`	`155`	`try`
`156`	`156`	`{`
`157`		`- packet = new SNIPacket(null);`
	`157`	`+ packet = new SNIPacket();`
`158`	`158`	`packet.Allocate(_bufferSize);`
`159`	`159`	`packet.ReadFromStream(_stream);`
`160`	`160`
`@@ -181,7 +181,7 @@ public override uint ReceiveAsync(ref SNIPacket packet, bool isMars = false)`
`181`	`181`	`{`
`182`	`182`	`lock (this)`
`183`	`183`	`{`
`184`		`- packet = new SNIPacket(null);`
	`184`	`+ packet = new SNIPacket();`
`185`	`185`	`packet.Allocate(_bufferSize);`
`186`	`186`
`187`	`187`	`try`
Original file line number	Diff line number	Diff line change
`@@ -15,20 +15,10 @@ internal class SNIPacket : IDisposable, IEquatable<SNIPacket>`
`15`	`15`	`{`
`16`	`16`	`private byte[] _data;`
`17`	`17`	`private int _length;`
`18`		`- private int _capacity;`
`19`	`18`	`private int _offset;`
`20`	`19`	`private string _description;`
`21`	`20`	`private SNIAsyncCallback _completionCallback;`
`22`	`21`
`23`		`- /// <summary>`
`24`		`- /// Constructor`
`25`		`- /// </summary>`
`26`		`- /// <param name="handle">Owning SNI handle</param>`
`27`		`- public SNIPacket(SNIHandle handle)`
`28`		`- {`
`29`		`- _offset = 0;`
`30`		`- }`
`31`		`-`
`32`	`22`	`/// <summary>`
`33`	`23`	`/// Packet description (used for debugging)`
`34`	`24`	`/// </summary>`
`@@ -46,7 +36,7 @@ public string Description`
`46`	`36`	`}`
`47`	`37`
`48`	`38`	`/// <summary>`
`49`		`- /// Data left to process`
	`39`	`+ /// Length of data left to process`
`50`	`40`	`/// </summary>`
`51`	`41`	`public int DataLeft`
`52`	`42`	`{`
`@@ -67,9 +57,6 @@ public int Length`
`67`	`57`	`}`
`68`	`58`	`}`
`69`	`59`
`70`		`- /// <summary>`
`71`		`- /// Packet validity`
`72`		`- /// </summary>`
`73`	`60`	`public bool IsInvalid`
`74`	`61`	`{`
`75`	`62`	`get`
`@@ -78,14 +65,10 @@ public bool IsInvalid`
`78`	`65`	`}`
`79`	`66`	`}`
`80`	`67`
`81`		`- /// <summary>`
`82`		`- /// Packet data`
`83`		`- /// </summary>`
`84`	`68`	`public void Dispose()`
`85`	`69`	`{`
`86`	`70`	`_data = null;`
`87`		`- _length = 0;`
`88`		`- _capacity = 0;`
	`71`	`+ Release();`
`89`	`72`	`}`
`90`	`73`
`91`	`74`	`/// <summary>`
`@@ -107,13 +90,18 @@ public void InvokeCompletionCallback(uint sniErrorCode)`
`107`	`90`	`}`
`108`	`91`
`109`	`92`	`/// <summary>`
`110`		`- /// Allocate space for data`
	`93`	`+ /// Allocate byte array for data.`
`111`	`94`	`/// </summary>`
`112`		`- /// <param name="capacity">Bytes to allocate</param>`
`113`		`- public void Allocate(int capacity)`
	`95`	`+ /// <param name="bufferSize">Minimum length of byte array to be allocated</param>`
	`96`	`+ public void Allocate(int bufferSize)`
`114`	`97`	`{`
`115`		`- _capacity = capacity;`
`116`		`- _data = new Byte[capacity];`
	`98`	`+ if (_data == null \|\| _data.Length != bufferSize)`
	`99`	`+ {`
	`100`	`+ _data = new byte[bufferSize];`
	`101`	`+ }`
	`102`	`+`
	`103`	`+ _length = 0;`
	`104`	`+ _offset = 0;`
`117`	`105`	`}`
`118`	`106`
`119`	`107`	`/// <summary>`
`@@ -122,18 +110,20 @@ public void Allocate(int capacity)`
`122`	`110`	`/// <returns>Cloned packet</returns>`
`123`	`111`	`public SNIPacket Clone()`
`124`	`112`	`{`
`125`		`- SNIPacket packet = new SNIPacket(null);`
`126`		`- packet._data = new byte[_length];`
`127`		`- Buffer.BlockCopy(_data, 0, packet._data, 0, _length);`
	`113`	`+ SNIPacket packet = new SNIPacket();`
	`114`	`+ packet._data = new byte[_data.Length];`
	`115`	`+ Buffer.BlockCopy(_data, 0, packet._data, 0, _data.Length);`
`128`	`116`	`packet._length = _length;`
	`117`	`+ packet._description = _description;`
	`118`	`+ packet._completionCallback = _completionCallback;`
`129`	`119`
`130`	`120`	`return packet;`
`131`	`121`	`}`
`132`	`122`
`133`	`123`	`/// <summary>`
`134`	`124`	`/// Get packet data`
`135`	`125`	`/// </summary>`
`136`		`- /// <param name="inBuff">Buffer</param>`
	`126`	`+ /// <param name="buffer">Buffer</param>`
`137`	`127`	`/// <param name="dataSize">Data in packet</param>`
`138`	`128`	`public void GetData(byte[] buffer, ref int dataSize)`
`139`	`129`	`{`
`@@ -150,7 +140,6 @@ public void SetData(byte[] data, int length)`
`150`	`140`	`{`
`151`	`141`	`_data = data;`
`152`	`142`	`_length = length;`
`153`		`- _capacity = length;`
`154`	`143`	`_offset = 0;`
`155`	`144`	`}`
`156`	`145`
`@@ -208,7 +197,8 @@ public int TakeData(byte[] buffer, int dataOffset, int size)`
`208`	`197`	`}`
`209`	`198`
`210`	`199`	`Buffer.BlockCopy(_data, _offset, buffer, dataOffset, size);`
`211`		`- _offset = _offset + size;`
	`200`	`+ _offset += size;`
	`201`	`+`
`212`	`202`	`return size;`
`213`	`203`	`}`
`214`	`204`
`@@ -217,9 +207,7 @@ public int TakeData(byte[] buffer, int dataOffset, int size)`
`217`	`207`	`/// </summary>`
`218`	`208`	`public void Release()`
`219`	`209`	`{`
`220`		`- _length = 0;`
`221`		`- _capacity = 0;`
`222`		`- _data = null;`
	`210`	`+ Reset();`
`223`	`211`	`}`
`224`	`212`
`225`	`213`	`/// <summary>`
`@@ -228,7 +216,9 @@ public void Release()`
`228`	`216`	`public void Reset()`
`229`	`217`	`{`
`230`	`218`	`_length = 0;`
`231`		`- _data = new byte[_capacity];`
	`219`	`+ _offset = 0;`
	`220`	`+ _description = null;`
	`221`	`+ _completionCallback = null;`
`232`	`222`	`}`
`233`	`223`
`234`	`224`	`/// <summary>`
`@@ -249,7 +239,7 @@ public void ReadFromStreamAsync(Stream stream, SNIAsyncCallback callback, bool i`
`249`	`239`	`options \|= TaskContinuationOptions.LongRunning;`
`250`	`240`	`}`
`251`	`241`
`252`		`- stream.ReadAsync(_data, 0, _capacity).ContinueWith(t =>`
	`242`	`+ stream.ReadAsync(_data, 0, _data.Length).ContinueWith(t =>`
`253`	`243`	`{`
`254`	`244`	`Exception e = t.Exception != null ? t.Exception.InnerException : null;`
`255`	`245`	`if (e != null)`
`@@ -270,7 +260,7 @@ public void ReadFromStreamAsync(Stream stream, SNIAsyncCallback callback, bool i`
`270`	`260`
`271`	`261`	`if (error)`
`272`	`262`	`{`
`273`		`- this.Release();`
	`263`	`+ Release();`
`274`	`264`	`}`
`275`	`265`
`276`	`266`	`callback(this, error ? TdsEnums.SNI_ERROR : TdsEnums.SNI_SUCCESS);`
`@@ -286,7 +276,7 @@ public void ReadFromStreamAsync(Stream stream, SNIAsyncCallback callback, bool i`
`286`	`276`	`/// <param name="stream">Stream to read from</param>`
`287`	`277`	`public void ReadFromStream(Stream stream)`
`288`	`278`	`{`
`289`		`- _length = stream.Read(_data, 0, _capacity);`
	`279`	`+ _length = stream.Read(_data, 0, _data.Length);`
`290`	`280`	`}`
`291`	`281`
`292`	`282`	`/// <summary>`
`@@ -298,6 +288,11 @@ public void WriteToStream(Stream stream)`
`298`	`288`	`stream.Write(_data, 0, _length);`
`299`	`289`	`}`
`300`	`290`
	`291`	`+ public Task WriteToStreamAsync(Stream stream)`
	`292`	`+ {`
	`293`	`+ return stream.WriteAsync(_data, 0, _length);`
	`294`	`+ }`
	`295`	`+`
`301`	`296`	`/// <summary>`
`302`	`297`	`/// Get hash code`
`303`	`298`	`/// </summary>`
Original file line number	Diff line number	Diff line change
`@@ -189,11 +189,11 @@ public uint SetConnectionBufferSize(SNIHandle handle, uint bufferSize)`
`189`	`189`	`}`
`190`	`190`
`191`	`191`	`/// <summary>`
`192`		`- /// Get packet data`
	`192`	`+ /// Copies data packets in SNIPacket object to given byte array parameter`
`193`	`193`	`/// </summary>`
`194`		`- /// <param name="packet">SNI packet</param>`
`195`		`- /// <param name="inBuff">Buffer</param>`
`196`		`- /// <param name="dataSize">Data size</param>`
	`194`	`+ /// <param name="packet">SNIPacket object containing data packets</param>`
	`195`	`+ /// <param name="inBuff">Destination byte array where data packets are copied to</param>`
	`196`	`+ /// <param name="dataSize">Length of data packets</param>`
`197`	`197`	`/// <returns>SNI error status</returns>`
`198`	`198`	`public uint PacketGetData(SNIPacket packet, byte[] inBuff, ref uint dataSize)`
`199`	`199`	`{`
`@@ -240,11 +240,11 @@ public uint WritePacket(SNIHandle handle, SNIPacket packet, bool sync)`
`240`	`240`	`{`
`241`	`241`	`if (sync)`
`242`	`242`	`{`
`243`		`- return handle.Send(packet.Clone());`
	`243`	`+ return handle.Send(packet);`
`244`	`244`	`}`
`245`	`245`	`else`
`246`	`246`	`{`
`247`		`- return handle.SendAsync(packet.Clone());`
	`247`	`+ return handle.SendAsync(packet);`
`248`	`248`	`}`
`249`	`249`	`}`
`250`	`250`
`@@ -426,8 +426,7 @@ private SNINpHandle CreateNpHandle(DataSource details, long timerExpire, object`
`426`	`426`	`/// <returns>SNI error status</returns>`
`427`	`427`	`public uint ReadAsync(SNIHandle handle, out SNIPacket packet, bool isMars = false)`
`428`	`428`	`{`
`429`		`- packet = new SNIPacket(null);`
`430`		`-`
	`429`	`+ packet = null;`
`431`	`430`	`return handle.ReceiveAsync(ref packet);`
`432`	`431`	`}`
`433`	`432`