Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Move datareader caches down to internal connection #499

Merged
merged 1 commit into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -4696,7 +4694,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 @@ -4740,9 +4746,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 @@ -4762,7 +4768,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 @@ -4863,7 +4872,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 @@ -4899,7 +4916,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 @@ -5047,7 +5067,7 @@ internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendi

#endif

private class Snapshot
internal class Snapshot
{
public bool _dataReady;
public bool _haltRead;
Expand All @@ -5068,7 +5088,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 @@ -5111,7 +5131,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 @@ -5132,7 +5152,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 @@ -5381,7 +5401,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 @@ -5454,9 +5481,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 @@ -5496,9 +5523,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
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