SqlClient improve managed memory usage (dotnet/corefx#34234)

* add missing packet disposal calls
remove temporary array use with mux header

* revert to using array in mux header

* mark MarsConnection mux header as readonly

Commit migrated from https://github.com/dotnet/corefx/commit/715c1e632ccec57b46a18eed2a268109e666ddab

diff --git a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNICommon.cs b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNICommon.cs
index ee96dbe..c54e1e5 100644 (file)
--- a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNICommon.cs
+++ b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNICommon.cs
@@ -34,7 +34,7 @@ namespace System.Data.SqlClient.SNI
     /// <summary>
     /// SMUX packet header
     /// </summary>
-    internal class SNISMUXHeader
+    internal sealed class SNISMUXHeader
     {
         public const int HEADER_LENGTH = 16;
 
@@ -44,6 +44,46 @@ namespace System.Data.SqlClient.SNI
         public uint length;
         public uint sequenceNumber;
         public uint highwater;
+
+        public void Read(byte[] bytes)
+        {
+            SMID = bytes[0];
+            flags = bytes[1];
+            sessionId = BitConverter.ToUInt16(bytes, 2);
+            length = BitConverter.ToUInt32(bytes, 4) - SNISMUXHeader.HEADER_LENGTH;
+            sequenceNumber = BitConverter.ToUInt32(bytes, 8);
+            highwater = BitConverter.ToUInt32(bytes, 12);
+        }
+
+        public void Write(Span<byte> bytes)
+        {
+            uint value = highwater;
+            // access the highest element first to cause the largest range check in the jit, then fill in the rest of the value and carry on as normal
+            bytes[15] = (byte)((value >> 24) & 0xff);
+            bytes[12] = (byte)(value & 0xff); // BitConverter.GetBytes(_currentHeader.highwater).CopyTo(headerBytes, 12);
+            bytes[13] = (byte)((value >> 8) & 0xff);
+            bytes[14] = (byte)((value >> 16) & 0xff);
+
+            bytes[0] = SMID; // BitConverter.GetBytes(_currentHeader.SMID).CopyTo(headerBytes, 0);
+            bytes[1] = flags; // BitConverter.GetBytes(_currentHeader.flags).CopyTo(headerBytes, 1);
+
+            value = sessionId;
+            bytes[2] = (byte)(value & 0xff); // BitConverter.GetBytes(_currentHeader.sessionId).CopyTo(headerBytes, 2);
+            bytes[3] = (byte)((value >> 8) & 0xff);
+
+            value = length;
+            bytes[4] = (byte)(value & 0xff); // BitConverter.GetBytes(_currentHeader.length).CopyTo(headerBytes, 4);
+            bytes[5] = (byte)((value >> 8) & 0xff);
+            bytes[6] = (byte)((value >> 16) & 0xff);
+            bytes[7] = (byte)((value >> 24) & 0xff);
+
+            value = sequenceNumber;
+            bytes[8] = (byte)(value & 0xff); // BitConverter.GetBytes(_currentHeader.sequenceNumber).CopyTo(headerBytes, 8);
+            bytes[9] = (byte)((value >> 8) & 0xff);
+            bytes[10] = (byte)((value >> 16) & 0xff);
+            bytes[11] = (byte)((value >> 24) & 0xff);
+
+        }
     }
 
     /// <summary>

diff --git a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsConnection.cs b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsConnection.cs
index c5ea70c..989e9a2 100644 (file)
--- a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsConnection.cs
+++ b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsConnection.cs
@@ -16,12 +16,11 @@ namespace System.Data.SqlClient.SNI
         private readonly Guid _connectionId = Guid.NewGuid();
         private readonly Dictionary<int, SNIMarsHandle> _sessions = new Dictionary<int, SNIMarsHandle>();
         private readonly byte[] _headerBytes = new byte[SNISMUXHeader.HEADER_LENGTH];
-
+        private readonly SNISMUXHeader _currentHeader = new SNISMUXHeader();
         private SNIHandle _lowerHandle;
         private ushort _nextSessionId = 0;
         private int _currentHeaderByteCount = 0;
         private int _dataBytesLeft = 0;
-        private SNISMUXHeader _currentHeader;
         private SNIPacket _currentPacket;
 
         /// <summary>
@@ -134,6 +133,7 @@ namespace System.Data.SqlClient.SNI
             {
                 handle.HandleReceiveError(packet);
             }
+            packet?.Dispose();
         }
 
         /// <summary>
@@ -183,6 +183,8 @@ namespace System.Data.SqlClient.SNI
 
                             if (bytesTaken == 0)
                             {
+                                packet.Dispose();
+                                packet = null;
                                 sniErrorCode = ReceiveAsync(ref packet);
 
                                 if (sniErrorCode == TdsEnums.SNI_SUCCESS_IO_PENDING)
@@ -195,16 +197,7 @@ namespace System.Data.SqlClient.SNI
                             }
                         }
 
-                        _currentHeader = new SNISMUXHeader()
-                        {
-                            SMID = _headerBytes[0],
-                            flags = _headerBytes[1],
-                            sessionId = BitConverter.ToUInt16(_headerBytes, 2),
-                            length = BitConverter.ToUInt32(_headerBytes, 4) - SNISMUXHeader.HEADER_LENGTH,
-                            sequenceNumber = BitConverter.ToUInt32(_headerBytes, 8),
-                            highwater = BitConverter.ToUInt32(_headerBytes, 12)
-                        };
-
+                        _currentHeader.Read(_headerBytes);
                         _dataBytesLeft = (int)_currentHeader.length;
                         _currentPacket = new SNIPacket((int)_currentHeader.length);
                     }
@@ -221,6 +214,8 @@ namespace System.Data.SqlClient.SNI
 
                             if (_dataBytesLeft > 0)
                             {
+                                packet.Dispose();
+                                packet = null;
                                 sniErrorCode = ReceiveAsync(ref packet);
 
                                 if (sniErrorCode == TdsEnums.SNI_SUCCESS_IO_PENDING)
@@ -276,6 +271,8 @@ namespace System.Data.SqlClient.SNI
                 {
                     if (packet.DataLeft == 0)
                     {
+                        packet.Dispose();
+                        packet = null;
                         sniErrorCode = ReceiveAsync(ref packet);
 
                         if (sniErrorCode == TdsEnums.SNI_SUCCESS_IO_PENDING)

diff --git a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs
index 5a0e8f7..6ba9039 100644 (file)
--- a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs
+++ b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs
@@ -93,43 +93,29 @@ namespace System.Data.SqlClient.SNI
         /// <param name="flags">SMUX header flags</param>
         private void SendControlPacket(SNISMUXFlags flags)
         {
-            byte[] headerBytes = null;
-
+            Span<byte> headerBytes = stackalloc byte[SNISMUXHeader.HEADER_LENGTH];
             lock (this)
             {
-                GetSMUXHeaderBytes(0, (byte)flags, ref headerBytes);
+                GetSMUXHeaderBytes(0, flags, headerBytes);
             }
 
-            SNIPacket packet = new SNIPacket();
-            packet.SetData(headerBytes, SNISMUXHeader.HEADER_LENGTH);
+            SNIPacket packet = new SNIPacket(SNISMUXHeader.HEADER_LENGTH);
+            packet.AppendData(headerBytes);
             
             _connection.Send(packet);
         }
 
-        /// <summary>
-        /// Generate SMUX header 
-        /// </summary>
-        /// <param name="length">Packet length</param>
-        /// <param name="flags">Packet flags</param>
-        /// <param name="headerBytes">Header in bytes</param>
-        private void GetSMUXHeaderBytes(int length, byte flags, ref byte[] headerBytes)
+        private void GetSMUXHeaderBytes(int length, SNISMUXFlags flags, Span<byte> bytes)
         {
-            headerBytes = new byte[SNISMUXHeader.HEADER_LENGTH];
-
             _currentHeader.SMID = 83;
-            _currentHeader.flags = flags;
+            _currentHeader.flags = (byte)flags;
             _currentHeader.sessionId = _sessionId;
             _currentHeader.length = (uint)SNISMUXHeader.HEADER_LENGTH + (uint)length;
-            _currentHeader.sequenceNumber = ((flags == (byte)SNISMUXFlags.SMUX_FIN) || (flags == (byte)SNISMUXFlags.SMUX_ACK)) ? _sequenceNumber - 1 : _sequenceNumber++;
+            _currentHeader.sequenceNumber = ((flags == SNISMUXFlags.SMUX_FIN) || (flags == SNISMUXFlags.SMUX_ACK)) ? _sequenceNumber - 1 : _sequenceNumber++;
             _currentHeader.highwater = _receiveHighwater;
             _receiveHighwaterLastAck = _currentHeader.highwater;
 
-            BitConverter.GetBytes(_currentHeader.SMID).CopyTo(headerBytes, 0);
-            BitConverter.GetBytes(_currentHeader.flags).CopyTo(headerBytes, 1);
-            BitConverter.GetBytes(_currentHeader.sessionId).CopyTo(headerBytes, 2);
-            BitConverter.GetBytes(_currentHeader.length).CopyTo(headerBytes, 4);
-            BitConverter.GetBytes(_currentHeader.sequenceNumber).CopyTo(headerBytes, 8);
-            BitConverter.GetBytes(_currentHeader.highwater).CopyTo(headerBytes, 12);
+            _currentHeader.Write(bytes);
         }
 
         /// <summary>
@@ -140,14 +126,14 @@ namespace System.Data.SqlClient.SNI
         private SNIPacket GetSMUXEncapsulatedPacket(SNIPacket packet)
         {
             uint xSequenceNumber = _sequenceNumber;
-            byte[] headerBytes = null;
-            GetSMUXHeaderBytes(packet.Length, (byte)SNISMUXFlags.SMUX_DATA, ref headerBytes);
+            Span<byte> header = stackalloc byte[SNISMUXHeader.HEADER_LENGTH];
+            GetSMUXHeaderBytes(packet.Length, SNISMUXFlags.SMUX_DATA, header);
 
-            SNIPacket smuxPacket = new SNIPacket(16 + packet.Length);
-            smuxPacket.Description = string.Format("({0}) SMUX packet {1}", packet.Description == null ? "" : packet.Description, xSequenceNumber);
-            smuxPacket.AppendData(headerBytes, 16);
-            smuxPacket.AppendPacket(packet);
 
+            SNIPacket smuxPacket = new SNIPacket(SNISMUXHeader.HEADER_LENGTH + packet.Length);
+            smuxPacket.AppendData(header);
+            smuxPacket.AppendPacket(packet);
+            packet.Dispose();
             return smuxPacket;
         }
 
@@ -175,8 +161,9 @@ namespace System.Data.SqlClient.SNI
                     _ackEvent.Reset();
                 }
             }
+            SNIPacket encapsulatedPacket = GetSMUXEncapsulatedPacket(packet);
 
-            return _connection.Send(GetSMUXEncapsulatedPacket(packet));
+            return _connection.Send(encapsulatedPacket);
         }
 
         /// <summary>
@@ -187,8 +174,6 @@ namespace System.Data.SqlClient.SNI
         /// <returns>SNI error code</returns>
         private uint InternalSendAsync(SNIPacket packet, SNIAsyncCallback callback)
         {
-            SNIPacket encapsulatedPacket = null;
-
             lock (this)
             {
                 if (_sequenceNumber >= _sendHighwater)
@@ -196,7 +181,7 @@ namespace System.Data.SqlClient.SNI
                     return TdsEnums.SNI_QUEUE_FULL;
                 }
 
-                encapsulatedPacket = GetSMUXEncapsulatedPacket(packet);
+                SNIPacket encapsulatedPacket = GetSMUXEncapsulatedPacket(packet);
 
                 if (callback != null)
                 {

diff --git a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs
index a047e94..cfc53aa 100644 (file)
--- a/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs
+++ b/src/libraries/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs
@@ -20,7 +20,7 @@ namespace System.Data.SqlClient.SNI
         private int _offset;
         private string _description;
         private SNIAsyncCallback _completionCallback;
-        private bool _isBufferFromArrayPool = false;
+        private bool _isBufferFromArrayPool;
 
         public SNIPacket() { }
 
@@ -176,6 +176,12 @@ namespace System.Data.SqlClient.SNI
             _length += size;
         }
 
+        public void AppendData(ReadOnlySpan<byte> data)
+        {
+            data.CopyTo(_data.AsSpan(_length));
+            _length += data.Length;
+        }
+
         /// <summary>
         /// Append another packet
         /// </summary>