From c033395bdfe8469c375cb7b09ffea78d99895c16 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Sun, 17 Sep 2023 22:46:57 +0100 Subject: [PATCH] move PacketData and list management to shared and reimplement as doubly linked list. --- .../SqlClient/TdsParserStateObject.netcore.cs | 152 ++---------- .../SqlClient/TdsParserStateObject.netfx.cs | 108 +-------- .../Data/SqlClient/TdsParserStateObject.cs | 229 ++++++++++++++++-- 3 files changed, 241 insertions(+), 248 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 9ac485cbbb..0a9aff81bc 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -20,7 +20,7 @@ internal abstract partial class TdsParserStateObject // Timeout variables private readonly WeakReference _cancellationOwner = new WeakReference(null); - // Async + // Async ////////////////// // Constructors // @@ -1072,12 +1072,18 @@ internal bool TryReadNetworkPacket() { if (_snapshotReplay) { - if (_snapshot.Replay()) +#if DEBUG + // in debug builds stack traces contain line numbers so if we want to be + // able to compare the stack traces they must all be created in the same + // location in the code + string stackTrace = Environment.StackTrace; +#endif + if (_snapshot.MoveNext()) { #if DEBUG if (s_checkNetworkPacketRetryStacks) { - _snapshot.CheckStack(Environment.StackTrace); + _snapshot.CheckStack(stackTrace); } #endif return true; @@ -1087,7 +1093,7 @@ internal bool TryReadNetworkPacket() { if (s_checkNetworkPacketRetryStacks) { - _lastStack = Environment.StackTrace; + _lastStack = stackTrace; } } #endif @@ -1123,7 +1129,7 @@ internal bool TryReadNetworkPacket() internal void PrepareReplaySnapshot() { _networkPacketTaskSource = null; - _snapshot.PrepareReplay(); + _snapshot.MoveToStart(); } internal void ReadSniSyncOverAsync() @@ -1434,7 +1440,7 @@ internal void ReadSni(TaskCompletionSource completion) Timeout.Infinite, Timeout.Infinite ); - + // -1 == Infinite // 0 == Already timed out (NOTE: To simulate the same behavior as sync we will only timeout on 0 if we receive an IO Pending from SNI) @@ -1740,10 +1746,10 @@ public void ProcessSniPacket(PacketHandle packet, uint error) if (_snapshot != null) { - _snapshot.PushBuffer(_inBuff, _inBytesRead); + _snapshot.AppendPacketData(_inBuff, _inBytesRead); if (_snapshotReplay) { - _snapshot.Replay(); + _snapshot.MoveNext(); #if DEBUG _snapshot.AssertCurrent(); #endif @@ -3024,137 +3030,7 @@ internal void CloneCleanupAltMetaDataSetArray() sealed partial class StateSnapshot { - private sealed partial class PacketData - { - public byte[] Buffer; - public int Read; - public PacketData Prev; - - public void SetStack(string value) - { - SetStackInternal(value); - } - partial void SetStackInternal(string value); - - public void Clear() - { - Buffer = null; - Read = 0; - Prev = null; - SetStackInternal(null); - } - } - -#if DEBUG - private sealed partial class PacketData - { - public string Stack; - - partial void SetStackInternal(string value) - { - Stack = value; - } - } - - -#endif - private PacketData _snapshotInBuffList; - private PacketData _sparePacket; - internal byte[] _plpBuffer; - - private int _snapshotInBuffCount; - -#if DEBUG - internal void AssertCurrent() - { - Debug.Assert(_snapshotInBuffCurrent == _snapshotInBuffCount, "Should not be reading new packets when not replaying last packet"); - } - - internal void CheckStack(string trace) - { - PacketData prev = _snapshotInBuffList?.Prev; - if (prev.Stack == null) - { - prev.Stack = trace; - } - else - { - Debug.Assert(_stateObj._permitReplayStackTraceToDiffer || prev.Stack.ToString() == trace.ToString(), "The stack trace on subsequent replays should be the same"); - } - } -#endif - internal void PushBuffer(byte[] buffer, int read) - { -#if DEBUG - for (PacketData current = _snapshotInBuffList; current != null; current = current.Prev) - { - Debug.Assert(!object.ReferenceEquals(current.Buffer, buffer)); - } -#endif - - PacketData packetData = _sparePacket; - if (packetData is null) - { - packetData = new PacketData(); - } - else - { - _sparePacket = null; - } - packetData.Buffer = buffer; - packetData.Read = read; - packetData.Prev = _snapshotInBuffList; -#if DEBUG - packetData.SetStack(_stateObj._lastStack); -#endif - _snapshotInBuffList = packetData; - _snapshotInBuffCount++; - } - - internal bool Replay() - { - if (_snapshotInBuffCurrent < _snapshotInBuffCount) - { - PacketData next = _snapshotInBuffList; - for ( - int position = (_snapshotInBuffCount - 1); - position != _snapshotInBuffCurrent; - position -= 1 - ) - { - next = next.Prev; - } - _stateObj._inBuff = next.Buffer; - _stateObj._inBytesUsed = 0; - _stateObj._inBytesRead = next.Read; - _snapshotInBuffCurrent++; - return true; - } - - return false; - } - - internal void Snap(TdsParserStateObject state) - { - _snapshotInBuffList = null; - _snapshotInBuffCount = 0; - _snapshotInBuffCurrent = 0; - - CaptureAsStart(state); - } - - internal void Clear() - { - PacketData packet = _snapshotInBuffList; - _snapshotInBuffList = null; - _snapshotInBuffCount = 0; - - packet.Clear(); - _sparePacket = packet; - - ClearCore(); - } } } } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index e64c25c523..4e06844eaa 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -1164,7 +1164,7 @@ internal bool TrySkipBytes(int num) ///////////////////////////////////////// #if DEBUG - StackTrace _lastStack; + string _lastStack; #endif internal bool TryReadNetworkPacket() @@ -1179,12 +1179,18 @@ internal bool TryReadNetworkPacket() { if (_snapshotReplay) { - if (_snapshot.Replay()) +#if DEBUG + // in debug builds stack traces contain line numbers so if we want to be + // able to compare the stack traces they must all be created in the same + // location in the code + string stackTrace = Environment.StackTrace; +#endif + if (_snapshot.MoveNext()) { #if DEBUG if (s_checkNetworkPacketRetryStacks) { - _snapshot.CheckStack(new StackTrace()); + _snapshot.CheckStack(stackTrace); } #endif return true; @@ -1194,7 +1200,7 @@ internal bool TryReadNetworkPacket() { if (s_checkNetworkPacketRetryStacks) { - _lastStack = new StackTrace(); + _lastStack = stackTrace; } } #endif @@ -1230,7 +1236,7 @@ internal bool TryReadNetworkPacket() internal void PrepareReplaySnapshot() { _networkPacketTaskSource = null; - _snapshot.PrepareReplay(); + _snapshot.MoveToStart(); } internal void ReadSniSyncOverAsync() @@ -1884,10 +1890,10 @@ public void ProcessSniPacket(IntPtr packet, uint error) if (_snapshot != null) { - _snapshot.PushBuffer(_inBuff, _inBytesRead); + _snapshot.AppendPacketData(_inBuff, _inBytesRead); if (_snapshotReplay) { - _snapshot.Replay(); + _snapshot.MoveNext(); #if DEBUG _snapshot.AssertCurrent(); #endif @@ -3189,98 +3195,10 @@ internal void CloneCleanupAltMetaDataSetArray() } } - class PacketData - { - public byte[] Buffer; - public int Read; -#if DEBUG - public StackTrace Stack; -#endif - } sealed partial class StateSnapshot { - private List _snapshotInBuffs; - - public StateSnapshot() - { - _snapshotInBuffs = new List(); - } - -#if DEBUG - internal void AssertCurrent() - { - Debug.Assert(_snapshotInBuffCurrent == _snapshotInBuffs.Count, "Should not be reading new packets when not replaying last packet"); - } - - internal void CheckStack(StackTrace trace) - { - PacketData prev = _snapshotInBuffs[_snapshotInBuffCurrent - 1]; - if (prev.Stack == null) - { - prev.Stack = trace; - } - else - { - Debug.Assert(_stateObj._permitReplayStackTraceToDiffer || prev.Stack.ToString() == trace.ToString(), "The stack trace on subsequent replays should be the same"); - } - } -#endif - - internal void PushBuffer(byte[] buffer, int read) - { -#if DEBUG - if (_snapshotInBuffs != null && _snapshotInBuffs.Count > 0) - { - foreach (PacketData packet in _snapshotInBuffs) - { - if (object.ReferenceEquals(packet.Buffer, buffer)) - { - Debug.Assert(false,"buffer is already present in packet list"); - } - } - } -#endif - - PacketData packetData = new PacketData(); - packetData.Buffer = buffer; - packetData.Read = read; -#if DEBUG - packetData.Stack = _stateObj._lastStack; -#endif - - _snapshotInBuffs.Add(packetData); - } - - internal bool Replay() - { - if (_snapshotInBuffCurrent < _snapshotInBuffs.Count) - { - PacketData next = _snapshotInBuffs[_snapshotInBuffCurrent]; - _stateObj._inBuff = next.Buffer; - _stateObj._inBytesUsed = 0; - _stateObj._inBytesRead = next.Read; - _snapshotInBuffCurrent++; - return true; - } - return false; - } - - internal void Snap(TdsParserStateObject state) - { - _snapshotInBuffs.Clear(); - _snapshotInBuffCurrent = 0; - - CaptureAsStart(state); - } - - internal void Clear() - { - _snapshotInBuffs.Clear(); - - ClearCore(); - } } } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 7147c10e5a..4c764a2bfa 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1163,7 +1163,7 @@ internal void SetSnapshot() snapshot.Clear(); } _snapshot = snapshot; - _snapshot.Snap(this); + _snapshot.CaptureAsStart(this); _snapshotReplay = false; } @@ -1181,6 +1181,109 @@ internal void ResetSnapshot() sealed partial class StateSnapshot { + private sealed partial class PacketData + { + public byte[] Buffer; + public int Read; + public PacketData NextPacket; + public PacketData PrevPacket; + + internal void Clear() + { + Buffer = null; + Read = 0; + NextPacket = null; + if (PrevPacket != null) + { + PrevPacket.NextPacket = null; + PrevPacket = null; + } + SetDebugStackInternal(null); + SetDebugPacketIdInternal(0); + } + + internal void SetDebugStack(string value) => SetDebugStackInternal(value); + internal void SetDebugPacketId(int value) => SetDebugPacketIdInternal(value); + + partial void SetDebugStackInternal(string value); + partial void SetDebugPacketIdInternal(int value); + } + +#if DEBUG + [DebuggerDisplay("{ToString(),nq}")] + [DebuggerTypeProxy(typeof(PacketDataDebugView))] + private sealed partial class PacketData + { + internal sealed class PacketDataDebugView + { + private readonly PacketData _data; + + public PacketDataDebugView(PacketData data) + { + if (data == null) + { + throw new ArgumentNullException(nameof(data)); + } + + _data = data; + } + + [DebuggerBrowsable(DebuggerBrowsableState.RootHidden)] + public PacketData[] Items + { + get + { + PacketData[] items = Array.Empty(); + if (_data != null) + { + int count = 0; + for (PacketData current = _data; current != null; current = current?.NextPacket) + { + count++; + } + items = new PacketData[count]; + int index = 0; + for (PacketData current = _data; current != null; current = current?.NextPacket, index++) + { + items[index] = current; + } + } + return items; + } + } + } + + public int PacketId; + public string Stack; + + partial void SetDebugStackInternal(string value) => Stack = value; + + partial void SetDebugPacketIdInternal(int value) => PacketId = value; + + + public override string ToString() + { + //return $"{PacketId}: [{Buffer.Length}] ( {GetPacketDataOffset():D4}, {GetPacketTotalSize():D4} ) {(NextPacket != null ? @"->" : string.Empty)}"; + string byteString = null; + if (Buffer != null && Buffer.Length >= 12) + { + ReadOnlySpan bytes = Buffer.AsSpan(0, 12); + StringBuilder buffer = new StringBuilder(12 * 3 + 10); + buffer.Append('{'); + for (int index = 0; index < bytes.Length; index++) + { + buffer.AppendFormat("{0:X2}", bytes[index]); + buffer.Append(", "); + } + buffer.Append("..."); + buffer.Append('}'); + byteString = buffer.ToString(); + } + return $"{PacketId}: [{Read}] {byteString} {(NextPacket != null ? @"->" : string.Empty)}"; + } + } +#endif + private sealed class PLPData { public readonly ulong SnapshotLongLen; @@ -1276,11 +1379,16 @@ internal void Restore(TdsParserStateObject stateObj) } } - private int _snapshotInBuffCurrent; private TdsParserStateObject _stateObj; private StateObjectData _replayStateData; + private PacketData _lastPacket; + private PacketData _firstPacket; + private PacketData _current; + private PacketData _sparePacket; + #if DEBUG + private int _packetCounter; private int _rollingPend = 0; private int _rollingPendCount = 0; @@ -1301,6 +1409,24 @@ internal bool DoPend() _rollingPendCount++; return false; } + + internal void AssertCurrent() + { + Debug.Assert(_current == _lastPacket); + } + + internal void CheckStack(string trace) + { + PacketData prev = _current?.PrevPacket; + if (prev.Stack == null) + { + prev.Stack = trace; + } + else + { + Debug.Assert(_stateObj._permitReplayStackTraceToDiffer || prev.Stack == trace, "The stack trace on subsequent replays should be the same"); + } + } #endif internal void CloneNullBitmapInfo() { @@ -1318,27 +1444,85 @@ internal void CloneCleanupAltMetaDataSetArray() } } - internal void ResetSnapshotState() + internal void AppendPacketData(byte[] buffer, int read) { - // go back to the beginning - _snapshotInBuffCurrent = 0; - - Replay(); + Debug.Assert(buffer != null, "packet data cannot be null"); + Debug.Assert(read >= TdsEnums.HEADER_LEN, "minimum packet length is TdsEnums.HEADER_LEN"); +#if DEBUG + for (PacketData current = _firstPacket; current != null; current = current.NextPacket) + { + Debug.Assert(!ReferenceEquals(current.Buffer, buffer)); + } +#endif + PacketData packetData = _sparePacket; + if (packetData is null) + { + packetData = new PacketData(); + } + else + { + _sparePacket = null; + } + packetData.Buffer = buffer; + packetData.Read = read; +#if DEBUG + packetData.SetDebugStack(_stateObj._lastStack); + packetData.SetDebugPacketId(Interlocked.Increment(ref _packetCounter)); +#endif + if (_firstPacket is null) + { + _firstPacket = packetData; + } + else + { + _lastPacket.NextPacket = packetData; + packetData.PrevPacket = _lastPacket; + } + _lastPacket = packetData; + } - _replayStateData.Restore(_stateObj); + internal bool MoveNext() + { + bool retval = false; + bool moved = false; + if (_current == null) + { + _current = _firstPacket; + moved = true; + } + else if (_current.NextPacket != null) + { + _current = _current.NextPacket; + moved = true; + } - _stateObj._snapshotReplay = true; + if (moved) + { + _stateObj._inBuff = _current.Buffer; + _stateObj._inBytesUsed = 0; + _stateObj._inBytesRead = _current.Read; + _stateObj._snapshotReplay = true; + retval = true; + } - _stateObj.AssertValidState(); + return retval; } - internal void PrepareReplay() + internal void MoveToStart() { - ResetSnapshotState(); + // go back to the beginning + _current = null; + MoveNext(); + _replayStateData.Restore(_stateObj); + _stateObj.AssertValidState(); } internal void CaptureAsStart(TdsParserStateObject stateObj) { + _firstPacket = null; + _lastPacket = null; + _current = null; + _stateObj = stateObj; _replayStateData ??= new StateObjectData(); _replayStateData.Capture(stateObj); @@ -1351,12 +1535,27 @@ internal void CaptureAsStart(TdsParserStateObject stateObj) Debug.Assert(stateObj._partialHeaderBytesRead == 0, "Has partially read header when snapshot taken"); #endif - PushBuffer(stateObj._inBuff, stateObj._inBytesRead); + AppendPacketData(stateObj._inBuff, stateObj._inBytesRead); + } + + internal void Clear() + { + ClearState(); + ClearPackets(); + } + + private void ClearPackets() + { + PacketData packet = _firstPacket; + _firstPacket = null; + _lastPacket = null; + _current = null; + packet.Clear(); + _sparePacket = packet; } - internal void ClearCore() + private void ClearState() { - _snapshotInBuffCurrent = 0; _replayStateData.Clear(_stateObj); #if DEBUG _rollingPend = 0;