Skip to content

Commit 3ca3984

Browse files
authored
Perf: Add managed packet recycling (#389)
1 parent 1395901 commit 3ca3984

File tree

12 files changed

+346
-174
lines changed

12 files changed

+346
-174
lines changed

src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,8 @@
343343
<Compile Include="Microsoft\Data\SqlClient\SNI\SNIMarsQueuedPacket.cs" />
344344
<Compile Include="Microsoft\Data\SqlClient\SNI\SNINpHandle.cs" />
345345
<Compile Include="Microsoft\Data\SqlClient\SNI\SNIPacket.cs" />
346+
<Compile Include="Microsoft\Data\SqlClient\SNI\SNIPacketPool.cs" />
347+
<Compile Include="Microsoft\Data\SqlClient\SNI\SNIPhysicalHandle.cs" />
346348
<Compile Include="Microsoft\Data\SqlClient\SNI\SNIProxy.cs" />
347349
<Compile Include="Microsoft\Data\SqlClient\SNI\SNITcpHandle.cs" />
348350
<Compile Include="Microsoft\Data\SqlClient\SNI\SslOverTdsStream.cs" />

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIHandle.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,9 @@ internal abstract class SNIHandle
4040
/// Send a packet asynchronously
4141
/// </summary>
4242
/// <param name="packet">SNI packet</param>
43-
/// <param name="disposePacketAfterSendAsync"></param>
4443
/// <param name="callback">Completion callback</param>
4544
/// <returns>SNI error code</returns>
46-
public abstract uint SendAsync(SNIPacket packet, bool disposePacketAfterSendAsync, SNIAsyncCallback callback = null);
45+
public abstract uint SendAsync(SNIPacket packet, SNIAsyncCallback callback = null);
4746

4847
/// <summary>
4948
/// Receive a packet synchronously
@@ -87,6 +86,11 @@ internal abstract class SNIHandle
8786
public abstract Guid ConnectionId { get; }
8887

8988
public virtual int ReserveHeaderSize => 0;
89+
90+
public abstract SNIPacket RentPacket(int headerSize, int dataSize);
91+
92+
public abstract void ReturnPacket(SNIPacket packet);
93+
9094
#if DEBUG
9195
/// <summary>
9296
/// Test handle for killing underlying connection

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIMarsConnection.cs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public uint SendAsync(SNIPacket packet, SNIAsyncCallback callback)
115115
{
116116
lock (this)
117117
{
118-
return _lowerHandle.SendAsync(packet, false, callback);
118+
return _lowerHandle.SendAsync(packet, callback);
119119
}
120120
}
121121
finally
@@ -136,7 +136,7 @@ public uint ReceiveAsync(ref SNIPacket packet)
136136
{
137137
if (packet != null)
138138
{
139-
packet.Release();
139+
ReturnPacket(packet);
140140
packet = null;
141141
}
142142

@@ -188,7 +188,8 @@ public void HandleReceiveError(SNIPacket packet)
188188
handle.HandleReceiveError(packet);
189189
}
190190
}
191-
packet?.Release();
191+
Debug.Assert(!packet.IsInvalid, "packet was returned by MarsConnection child, child sessions should not release the packet");
192+
ReturnPacket(packet);
192193
}
193194

194195
/// <summary>
@@ -258,7 +259,7 @@ public void HandleReceiveComplete(SNIPacket packet, uint sniErrorCode)
258259
_currentHeader.Read(_headerBytes);
259260

260261
_dataBytesLeft = (int)_currentHeader.length;
261-
_currentPacket = new SNIPacket(headerSize: 0, dataSize: (int)_currentHeader.length);
262+
_currentPacket = _lowerHandle.RentPacket(headerSize: 0, dataSize: (int)_currentHeader.length);
262263
}
263264

264265
currentHeader = _currentHeader;
@@ -322,6 +323,11 @@ public void HandleReceiveComplete(SNIPacket packet, uint sniErrorCode)
322323
{
323324
SNICommon.ReportSNIError(SNIProviders.SMUX_PROV, SNICommon.InternalExceptionError, e);
324325
}
326+
327+
Debug.Assert(_currentPacket == currentPacket, "current and _current are not the same");
328+
ReturnPacket(currentPacket);
329+
currentPacket = null;
330+
_currentPacket = null;
325331
}
326332

327333
lock (this)
@@ -379,6 +385,16 @@ public void DisableSsl()
379385
}
380386
}
381387

388+
public SNIPacket RentPacket(int headerSize, int dataSize)
389+
{
390+
return _lowerHandle.RentPacket(headerSize, dataSize);
391+
}
392+
393+
public void ReturnPacket(SNIPacket packet)
394+
{
395+
_lowerHandle.ReturnPacket(packet);
396+
}
397+
382398
#if DEBUG
383399
/// <summary>
384400
/// Test handle for killing underlying connection

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIMarsHandle.cs

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ internal sealed class SNIMarsHandle : SNIHandle
2626
private readonly ManualResetEventSlim _packetEvent = new ManualResetEventSlim(false);
2727
private readonly ManualResetEventSlim _ackEvent = new ManualResetEventSlim(false);
2828
private readonly SNISMUXHeader _currentHeader = new SNISMUXHeader();
29+
private readonly SNIAsyncCallback _handleSendCompleteCallback;
2930

3031
private uint _sendHighwater = 4;
3132
private int _asyncReceives = 0;
@@ -79,6 +80,7 @@ public SNIMarsHandle(SNIMarsConnection connection, ushort sessionId, object call
7980
_sessionId = sessionId;
8081
_connection = connection;
8182
_callbackObject = callbackObject;
83+
_handleSendCompleteCallback = HandleSendComplete;
8284
SendControlPacket(SNISMUXFlags.SMUX_SYN);
8385
_status = TdsEnums.SNI_SUCCESS;
8486
}
@@ -92,7 +94,7 @@ private void SendControlPacket(SNISMUXFlags flags)
9294
long scopeID = SqlClientEventSource.Log.SNIScopeEnterEvent("<sc.SNI.SNIMarsHandle.SendControlPacket |SNI|INFO|SCOPE>");
9395
try
9496
{
95-
SNIPacket packet = new SNIPacket(headerSize: SNISMUXHeader.HEADER_LENGTH, dataSize: 0);
97+
SNIPacket packet = RentPacket(headerSize: SNISMUXHeader.HEADER_LENGTH, dataSize: 0);
9698

9799
lock (this)
98100
{
@@ -102,6 +104,7 @@ private void SendControlPacket(SNISMUXFlags flags)
102104
}
103105

104106
_connection.Send(packet);
107+
ReturnPacket(packet);
105108
}
106109
finally
107110
{
@@ -263,17 +266,16 @@ private uint SendPendingPackets()
263266
/// Send a packet asynchronously
264267
/// </summary>
265268
/// <param name="packet">SNI packet</param>
266-
/// <param name="disposePacketAfterSendAsync"></param>
267269
/// <param name="callback">Completion callback</param>
268270
/// <returns>SNI error code</returns>
269-
public override uint SendAsync(SNIPacket packet, bool disposePacketAfterSendAsync, SNIAsyncCallback callback = null)
271+
public override uint SendAsync(SNIPacket packet, SNIAsyncCallback callback = null)
270272
{
271273
long scopeID = SqlClientEventSource.Log.SNIScopeEnterEvent("<sc.SNI.SNIMarsHandle.SendAsync |SNI|INFO|SCOPE>");
272274
try
273275
{
274276
lock (this)
275277
{
276-
_sendPacketQueue.Enqueue(new SNIMarsQueuedPacket(packet, callback != null ? callback : HandleSendComplete));
278+
_sendPacketQueue.Enqueue(new SNIMarsQueuedPacket(packet, callback ?? _handleSendCompleteCallback));
277279
}
278280

279281
SendPendingPackets();
@@ -340,13 +342,17 @@ public void HandleReceiveError(SNIPacket packet)
340342
long scopeID = SqlClientEventSource.Log.SNIScopeEnterEvent("<sc.SNI.SNIMarsHandle.HandleReceiveError |SNI|INFO|SCOPE>");
341343
try
342344
{
345+
// SNIMarsHandle should only receive calls to this function from the SNIMarsConnection aggregator class
346+
// which should handle ownership of the packet because the individual mars handles are not aware of
347+
// each other and cannot know if they are the last one in the list and that it is safe to return the packet
348+
343349
lock (_receivedPacketQueue)
344350
{
345351
_connectionError = SNILoadHandle.SingletonInstance.LastError;
346352
_packetEvent.Set();
347353
}
348354

349-
((TdsParserStateObject)_callbackObject).ReadAsyncCallback(PacketHandle.FromManagedPacket(packet), 1);
355+
((TdsParserStateObject)_callbackObject).ReadAsyncCallback(PacketHandle.FromManagedPacket(packet), 1);
350356
}
351357
finally
352358
{
@@ -370,6 +376,7 @@ public void HandleSendComplete(SNIPacket packet, uint sniErrorCode)
370376

371377
((TdsParserStateObject)_callbackObject).WriteAsyncCallback(PacketHandle.FromManagedPacket(packet), sniErrorCode);
372378
}
379+
_connection.ReturnPacket(packet);
373380
}
374381
finally
375382
{
@@ -432,6 +439,8 @@ public void HandleReceiveComplete(SNIPacket packet, SNISMUXHeader header)
432439

433440
((TdsParserStateObject)_callbackObject).ReadAsyncCallback(PacketHandle.FromManagedPacket(packet), 0);
434441
}
442+
443+
_connection.ReturnPacket(packet);
435444
}
436445

437446
lock (this)
@@ -556,21 +565,14 @@ public override void SetBufferSize(int bufferSize)
556565
{
557566
}
558567

559-
/// <summary>
560-
/// Enable SSL
561-
/// </summary>
562-
public override uint EnableSsl(uint options)
563-
{
564-
return _connection.EnableSsl(options);
565-
}
568+
public override uint EnableSsl(uint options) => _connection.EnableSsl(options);
569+
570+
public override void DisableSsl() => _connection.DisableSsl();
571+
572+
public override SNIPacket RentPacket(int headerSize, int dataSize) => _connection.RentPacket(headerSize, dataSize);
573+
574+
public override void ReturnPacket(SNIPacket packet) => _connection.ReturnPacket(packet);
566575

567-
/// <summary>
568-
/// Disable SSL
569-
/// </summary>
570-
public override void DisableSsl()
571-
{
572-
_connection.DisableSsl();
573-
}
574576

575577
#if DEBUG
576578
/// <summary>

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIMarsQueuedPacket.cs

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ namespace Microsoft.Data.SqlClient.SNI
77
/// <summary>
88
/// Mars queued packet
99
/// </summary>
10-
internal class SNIMarsQueuedPacket
10+
internal sealed class SNIMarsQueuedPacket
1111
{
12-
private SNIPacket _packet;
13-
private SNIAsyncCallback _callback;
12+
private readonly SNIPacket _packet;
13+
private readonly SNIAsyncCallback _callback;
1414

1515
/// <summary>
1616
/// Constructor
@@ -23,36 +23,8 @@ public SNIMarsQueuedPacket(SNIPacket packet, SNIAsyncCallback callback)
2323
_callback = callback;
2424
}
2525

26-
/// <summary>
27-
/// SNI packet
28-
/// </summary>
29-
public SNIPacket Packet
30-
{
31-
get
32-
{
33-
return _packet;
34-
}
35-
36-
set
37-
{
38-
_packet = value;
39-
}
40-
}
26+
public SNIPacket Packet => _packet;
4127

42-
/// <summary>
43-
/// Completion callback
44-
/// </summary>
45-
public SNIAsyncCallback Callback
46-
{
47-
get
48-
{
49-
return _callback;
50-
}
51-
52-
set
53-
{
54-
_callback = value;
55-
}
56-
}
28+
public SNIAsyncCallback Callback => _callback;
5729
}
5830
}

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace Microsoft.Data.SqlClient.SNI
1616
/// <summary>
1717
/// Named Pipe connection handle
1818
/// </summary>
19-
internal sealed class SNINpHandle : SNIHandle
19+
internal sealed class SNINpHandle : SNIPhysicalHandle
2020
{
2121
internal const string DefaultPipePath = @"sql\query"; // e.g. \\HOSTNAME\pipe\sql\query
2222
private const int MAX_PIPE_INSTANCES = 255;
@@ -179,7 +179,7 @@ public override uint Receive(out SNIPacket packet, int timeout)
179179
packet = null;
180180
try
181181
{
182-
packet = new SNIPacket(headerSize: 0, dataSize: _bufferSize);
182+
packet = RentPacket(headerSize: 0, dataSize: _bufferSize);
183183
packet.ReadFromStream(_stream);
184184

185185
if (packet.Length == 0)
@@ -220,7 +220,7 @@ public override uint ReceiveAsync(ref SNIPacket packet)
220220
try
221221
{
222222
SNIPacket errorPacket;
223-
packet = new SNIPacket(headerSize: 0, dataSize: _bufferSize);
223+
packet = RentPacket(headerSize: 0, dataSize: _bufferSize);
224224

225225
try
226226
{
@@ -307,13 +307,13 @@ public override uint Send(SNIPacket packet)
307307
}
308308
}
309309

310-
public override uint SendAsync(SNIPacket packet, bool disposePacketAfterSendAsync, SNIAsyncCallback callback = null)
310+
public override uint SendAsync(SNIPacket packet, SNIAsyncCallback callback = null)
311311
{
312312
long scopeID = SqlClientEventSource.Log.SNIScopeEnterEvent("<sc.SNI.SNINpHandle.SendAsync |SNI|SCOPE>");
313313
try
314314
{
315315
SNIAsyncCallback cb = callback ?? _sendCallback;
316-
packet.WriteToStreamAsync(_stream, cb, SNIProviders.NP_PROV, disposePacketAfterSendAsync);
316+
packet.WriteToStreamAsync(_stream, cb, SNIProviders.NP_PROV);
317317
return TdsEnums.SNI_SUCCESS_IO_PENDING;
318318
}
319319
finally
@@ -407,7 +407,7 @@ private uint ReportErrorAndReleasePacket(SNIPacket packet, Exception sniExceptio
407407
{
408408
if (packet != null)
409409
{
410-
packet.Release();
410+
ReturnPacket(packet);
411411
}
412412
return SNICommon.ReportSNIError(SNIProviders.NP_PROV, SNICommon.InternalExceptionError, sniException);
413413
}
@@ -416,7 +416,7 @@ private uint ReportErrorAndReleasePacket(SNIPacket packet, uint nativeError, uin
416416
{
417417
if (packet != null)
418418
{
419-
packet.Release();
419+
ReturnPacket(packet);
420420
}
421421
return SNICommon.ReportSNIError(SNIProviders.NP_PROV, nativeError, sniError, errorMessage);
422422
}

0 commit comments

Comments
 (0)