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

Reduce allocation in query for execution strategy #24208

Merged
merged 2 commits into from
Feb 23, 2021
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
58 changes: 32 additions & 26 deletions src/EFCore.Relational/Query/Internal/FromSqlQueryingEnumerable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public bool MoveNext()
if (_dataReader == null)
{
_relationalQueryContext.ExecutionStrategyFactory.Create()
.Execute(true, InitializeReader, null);
.Execute(this, (_, enumerator) => InitializeReader(enumerator), null);
}

var hasNext = _dataReader!.Read();
Expand All @@ -220,26 +220,27 @@ public bool MoveNext()
}
}

private bool InitializeReader(DbContext _, bool result)
private static bool InitializeReader(Enumerator enumerator)
{
EntityFrameworkEventSource.Log.QueryExecuting();

var relationalCommand = _relationalCommandCache.GetRelationalCommand(_relationalQueryContext.ParameterValues);
var relationalCommand = enumerator._relationalCommandCache.GetRelationalCommand(
enumerator._relationalQueryContext.ParameterValues);

_dataReader = relationalCommand.ExecuteReader(
enumerator._dataReader = relationalCommand.ExecuteReader(
new RelationalCommandParameterObject(
_relationalQueryContext.Connection,
_relationalQueryContext.ParameterValues,
_relationalCommandCache.ReaderColumns,
_relationalQueryContext.Context,
_relationalQueryContext.CommandLogger,
_detailedErrorsEnabled));
enumerator._relationalQueryContext.Connection,
enumerator._relationalQueryContext.ParameterValues,
enumerator._relationalCommandCache.ReaderColumns,
enumerator._relationalQueryContext.Context,
enumerator._relationalQueryContext.CommandLogger,
enumerator._detailedErrorsEnabled));

_indexMap = BuildIndexMap(_columnNames, _dataReader.DbDataReader);
enumerator._indexMap = BuildIndexMap(enumerator._columnNames, enumerator._dataReader.DbDataReader);

_relationalQueryContext.InitializeStateManager(_standAloneStateManager);
enumerator._relationalQueryContext.InitializeStateManager(enumerator._standAloneStateManager);

return result;
return false;
}

public void Dispose()
Expand Down Expand Up @@ -297,7 +298,11 @@ public async ValueTask<bool> MoveNextAsync()
if (_dataReader == null)
{
await _relationalQueryContext.ExecutionStrategyFactory.Create()
.ExecuteAsync(true, InitializeReaderAsync, null, _relationalQueryContext.CancellationToken)
.ExecuteAsync(
this,
(_, enumerator, cancellationToken) => InitializeReaderAsync(enumerator, cancellationToken),
null,
_relationalQueryContext.CancellationToken)
.ConfigureAwait(false);
}

Expand All @@ -322,28 +327,29 @@ await _relationalQueryContext.ExecutionStrategyFactory.Create()
}
}

private async Task<bool> InitializeReaderAsync(DbContext _, bool result, CancellationToken cancellationToken)
private static async Task<bool> InitializeReaderAsync(AsyncEnumerator enumerator, CancellationToken cancellationToken)
{
EntityFrameworkEventSource.Log.QueryExecuting();

var relationalCommand = _relationalCommandCache.GetRelationalCommand(_relationalQueryContext.ParameterValues);
var relationalCommand = enumerator._relationalCommandCache.GetRelationalCommand(
enumerator._relationalQueryContext.ParameterValues);

_dataReader = await relationalCommand.ExecuteReaderAsync(
enumerator._dataReader = await relationalCommand.ExecuteReaderAsync(
new RelationalCommandParameterObject(
_relationalQueryContext.Connection,
_relationalQueryContext.ParameterValues,
_relationalCommandCache.ReaderColumns,
_relationalQueryContext.Context,
_relationalQueryContext.CommandLogger,
_detailedErrorsEnabled),
enumerator._relationalQueryContext.Connection,
enumerator._relationalQueryContext.ParameterValues,
enumerator._relationalCommandCache.ReaderColumns,
enumerator._relationalQueryContext.Context,
enumerator._relationalQueryContext.CommandLogger,
enumerator._detailedErrorsEnabled),
cancellationToken)
.ConfigureAwait(false);

_indexMap = BuildIndexMap(_columnNames, _dataReader.DbDataReader);
enumerator._indexMap = BuildIndexMap(enumerator._columnNames, enumerator._dataReader.DbDataReader);

_relationalQueryContext.InitializeStateManager(_standAloneStateManager);
enumerator._relationalQueryContext.InitializeStateManager(enumerator._standAloneStateManager);

return result;
return false;
}

public ValueTask DisposeAsync()
Expand Down
58 changes: 32 additions & 26 deletions src/EFCore.Relational/Query/Internal/SingleQueryingEnumerable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public bool MoveNext()
if (_dataReader == null)
{
_relationalQueryContext.ExecutionStrategyFactory.Create()
.Execute(true, InitializeReader, null);
.Execute(this, (_, enumerator) => InitializeReader(enumerator), null);
}

var hasNext = _resultCoordinator!.HasNext ?? _dataReader!.Read();
Expand Down Expand Up @@ -214,26 +214,27 @@ public bool MoveNext()
}
}

private bool InitializeReader(DbContext _, bool result)
private static bool InitializeReader(Enumerator enumerator)
smitpatel marked this conversation as resolved.
Show resolved Hide resolved
{
EntityFrameworkEventSource.Log.QueryExecuting();

var relationalCommand = _relationalCommandCache.GetRelationalCommand(_relationalQueryContext.ParameterValues);
var relationalCommand = enumerator._relationalCommandCache.GetRelationalCommand(
enumerator._relationalQueryContext.ParameterValues);

_dataReader = relationalCommand.ExecuteReader(
enumerator._dataReader = relationalCommand.ExecuteReader(
new RelationalCommandParameterObject(
_relationalQueryContext.Connection,
_relationalQueryContext.ParameterValues,
_relationalCommandCache.ReaderColumns,
_relationalQueryContext.Context,
_relationalQueryContext.CommandLogger,
_detailedErrorsEnabled));
enumerator._relationalQueryContext.Connection,
enumerator._relationalQueryContext.ParameterValues,
enumerator._relationalCommandCache.ReaderColumns,
enumerator._relationalQueryContext.Context,
enumerator._relationalQueryContext.CommandLogger,
enumerator._detailedErrorsEnabled));

_resultCoordinator = new SingleQueryResultCoordinator();
enumerator._resultCoordinator = new SingleQueryResultCoordinator();

_relationalQueryContext.InitializeStateManager(_standAloneStateManager);
enumerator._relationalQueryContext.InitializeStateManager(enumerator._standAloneStateManager);

return result;
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this is false?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was it true before? :trollface:

Doesn't matter, it's not used :) I can make it true if we care.

Copy link
Contributor

@smitpatel smitpatel Feb 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was it true before?

Don't know. :trollface:
I am a bot, I question for changes only disregarding correctness of old code. 😉

Can use whatever if the value is irrelevant. Curiosity why not void if value is not used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only because Execute/ExecuteAsync return TResult, and C# doesn't allow void.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have overloads which don't return TResult, but...

}

public void Dispose()
Expand Down Expand Up @@ -289,7 +290,11 @@ public async ValueTask<bool> MoveNextAsync()
if (_dataReader == null)
{
await _relationalQueryContext.ExecutionStrategyFactory.Create()
.ExecuteAsync(true, InitializeReaderAsync, null, _relationalQueryContext.CancellationToken)
.ExecuteAsync(
this,
(_, enumerator, cancellationToken) => InitializeReaderAsync(enumerator, cancellationToken),
null,
_relationalQueryContext.CancellationToken)
.ConfigureAwait(false);
}

Expand Down Expand Up @@ -342,28 +347,29 @@ await _relationalQueryContext.ExecutionStrategyFactory.Create()
}
}

private async Task<bool> InitializeReaderAsync(DbContext _, bool result, CancellationToken cancellationToken)
private static async Task<bool> InitializeReaderAsync(AsyncEnumerator enumerator, CancellationToken cancellationToken)
{
EntityFrameworkEventSource.Log.QueryExecuting();

var relationalCommand = _relationalCommandCache.GetRelationalCommand(_relationalQueryContext.ParameterValues);
var relationalCommand = enumerator._relationalCommandCache.GetRelationalCommand(
enumerator._relationalQueryContext.ParameterValues);

_dataReader = await relationalCommand.ExecuteReaderAsync(
enumerator._dataReader = await relationalCommand.ExecuteReaderAsync(
new RelationalCommandParameterObject(
_relationalQueryContext.Connection,
_relationalQueryContext.ParameterValues,
_relationalCommandCache.ReaderColumns,
_relationalQueryContext.Context,
_relationalQueryContext.CommandLogger,
_detailedErrorsEnabled),
enumerator._relationalQueryContext.Connection,
enumerator._relationalQueryContext.ParameterValues,
enumerator._relationalCommandCache.ReaderColumns,
enumerator._relationalQueryContext.Context,
enumerator._relationalQueryContext.CommandLogger,
enumerator._detailedErrorsEnabled),
cancellationToken)
.ConfigureAwait(false);

_resultCoordinator = new SingleQueryResultCoordinator();
enumerator._resultCoordinator = new SingleQueryResultCoordinator();

_relationalQueryContext.InitializeStateManager(_standAloneStateManager);
enumerator._relationalQueryContext.InitializeStateManager(enumerator._standAloneStateManager);

return result;
return false;
}

public ValueTask DisposeAsync()
Expand Down
58 changes: 32 additions & 26 deletions src/EFCore.Relational/Query/Internal/SplitQueryingEnumerable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public bool MoveNext()
_executionStrategy = _relationalQueryContext.ExecutionStrategyFactory.Create();
}

_executionStrategy.Execute(true, InitializeReader, null);
_executionStrategy.Execute(this, (_, enumerator) => InitializeReader(enumerator), null);
}

var hasNext = _dataReader!.Read();
Expand Down Expand Up @@ -208,26 +208,27 @@ public bool MoveNext()
}
}

private bool InitializeReader(DbContext _, bool result)
private static bool InitializeReader(Enumerator enumerator)
{
EntityFrameworkEventSource.Log.QueryExecuting();

var relationalCommand = _relationalCommandCache.GetRelationalCommand(_relationalQueryContext.ParameterValues);
var relationalCommand = enumerator._relationalCommandCache.GetRelationalCommand(
enumerator._relationalQueryContext.ParameterValues);

_dataReader = relationalCommand.ExecuteReader(
enumerator._dataReader = relationalCommand.ExecuteReader(
new RelationalCommandParameterObject(
_relationalQueryContext.Connection,
_relationalQueryContext.ParameterValues,
_relationalCommandCache.ReaderColumns,
_relationalQueryContext.Context,
_relationalQueryContext.CommandLogger,
_detailedErrorsEnabled));
enumerator._relationalQueryContext.Connection,
enumerator._relationalQueryContext.ParameterValues,
enumerator._relationalCommandCache.ReaderColumns,
enumerator._relationalQueryContext.Context,
enumerator._relationalQueryContext.CommandLogger,
enumerator._detailedErrorsEnabled));

_resultCoordinator = new SplitQueryResultCoordinator();
enumerator._resultCoordinator = new SplitQueryResultCoordinator();

_relationalQueryContext.InitializeStateManager(_standAloneStateManager);
enumerator._relationalQueryContext.InitializeStateManager(enumerator._standAloneStateManager);

return result;
return false;
}

public void Dispose()
Expand Down Expand Up @@ -303,7 +304,11 @@ public async ValueTask<bool> MoveNextAsync()
}

await _executionStrategy.ExecuteAsync(
true, InitializeReaderAsync, null, _relationalQueryContext.CancellationToken).ConfigureAwait(false);
this,
(_, enumerator, cancellationToken) => InitializeReaderAsync(enumerator, cancellationToken),
null,
_relationalQueryContext.CancellationToken)
.ConfigureAwait(false);
}

var hasNext = await _dataReader!.ReadAsync(_relationalQueryContext.CancellationToken).ConfigureAwait(false);
Expand Down Expand Up @@ -339,28 +344,29 @@ await _relatedDataLoaders(_relationalQueryContext, _executionStrategy!, _resultC
}
}

private async Task<bool> InitializeReaderAsync(DbContext _, bool result, CancellationToken cancellationToken)
private static async Task<bool> InitializeReaderAsync(AsyncEnumerator enumerator, CancellationToken cancellationToken)
{
EntityFrameworkEventSource.Log.QueryExecuting();

var relationalCommand = _relationalCommandCache.GetRelationalCommand(_relationalQueryContext.ParameterValues);
var relationalCommand = enumerator._relationalCommandCache.GetRelationalCommand(
enumerator._relationalQueryContext.ParameterValues);

_dataReader = await relationalCommand.ExecuteReaderAsync(
enumerator._dataReader = await relationalCommand.ExecuteReaderAsync(
new RelationalCommandParameterObject(
_relationalQueryContext.Connection,
_relationalQueryContext.ParameterValues,
_relationalCommandCache.ReaderColumns,
_relationalQueryContext.Context,
_relationalQueryContext.CommandLogger,
_detailedErrorEnabled),
enumerator._relationalQueryContext.Connection,
enumerator._relationalQueryContext.ParameterValues,
enumerator._relationalCommandCache.ReaderColumns,
enumerator._relationalQueryContext.Context,
enumerator._relationalQueryContext.CommandLogger,
enumerator._detailedErrorEnabled),
cancellationToken)
.ConfigureAwait(false);

_resultCoordinator = new SplitQueryResultCoordinator();
enumerator._resultCoordinator = new SplitQueryResultCoordinator();

_relationalQueryContext.InitializeStateManager(_standAloneStateManager);
enumerator._relationalQueryContext.InitializeStateManager(enumerator._standAloneStateManager);

return result;
return false;
}

public async ValueTask DisposeAsync()
Expand Down
Loading