Skip to content

Commit

Permalink
Perf: Move datareader caches down to internal connection (#499)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Wraith2
Wraith2 authored Jul 16, 2020
1 parent 5b8a602 commit 762d24f
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 24 deletions.
69 changes: 48 additions & 21 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace Microsoft.Data.SqlClient
/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml' path='docs/members[@name="SqlDataReader"]/SqlDataReader/*' />
public class SqlDataReader : DbDataReader, IDataReader, IDbColumnSchemaGenerator
{
private enum ALTROWSTATUS
internal enum ALTROWSTATUS
{
Null = 0, // default and after Done
AltRow, // after calling NextResult and the first AltRow is available for read
Expand Down Expand Up @@ -87,7 +87,7 @@ internal class SharedState

private Task _currentTask;
private Snapshot _snapshot;
private Snapshot _cachedSnapshot;

private CancellationTokenSource _cancelAsyncOnCloseTokenSource;
private CancellationToken _cancelAsyncOnCloseToken;

Expand All @@ -97,8 +97,6 @@ internal class SharedState

private SqlSequentialStream _currentStream;
private SqlSequentialTextReader _currentTextReader;
private IsDBNullAsyncCallContext _cachedIsDBNullContext;
private ReadAsyncCallContext _cachedReadAsyncContext;

internal SqlDataReader(SqlCommand command, CommandBehavior behavior)
{
Expand Down Expand Up @@ -3781,9 +3779,9 @@ private bool TryReadColumnInternal(int i, bool readHeaderOnly = false)
{
// reset snapshot to save memory use. We can safely do that here because all SqlDataReader values are stable.
// The retry logic can use the current values to get back to the right state.
if (_cachedSnapshot is null)
if (_connection?.InnerConnection is SqlInternalConnection sqlInternalConnection && sqlInternalConnection.CachedDataReaderSnapshot is null)
{
_cachedSnapshot = _snapshot;
sqlInternalConnection.CachedDataReaderSnapshot = _snapshot;
}
_snapshot = null;
PrepareAsyncInvocation(useSnapshot: true);
Expand Down Expand Up @@ -4705,7 +4703,15 @@ public override Task<bool> ReadAsync(CancellationToken cancellationToken)
registration = cancellationToken.Register(SqlCommand.s_cancelIgnoreFailure, _command);
}

var context = Interlocked.Exchange(ref _cachedReadAsyncContext, null) ?? new ReadAsyncCallContext();
ReadAsyncCallContext context = null;
if (_connection?.InnerConnection is SqlInternalConnection sqlInternalConnection)
{
context = Interlocked.Exchange(ref sqlInternalConnection.CachedDataReaderReadAsyncContext, null);
}
if (context is null)
{
context = new ReadAsyncCallContext();
}

Debug.Assert(context._reader == null && context._source == null && context._disposable == null, "cached ReadAsyncCallContext was not properly disposed");

Expand Down Expand Up @@ -4749,9 +4755,9 @@ private static Task<bool> ReadAsyncExecute(Task task, object state)
if (!hasReadRowToken)
{
hasReadRowToken = true;
if (reader._cachedSnapshot is null)
if (reader.Connection?.InnerConnection is SqlInternalConnection sqlInternalConnection && sqlInternalConnection.CachedDataReaderSnapshot is null)
{
reader._cachedSnapshot = reader._snapshot;
sqlInternalConnection.CachedDataReaderSnapshot = reader._snapshot;
}
reader._snapshot = null;
reader.PrepareAsyncInvocation(useSnapshot: true);
Expand All @@ -4771,7 +4777,10 @@ private static Task<bool> ReadAsyncExecute(Task task, object state)

private void SetCachedReadAsyncCallContext(ReadAsyncCallContext instance)
{
Interlocked.CompareExchange(ref _cachedReadAsyncContext, instance, null);
if (_connection?.InnerConnection is SqlInternalConnection sqlInternalConnection)
{
Interlocked.CompareExchange(ref sqlInternalConnection.CachedDataReaderReadAsyncContext, instance, null);
}
}

/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml' path='docs/members[@name="SqlDataReader"]/IsDBNullAsync/*' />
Expand Down Expand Up @@ -4872,7 +4881,15 @@ override public Task<bool> IsDBNullAsync(int i, CancellationToken cancellationTo
registration = cancellationToken.Register(SqlCommand.s_cancelIgnoreFailure, _command);
}

IsDBNullAsyncCallContext context = Interlocked.Exchange(ref _cachedIsDBNullContext, null) ?? new IsDBNullAsyncCallContext();
IsDBNullAsyncCallContext context = null;
if (_connection?.InnerConnection is SqlInternalConnection sqlInternalConnection)
{
context = Interlocked.Exchange(ref sqlInternalConnection.CachedDataReaderIsDBNullContext, null);
}
if (context is null)
{
context = new IsDBNullAsyncCallContext();
}

Debug.Assert(context._reader == null && context._source == null && context._disposable == null, "cached ISDBNullAsync context not properly disposed");

Expand Down Expand Up @@ -4908,7 +4925,10 @@ private static Task<bool> IsDBNullAsyncExecute(Task task, object state)

private void SetCachedIDBNullAsyncCallContext(IsDBNullAsyncCallContext instance)
{
Interlocked.CompareExchange(ref _cachedIsDBNullContext, instance, null);
if (_connection?.InnerConnection is SqlInternalConnection sqlInternalConnection)
{
Interlocked.CompareExchange(ref sqlInternalConnection.CachedDataReaderIsDBNullContext, instance, null);
}
}

/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml' path='docs/members[@name="SqlDataReader"]/GetFieldValueAsync/*' />
Expand Down Expand Up @@ -5056,7 +5076,7 @@ internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendi

#endif

private class Snapshot
internal class Snapshot
{
public bool _dataReady;
public bool _haltRead;
Expand All @@ -5077,7 +5097,7 @@ private class Snapshot
public SqlSequentialTextReader _currentTextReader;
}

private abstract class AAsyncCallContext<T> : IDisposable
internal abstract class AAsyncCallContext<T> : IDisposable
{
internal static readonly Action<Task<T>, object> s_completeCallback = SqlDataReader.CompleteAsyncCallCallback<T>;

Expand Down Expand Up @@ -5120,7 +5140,7 @@ public virtual void Dispose()
}
}

private sealed class ReadAsyncCallContext : AAsyncCallContext<bool>
internal sealed class ReadAsyncCallContext : AAsyncCallContext<bool>
{
internal static readonly Func<Task, object, Task<bool>> s_execute = SqlDataReader.ReadAsyncExecute;

Expand All @@ -5141,7 +5161,7 @@ public override void Dispose()
}
}

private sealed class IsDBNullAsyncCallContext : AAsyncCallContext<bool>
internal sealed class IsDBNullAsyncCallContext : AAsyncCallContext<bool>
{
internal static readonly Func<Task, object, Task<bool>> s_execute = SqlDataReader.IsDBNullAsyncExecute;

Expand Down Expand Up @@ -5390,7 +5410,14 @@ private void PrepareAsyncInvocation(bool useSnapshot)

if (_snapshot == null)
{
_snapshot = Interlocked.Exchange(ref _cachedSnapshot, null) ?? new Snapshot();
if (_connection?.InnerConnection is SqlInternalConnection sqlInternalConnection)
{
_snapshot = Interlocked.Exchange(ref sqlInternalConnection.CachedDataReaderSnapshot, null) ?? new Snapshot();
}
else
{
_snapshot = new Snapshot();
}

_snapshot._dataReady = _sharedState._dataReady;
_snapshot._haltRead = _haltRead;
Expand Down Expand Up @@ -5463,9 +5490,9 @@ private void CleanupAfterAsyncInvocationInternal(TdsParserStateObject stateObj,
stateObj._permitReplayStackTraceToDiffer = false;
#endif

if (_cachedSnapshot is null)
if (_connection?.InnerConnection is SqlInternalConnection sqlInternalConnection && sqlInternalConnection.CachedDataReaderSnapshot is null)
{
_cachedSnapshot = _snapshot;
sqlInternalConnection.CachedDataReaderSnapshot = _snapshot;
}
// We are setting this to null inside the if-statement because stateObj==null means that the reader hasn't been initialized or has been closed (either way _snapshot should already be null)
_snapshot = null;
Expand Down Expand Up @@ -5505,9 +5532,9 @@ private void SwitchToAsyncWithoutSnapshot()
Debug.Assert(_snapshot != null, "Should currently have a snapshot");
Debug.Assert(_stateObj != null && !_stateObj._asyncReadWithoutSnapshot, "Already in async without snapshot");

if (_cachedSnapshot is null)
if (_connection?.InnerConnection is SqlInternalConnection sqlInternalConnection && sqlInternalConnection.CachedDataReaderSnapshot is null)
{
_cachedSnapshot = _snapshot;
sqlInternalConnection.CachedDataReaderSnapshot = _snapshot;
}
_snapshot = null;
_stateObj.ResetSnapshot();
Expand Down
10 changes: 7 additions & 3 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,21 @@

namespace Microsoft.Data.SqlClient
{
abstract internal class SqlInternalConnection : DbConnectionInternal
internal abstract class SqlInternalConnection : DbConnectionInternal
{
private readonly SqlConnectionString _connectionOptions;
private bool _isEnlistedInTransaction; // is the server-side connection enlisted? true while we're enlisted, reset only after we send a null...
private byte[] _promotedDTCToken; // token returned by the server when we promote transaction
private byte[] _whereAbouts; // cache the whereabouts (DTC Address) for exporting

private bool _isGlobalTransaction = false; // Whether this is a Global Transaction (Non-MSDTC, Azure SQL DB Transaction)
private bool _isGlobalTransactionEnabledForServer = false; // Whether Global Transactions are enabled for this Azure SQL DB Server
private bool _isGlobalTransaction; // Whether this is a Global Transaction (Non-MSDTC, Azure SQL DB Transaction)
private bool _isGlobalTransactionEnabledForServer; // Whether Global Transactions are enabled for this Azure SQL DB Server
private static readonly Guid _globalTransactionTMID = new Guid("1c742caf-6680-40ea-9c26-6b6846079764"); // ID of the Non-MSDTC, Azure SQL DB Transaction Manager

internal SqlDataReader.Snapshot CachedDataReaderSnapshot;
internal SqlDataReader.IsDBNullAsyncCallContext CachedDataReaderIsDBNullContext;
internal SqlDataReader.ReadAsyncCallContext CachedDataReaderReadAsyncContext;

// if connection is not open: null
// if connection is open: currently active database
internal string CurrentDatabase { get; set; }
Expand Down

0 comments on commit 762d24f

Please sign in to comment.