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

Commit d3d8c74

Browse files
Wraith2saurabh500
Wraith2
authored and
saurabh500
committed
Optimize SqlClient tds state to remove handle boxing (#34044)
* change TdsParserStateObject to pass packets using a ref struct to avoid boxing of IntPtr in native mode * add project define for FEATURE_INTEROPSNI on windows non uap builds * update interop to use SniPacketHandle type name * rename SNIPacketHandle to SNIPacket * split PacketHandle and SessionHandle into separate files and implementations * add comments to PacketHandle and SessionHandle remove unused packethandle variable in IsConnectionAlive remove identidal overridden implementations of EmptyReadHandle * move lazy bool into debug region * re-add EmptyReadPackt and provide correctly types valid but empty packets in implementations define IsValidPacket implementations more stringently with type checks * change implementation switch name to make more sense * add packet type assertion in IsValiePacket
1 parent a8a1957 commit d3d8c74

13 files changed

+313
-115
lines changed

‎src/System.Data.SqlClient/src/System.Data.SqlClient.csproj

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<ProjectGuid>{D4550556-4745-457F-BA8F-3EBF3836D6B4}</ProjectGuid>
44
<AssemblyName>System.Data.SqlClient</AssemblyName>
@@ -280,6 +280,8 @@
280280
<!-- Manage the SNI toggle for Windows netstandard and UWP -->
281281
<Compile Include="System\Data\SqlClient\SNI\SNITcpHandle.Windows.cs" />
282282
<Compile Include="System\Data\SqlClient\TdsParserStateObjectFactory.Windows.cs" />
283+
<Compile Include="System\Data\SqlClient\PacketHandle.Windows.cs" />
284+
<Compile Include="System\Data\SqlClient\SessionHandle.Windows.cs" />
283285
<AdditionalFiles Include="$(MSBuildProjectDirectory)/*.analyzerdata.windows" />
284286
</ItemGroup>
285287
<ItemGroup Condition="'$(IsUAPAssembly)' == 'true'">
@@ -289,6 +291,8 @@
289291
<Compile Include="System\Data\SqlClient\SNI\LocalDB.uap.cs" />
290292
<Compile Include="System\Data\ProviderBase\DbConnectionPoolIdentity.Unix.cs" />
291293
<Compile Include="System\Data\SqlClient\TdsParser.Unix.cs" />
294+
<Compile Include="System\Data\SqlClient\PacketHandle.Unix.cs" />
295+
<Compile Include="System\Data\SqlClient\SessionHandle.Unix.cs" />
292296
</ItemGroup>
293297
<!-- Assets needed on Windows but should be avoided on UAP to avoid sni.dll -->
294298
<ItemGroup Condition=" '$(TargetsWindows)' == 'true' And '$(IsPartialFacadeAssembly)' != 'true' and '$(IsUAPAssembly)' != 'true'">
@@ -483,6 +487,8 @@
483487
<Compile Include="System\Data\SqlClient\LocalDBAPI.Unix.cs" />
484488
<Compile Include="System\Data\SqlClient\SNI\LocalDB.Unix.cs" />
485489
<Compile Include="System\Data\SqlClient\SNI\SNITcpHandle.Unix.cs" />
490+
<Compile Include="System\Data\SqlClient\PacketHandle.Unix.cs" />
491+
<Compile Include="System\Data\SqlClient\SessionHandle.Unix.cs" />
486492
</ItemGroup>
487493
<ItemGroup Condition="'$(TargetsWindows)' == 'true' And '$(IsPartialFacadeAssembly)' != 'true' and '$(IsUAPAssembly)' != 'true'">
488494
<Reference Include="Microsoft.Win32.Registry" />

‎src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Unix.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
6+
namespace System.Data.SqlClient
7+
{
8+
// this structure is used for transporting packet handle references between the TdsParserStateObject
9+
// base class and Managed or Native implementations.
10+
// It prevents the native IntPtr type from being boxed and prevents the need to cast from object which loses compile time type safety
11+
// It carries type information so that assertions about the type of handle can be made in the implemented abstract methods
12+
// it is a ref struct so that it can only be used to transport the handles and not store them
13+
14+
// N.B. If you change this type you must also change the version for the other platform
15+
16+
internal readonly ref struct PacketHandle
17+
{
18+
public const int NativePointerType = 1;
19+
public const int NativePacketType = 2;
20+
public const int ManagedPacketType = 3;
21+
22+
public readonly SNI.SNIPacket ManagedPacket;
23+
public readonly int Type;
24+
25+
private PacketHandle(SNI.SNIPacket managedPacket, int type)
26+
{
27+
Type = type;
28+
ManagedPacket = managedPacket;
29+
}
30+
31+
public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) => new PacketHandle(managedPacket, ManagedPacketType);
32+
}
33+
}

‎src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Windows.cs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
6+
namespace System.Data.SqlClient
7+
{
8+
// this structure is used for transporting packet handle references between the TdsParserStateObject
9+
// base class and Managed or Native implementations.
10+
// It prevents the native IntPtr type from being boxed and prevents the need to cast from object which loses compile time type safety
11+
// It carries type information so that assertions about the type of handle can be made in the implemented abstract methods
12+
// it is a ref struct so that it can only be used to transport the handles and not store them
13+
14+
// N.B. If you change this type you must also change the version for the other platform
15+
16+
internal readonly ref struct PacketHandle
17+
{
18+
public const int NativePointerType = 1;
19+
public const int NativePacketType = 2;
20+
public const int ManagedPacketType = 3;
21+
22+
public readonly IntPtr NativePointer;
23+
public readonly SNIPacket NativePacket;
24+
25+
public readonly SNI.SNIPacket ManagedPacket;
26+
public readonly int Type;
27+
28+
private PacketHandle(IntPtr nativePointer, SNIPacket nativePacket, SNI.SNIPacket managedPacket, int type)
29+
{
30+
Type = type;
31+
ManagedPacket = managedPacket;
32+
NativePointer = nativePointer;
33+
NativePacket = nativePacket;
34+
}
35+
36+
public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) => new PacketHandle(default, default, managedPacket, ManagedPacketType);
37+
38+
public static PacketHandle FromNativePointer(IntPtr nativePointer) => new PacketHandle(nativePointer, default, default, NativePointerType);
39+
40+
public static PacketHandle FromNativePacket(SNIPacket nativePacket) => new PacketHandle(default, nativePacket, default, NativePacketType);
41+
42+
43+
}
44+
}

‎src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ public void HandleReceiveError(SNIPacket packet)
318318
_packetEvent.Set();
319319
}
320320

321-
((TdsParserStateObject)_callbackObject).ReadAsyncCallback(packet, 1);
321+
((TdsParserStateObject)_callbackObject).ReadAsyncCallback(PacketHandle.FromManagedPacket(packet), 1);
322322
}
323323

324324
/// <summary>
@@ -332,7 +332,7 @@ public void HandleSendComplete(SNIPacket packet, uint sniErrorCode)
332332
{
333333
Debug.Assert(_callbackObject != null);
334334

335-
((TdsParserStateObject)_callbackObject).WriteAsyncCallback(packet, sniErrorCode);
335+
((TdsParserStateObject)_callbackObject).WriteAsyncCallback(PacketHandle.FromManagedPacket(packet), sniErrorCode);
336336
}
337337
}
338338

@@ -378,7 +378,7 @@ public void HandleReceiveComplete(SNIPacket packet, SNISMUXHeader header)
378378
_asyncReceives--;
379379
Debug.Assert(_callbackObject != null);
380380

381-
((TdsParserStateObject)_callbackObject).ReadAsyncCallback(packet, 0);
381+
((TdsParserStateObject)_callbackObject).ReadAsyncCallback(PacketHandle.FromManagedPacket(packet), 0);
382382
}
383383
}
384384

‎src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ internal partial class SNIPacket : IDisposable, IEquatable<SNIPacket>
2020
private int _offset;
2121
private string _description;
2222
private SNIAsyncCallback _completionCallback;
23-
24-
private ArrayPool<byte> _arrayPool = ArrayPool<byte>.Shared;
2523
private bool _isBufferFromArrayPool = false;
2624

2725
public SNIPacket() { }
@@ -98,14 +96,14 @@ public void Allocate(int capacity)
9896
{
9997
if (_isBufferFromArrayPool)
10098
{
101-
_arrayPool.Return(_data);
99+
ArrayPool<byte>.Shared.Return(_data);
102100
}
103101
_data = null;
104102
}
105103

106104
if (_data == null)
107105
{
108-
_data = _arrayPool.Rent(capacity);
106+
_data = ArrayPool<byte>.Shared.Rent(capacity);
109107
_isBufferFromArrayPool = true;
110108
}
111109

@@ -221,7 +219,7 @@ public void Release()
221219
{
222220
if(_isBufferFromArrayPool)
223221
{
224-
_arrayPool.Return(_data);
222+
ArrayPool<byte>.Shared.Return(_data);
225223
}
226224
_data = null;
227225
_capacity = 0;

‎src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Unix.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
6+
namespace System.Data.SqlClient
7+
{
8+
// this structure is used for transporting packet handle references between the TdsParserStateObject
9+
// base class and Managed or Native implementations.
10+
// It carries type information so that assertions about the type of handle can be made in the
11+
// implemented abstract methods
12+
// it is a ref struct so that it can only be used to transport the handles and not store them
13+
14+
// N.B. If you change this type you must also change the version for the other platform
15+
16+
internal readonly ref struct SessionHandle
17+
{
18+
public const int NativeHandleType = 1;
19+
public const int ManagedHandleType = 2;
20+
21+
public readonly SNI.SNIHandle ManagedHandle;
22+
public readonly int Type;
23+
24+
public SessionHandle(SNI.SNIHandle managedHandle, int type)
25+
{
26+
Type = type;
27+
ManagedHandle = managedHandle;
28+
}
29+
30+
public bool IsNull => ManagedHandle is null;
31+
32+
public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) => new SessionHandle(managedSessionHandle, ManagedHandleType);
33+
}
34+
}

‎src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Windows.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
6+
namespace System.Data.SqlClient
7+
{
8+
// this structure is used for transporting packet handle references between the TdsParserStateObject
9+
// base class and Managed or Native implementations.
10+
// It carries type information so that assertions about the type of handle can be made in the
11+
// implemented abstract methods
12+
// it is a ref struct so that it can only be used to transport the handles and not store them
13+
14+
// N.B. If you change this type you must also change the version for the other platform
15+
16+
internal readonly ref struct SessionHandle
17+
{
18+
public const int NativeHandleType = 1;
19+
public const int ManagedHandleType = 2;
20+
21+
public readonly SNI.SNIHandle ManagedHandle;
22+
public readonly SNIHandle NativeHandle;
23+
24+
public readonly int Type;
25+
26+
public SessionHandle(SNI.SNIHandle managedHandle, SNIHandle nativeHandle, int type)
27+
{
28+
Type = type;
29+
ManagedHandle = managedHandle;
30+
NativeHandle = nativeHandle;
31+
}
32+
33+
public bool IsNull => (Type == NativeHandleType) ? NativeHandle is null : ManagedHandle is null;
34+
35+
public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) => new SessionHandle(managedSessionHandle, default, ManagedHandleType);
36+
37+
public static SessionHandle FromNativeHandle(SNIHandle nativeSessionHandle) => new SessionHandle(default, nativeSessionHandle, NativeHandleType);
38+
}
39+
}

‎src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.Windows.cs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,22 @@ internal void PostReadAsyncForMars()
2020
// Have to post read to initialize MARS - will get callback on this when connection goes
2121
// down or is closed.
2222

23-
IntPtr temp = IntPtr.Zero;
23+
PacketHandle temp = default;
2424
uint error = TdsEnums.SNI_SUCCESS;
2525

2626
_pMarsPhysicalConObj.IncrementPendingCallbacks();
27-
object handle = _pMarsPhysicalConObj.SessionHandle;
28-
temp = (IntPtr)_pMarsPhysicalConObj.ReadAsync(out error, ref handle);
27+
SessionHandle handle = _pMarsPhysicalConObj.SessionHandle;
28+
temp = _pMarsPhysicalConObj.ReadAsync(handle, out error);
2929

30-
if (temp != IntPtr.Zero)
30+
Debug.Assert(temp.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer");
31+
32+
if (temp.NativePointer != IntPtr.Zero)
3133
{
3234
// Be sure to release packet, otherwise it will be leaked by native.
3335
_pMarsPhysicalConObj.ReleasePacket(temp);
3436
}
35-
36-
Debug.Assert(IntPtr.Zero == temp, "unexpected syncReadPacket without corresponding SNIPacketRelease");
37+
38+
Debug.Assert(IntPtr.Zero == temp.NativePointer, "unexpected syncReadPacket without corresponding SNIPacketRelease");
3739
if (TdsEnums.SNI_SUCCESS_IO_PENDING != error)
3840
{
3941
Debug.Assert(TdsEnums.SNI_SUCCESS != error, "Unexpected successful read async on physical connection before enabling MARS!");
@@ -118,4 +120,4 @@ private SNIErrorDetails GetSniErrorDetails()
118120
}
119121

120122
} // tdsparser
121-
}//namespace
123+
}//namespace

‎src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserSafeHandles.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ private static void ReadDispatcher(IntPtr key, IntPtr packet, uint error)
104104

105105
if (null != stateObj)
106106
{
107-
stateObj.ReadAsyncCallback(IntPtr.Zero, packet, error);
107+
stateObj.ReadAsyncCallback(IntPtr.Zero, PacketHandle.FromNativePointer(packet), error);
108108
}
109109
}
110110
}
@@ -125,7 +125,7 @@ private static void WriteDispatcher(IntPtr key, IntPtr packet, uint error)
125125

126126
if (null != stateObj)
127127
{
128-
stateObj.WriteAsyncCallback(IntPtr.Zero, packet, error);
128+
stateObj.WriteAsyncCallback(IntPtr.Zero, PacketHandle.FromNativePointer(packet), error);
129129
}
130130
}
131131
}
@@ -296,4 +296,4 @@ public void Dispose()
296296
}
297297
}
298298
}
299-
}
299+
}

0 commit comments

Comments
 (0)