Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Optimize SqlClient primitive type writes #34048

Merged
saurabh500 merged 5 commits into from
Dec 17, 2018
Merged

Optimize SqlClient primitive type writes #34048

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ae678cf
change public WriteArrayBytes to private WriteBytes and add span capa…
Wraith2 Dec 11, 2018
a543f92
add netcoreapp specific implementations of WriteFloat and WriteDouble…
Wraith2 Dec 11, 2018
6062aab
fixup array compy slice to not use remainder as start since it has al…
Wraith2 Dec 14, 2018
46d9875
Merge remote-tracking branch 'dotnet/master' into sqlperf-writespan
Wraith2 Dec 14, 2018
0684c90
remove additional span
Wraith2 Dec 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<byte> 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
}

//
Expand Down Expand Up @@ -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<byte> 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)
Expand Down
72 changes: 58 additions & 14 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2984,11 +2984,29 @@ internal void WriteByte(byte b)
_outBuff[_outBytesUsed++] = b;
}

//
// Takes a byte array and writes it to the buffer.
//

internal Task WriteByteSpan(ReadOnlySpan<byte> span, bool canAccumulate = true, TaskCompletionSource<object> completion = null)
{
return WriteBytes(span, span.Length, 0, canAccumulate, completion);
}

internal Task WriteByteArray(byte[] b, int len, int offsetBuffer, bool canAccumulate = true, TaskCompletionSource<object> completion = null)
{
return WriteBytes(ReadOnlySpan<byte>.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<byte> b, int len, int offsetBuffer, bool canAccumulate = true, TaskCompletionSource<object> completion = null, byte[] array = null)
{
if (array != null)
{
b = new ReadOnlySpan<byte>(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
Expand All @@ -3003,26 +3021,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<byte> copyTo = _outBuff.AsSpan(_outBytesUsed, remainder);
ReadOnlySpan<byte> 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);

Expand All @@ -3035,16 +3058,36 @@ internal Task WriteByteArray(byte[] b, int len, int offsetBuffer, bool canAccumu
completion = new TaskCompletionSource<object>();
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<byte> copyTempTo = tempArray.AsSpan();
ReadOnlySpan<byte> copyTempFrom = b.Slice(0, len);

@saurabh500 saurabh500 Dec 14, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you just get rid of the call at 3048. It doesn't seem to be doing anything useful. And call b.Slice(remainder, len) like you were doing earlier, at this location? if ( array == null) is the only location where you are going to get the rest of the content of b
Does this make sense ?

@Wraith2 Wraith2 Dec 14, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, there is no guarantee that the WritePacket call will return a task so it's possible to go around the loop without allocating the array. This was a problem that occurred with one of the SplitPacket tests that was pointed out on the original PR.

@saurabh500 saurabh500 Dec 14, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, now I understand. In that case, what is the effect of just doing a b = b.Slice(remainder, len) at 3048 and eliminate this slicing at 3066. Would that work? Then this line would read ReadOnlySpan<byte> copyTempFrom = b;


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<byte> copyTo = _outBuff.AsSpan(_outBytesUsed, len);
ReadOnlySpan<byte> 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;
Expand Down Expand Up @@ -3072,12 +3115,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<object> 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<object> completion, int offset, Task packetTask)
{
AsyncHelper.ContinueTask(packetTask, completion,
() => WriteByteArray(b, len: len, offsetBuffer: offset, canAccumulate: false, completion: completion),
connectionToDoom: _parser.Connection);
() => WriteBytes(ReadOnlySpan<byte>.Empty, len: len, offsetBuffer: offset, canAccumulate: false, completion: completion, array),
connectionToDoom: _parser.Connection
);
}

// Dumps contents of buffer to SNI for network write.
Expand Down