From ae678cfbe4256d1403b08fa5b4d624935f057188 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Tue, 11 Dec 2018 23:16:16 +0000 Subject: [PATCH 1/4] change public WriteArrayBytes to private WriteBytes and add span capable path with fallback to array on continuation add WriteByteSpan and WriteByteArray public methods --- .../Data/SqlClient/TdsParserStateObject.cs | 72 +++++++++++++++---- 1 file changed, 58 insertions(+), 14 deletions(-) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs index dcc7eae1744e..2f2a326d508a 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -2998,11 +2998,29 @@ internal void WriteByte(byte b) _outBuff[_outBytesUsed++] = b; } - // - // Takes a byte array and writes it to the buffer. - // + + internal Task WriteByteSpan(ReadOnlySpan span, bool canAccumulate = true, TaskCompletionSource completion = null) + { + return WriteBytes(span, span.Length, 0, canAccumulate, completion); + } + internal Task WriteByteArray(byte[] b, int len, int offsetBuffer, bool canAccumulate = true, TaskCompletionSource completion = null) { + return WriteBytes(ReadOnlySpan.Empty, len, offsetBuffer, canAccumulate, completion, b); + } + + // + // Takes a span or a byte array and writes it to the buffer + // If you pass in a span and a null array then the span wil be used. + // If you pass in a non-null array then the array will be used and the span is ignored. + // if the span cannot be written into the current packet then the remaining contents of the span are copied to a + // new heap allocated array that will used to callback into the method to continue the write operation. + private Task WriteBytes(ReadOnlySpan b, int len, int offsetBuffer, bool canAccumulate = true, TaskCompletionSource completion = null, byte[] array = null) + { + if (array != null) + { + b = new ReadOnlySpan(array, offsetBuffer, len); + } try { bool async = _parser._asyncWrite; // NOTE: We are capturing this now for the assert after the Task is returned, since WritePacket will turn off async if there is an exception @@ -3017,26 +3035,31 @@ internal Task WriteByteArray(byte[] b, int len, int offsetBuffer, bool canAccumu int offset = offsetBuffer; - Debug.Assert(b.Length >= len, "Invalid length sent to WriteByteArray()!"); + Debug.Assert(b.Length >= len, "Invalid length sent to WriteBytes()!"); // loop through and write the entire array do { if ((_outBytesUsed + len) > _outBuff.Length) { - // If the remainder of the string won't fit into the buffer, then we have to put + // If the remainder of the data won't fit into the buffer, then we have to put // whatever we can into the buffer, and flush that so we can then put more into // the buffer on the next loop of the while. int remainder = _outBuff.Length - _outBytesUsed; // write the remainder - Buffer.BlockCopy(b, offset, _outBuff, _outBytesUsed, remainder); + Span copyTo = _outBuff.AsSpan(_outBytesUsed, remainder); + ReadOnlySpan copyFrom = b.Slice(0, remainder); + + Debug.Assert(copyTo.Length == copyFrom.Length, $"copyTo.Length:{copyTo.Length} and copyFrom.Length{copyFrom.Length:D} should be the same"); + + copyFrom.CopyTo(copyTo); - // handle counters offset += remainder; _outBytesUsed += remainder; len -= remainder; + b = b.Slice(remainder); Task packetTask = WritePacket(TdsEnums.SOFTFLUSH, canAccumulate); @@ -3049,16 +3072,36 @@ internal Task WriteByteArray(byte[] b, int len, int offsetBuffer, bool canAccumu completion = new TaskCompletionSource(); task = completion.Task; // we only care about return from topmost call, so do not access Task property in other cases } - WriteByteArraySetupContinuation(b, len, completion, offset, packetTask); + + if (array == null) + { + byte[] tempArray = new byte[len]; + Span copyTempTo = tempArray.AsSpan(); + ReadOnlySpan copyTempFrom = b.Slice(remainder, len); + + Debug.Assert(copyTempTo.Length == copyTempFrom.Length, $"copyTempTo.Length:{copyTempTo.Length} and copyTempFrom.Length:{copyTempFrom.Length:D} should be the same"); + + copyTempFrom.CopyTo(copyTempTo); + array = tempArray; + offset = 0; + } + + WriteBytesSetupContinuation(array, len, completion, offset, packetTask); return task; } } else - { //((stateObj._outBytesUsed + len) <= stateObj._outBuff.Length ) + { + //((stateObj._outBytesUsed + len) <= stateObj._outBuff.Length ) // Else the remainder of the string will fit into the buffer, so copy it into the // buffer and then break out of the loop. - Buffer.BlockCopy(b, offset, _outBuff, _outBytesUsed, len); + Span copyTo = _outBuff.AsSpan(_outBytesUsed, len); + ReadOnlySpan copyFrom = b.Slice(0, len); + + Debug.Assert(copyTo.Length == copyFrom.Length, $"copyTo.Length:{copyTo.Length} and copyFrom.Length:{copyFrom.Length:D} should be the same"); + + copyFrom.CopyTo(copyTo); // handle out buffer bytes used counter _outBytesUsed += len; @@ -3086,12 +3129,13 @@ internal Task WriteByteArray(byte[] b, int len, int offsetBuffer, bool canAccumu } } - // This is in its own method to avoid always allocating the lambda in WriteByteArray - private void WriteByteArraySetupContinuation(byte[] b, int len, TaskCompletionSource completion, int offset, Task packetTask) + // This is in its own method to avoid always allocating the lambda in WriteBytes + private void WriteBytesSetupContinuation(byte[] array, int len, TaskCompletionSource completion, int offset, Task packetTask) { AsyncHelper.ContinueTask(packetTask, completion, - () => WriteByteArray(b, len: len, offsetBuffer: offset, canAccumulate: false, completion: completion), - connectionToDoom: _parser.Connection); + () => WriteBytes(ReadOnlySpan.Empty, len: len, offsetBuffer: offset, canAccumulate: false, completion: completion, array), + connectionToDoom: _parser.Connection + ); } // Dumps contents of buffer to SNI for network write. From a543f925ee87221ea409a3a1a9a3c2321be0c65f Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Tue, 11 Dec 2018 23:17:02 +0000 Subject: [PATCH 2/4] add netcoreapp specific implementations of WriteFloat and WriteDouble which avoid byte array allocation in favour of span --- .../src/System/Data/SqlClient/TdsParser.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs index e7e35d351c0e..43f3d8af6dd4 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs @@ -1419,9 +1419,14 @@ internal void WriteInt(int v, TdsParserStateObject stateObj) // internal void WriteFloat(float v, TdsParserStateObject stateObj) { +#if netcoreapp // BitConverter.TryGetBytes is only availble in netcoreapp 2.1> at this time, review support when changing + Span bytes = stackalloc byte[sizeof(float)]; + BitConverter.TryWriteBytes(bytes, v); + stateObj.WriteByteSpan(bytes); +#else byte[] bytes = BitConverter.GetBytes(v); - - stateObj.WriteByteArray(bytes, bytes.Length, 0); + stateObj.WriteByteArray(bytes, bytes.Length, 0); +#endif } // @@ -1493,9 +1498,14 @@ internal void WriteUnsignedLong(ulong uv, TdsParserStateObject stateObj) // internal void WriteDouble(double v, TdsParserStateObject stateObj) { +#if netcoreapp // BitConverter.TryGetBytes is only availble in netcoreapp 2.1> at this time, review support when changing + Span bytes = stackalloc byte[sizeof(double)]; + BitConverter.TryWriteBytes(bytes, v); + stateObj.WriteByteSpan(bytes); +#else byte[] bytes = BitConverter.GetBytes(v); - stateObj.WriteByteArray(bytes, bytes.Length, 0); +#endif } internal void PrepareResetConnection(bool preserveTransaction) From 6062aab21d95e57b57ca2e84dd5159d99107481b Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Fri, 14 Dec 2018 10:45:23 +0000 Subject: [PATCH 3/4] fixup array compy slice to not use remainder as start since it has already advanced --- .../src/System/Data/SqlClient/TdsParserStateObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs index 2f2a326d508a..174e162c3ac6 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -3077,7 +3077,7 @@ private Task WriteBytes(ReadOnlySpan b, int len, int offsetBuffer, bool ca { byte[] tempArray = new byte[len]; Span copyTempTo = tempArray.AsSpan(); - ReadOnlySpan copyTempFrom = b.Slice(remainder, len); + ReadOnlySpan copyTempFrom = b.Slice(0, len); Debug.Assert(copyTempTo.Length == copyTempFrom.Length, $"copyTempTo.Length:{copyTempTo.Length} and copyTempFrom.Length:{copyTempFrom.Length:D} should be the same"); From 0684c903a1e87de6e96d0dbde7a6833af7e0c9d0 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Fri, 14 Dec 2018 20:33:41 +0000 Subject: [PATCH 4/4] remove additional span --- .../src/System/Data/SqlClient/TdsParserStateObject.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs index 388d9219747b..fea7447e4f5f 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -3045,7 +3045,7 @@ private Task WriteBytes(ReadOnlySpan b, int len, int offsetBuffer, bool ca offset += remainder; _outBytesUsed += remainder; len -= remainder; - b = b.Slice(remainder); + b = b.Slice(remainder, len); Task packetTask = WritePacket(TdsEnums.SOFTFLUSH, canAccumulate); @@ -3063,11 +3063,10 @@ private Task WriteBytes(ReadOnlySpan b, int len, int offsetBuffer, bool ca { byte[] tempArray = new byte[len]; Span copyTempTo = tempArray.AsSpan(); - ReadOnlySpan copyTempFrom = b.Slice(0, len); - Debug.Assert(copyTempTo.Length == copyTempFrom.Length, $"copyTempTo.Length:{copyTempTo.Length} and copyTempFrom.Length:{copyTempFrom.Length:D} should be the same"); + Debug.Assert(copyTempTo.Length == b.Length, $"copyTempTo.Length:{copyTempTo.Length} and copyTempFrom.Length:{b.Length:D} should be the same"); - copyTempFrom.CopyTo(copyTempTo); + b.CopyTo(copyTempTo); array = tempArray; offset = 0; }