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.

Sign up for GitHub

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

Jump to bottom

Switch to returning ValueTask in loggers and interceptors #21109

Closed
roji opened this issue Jun 2, 2020 · 2 comments · Fixed by #21152
Closed

Switch to returning ValueTask in loggers and interceptors #21109

roji opened this issue Jun 2, 2020 · 2 comments · Fixed by #21152
Assignees
@roji
Labels
area-perf breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone
5.0.0

Comments

@roji
Copy link
Member

roji commented Jun 2, 2020 •
edited
Loading

Many of our loggers currently return generic Tasks in order to support interception, incurring lots of needless heap allocations. For example, RelationalCommand.ExecuteReaderAsync calls CommandReaderExecutingAsync and CommandReaderExecutedAsync, both of which actually return synchronously in almost all cases but allocate Task instances.

In addition, our interceptor API surface also returns generic Tasks from all methods. This means that the moment an interceptor is set up, the user starts paying additional allocations for all of its methods, regardless of whether those are intercepted or if they complete synchronously.

As this is a (minor) breaking change in a relatively new API, it may be good to do this early.
@roji roji self-assigned this Jun 2, 2020
@ajcvickers ajcvickers added this to the 5.0.0 milestone Jun 5, 2020
roji added a commit that referenced this issue Jun 5, 2020
@roji
Return ValueTask in loggers and interceptors
80fa236 
Closes #21109
@roji roji mentioned this issue Jun 5, 2020
Merged
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jun 5, 2020
roji added a commit that referenced this issue Jun 5, 2020
@roji
Return ValueTask in loggers and interceptors
6b59e8f 
Closes #21109
roji added a commit that referenced this issue Jun 6, 2020
@roji
Return ValueTask in loggers and interceptors (#21152)
9f316e5 
Closes #21109
roji added a commit that referenced this issue Jun 6, 2020
@roji
Return ValueTask in loggers and interceptors (#21152)
e44ce0c 
Closes #21109

(cherry picked from commit 9f316e5)
@roji roji mentioned this issue Jun 6, 2020
Merged
roji added a commit that referenced this issue Jun 6, 2020
@roji
Return ValueTask in loggers and interceptors (#21152)
48f3479 
Closes #21109

(cherry picked from commit 9f316e5)
roji added a commit that referenced this issue Jun 6, 2020
@roji
Return ValueTask in loggers and interceptors (#21152) (#21158)
869b4af 
Closes #21109

(cherry picked from commit 9f316e5)
wtgodbe pushed a commit that referenced this issue Jun 8, 2020
@dotnet-maestro-bot @dotnet-maestro
@BrennanConroy @roji
[automated] Merge branch 'release/5.0-preview6' => 'master' (#21162)
011e18c 
* [release/5.0-preview6] Update dependencies from dotnet/arcade (#21111)

* Update dependencies from https://github.com/dotnet/arcade build 20200530.1

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk
 From Version 5.0.0-beta.20261.9 -> To Version 5.0.0-beta.20280.1

* [master] Update dependencies from dotnet/arcade (#21091)

* Update dependencies from https://github.com/dotnet/arcade build 20200528.4

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk
 From Version 5.0.0-beta.20261.9 -> To Version 5.0.0-beta.20278.4

* Update dotnet on helix

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Brennan <[email protected]>

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Brennan <[email protected]>

* Return ValueTask in loggers and interceptors (#21152) (#21158)

Closes #21109

(cherry picked from commit 9f316e5)

Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Brennan <[email protected]>
Co-authored-by: Shay Rojansky <[email protected]>
wtgodbe pushed a commit that referenced this issue Jun 9, 2020
@dotnet-maestro-bot @dotnet-maestro
@BrennanConroy @roji
[automated] Merge branch 'release/5.0-preview6' => 'master' (#21193)
c5d04a0 
* [release/5.0-preview6] Update dependencies from dotnet/arcade (#21111)

* Update dependencies from https://github.com/dotnet/arcade build 20200530.1

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk
 From Version 5.0.0-beta.20261.9 -> To Version 5.0.0-beta.20280.1

* [master] Update dependencies from dotnet/arcade (#21091)

* Update dependencies from https://github.com/dotnet/arcade build 20200528.4

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk
 From Version 5.0.0-beta.20261.9 -> To Version 5.0.0-beta.20278.4

* Update dotnet on helix

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Brennan <[email protected]>

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Brennan <[email protected]>

* Return ValueTask in loggers and interceptors (#21152) (#21158)

Closes #21109

(cherry picked from commit 9f316e5)

* Add ConfigureAwait(false) (#21110) (#21185)

Set up Microsoft.CodeAnalysis.FxCopAnalyzers with only the
ConfigureAwait rule enabled.

Closes #10164

(cherry picked from commit e1c9a3a)

Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Brennan <[email protected]>
Co-authored-by: Shay Rojansky <[email protected]>
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview7 Jun 22, 2020
@ajcvickers ajcvickers removed this from the 5.0.0-preview7 milestone Jul 20, 2020
@roji
Copy link
Member Author

roji commented Jul 28, 2020

@ajcvickers milestone-deprived issue

@smitpatel smitpatel added this to the 5.0.0-preview6 milestone Jul 28, 2020
@ajcvickers ajcvickers reopened this Jul 29, 2020
@ajcvickers ajcvickers removed this from the 5.0.0-preview6 milestone Jul 29, 2020
@ajcvickers
Copy link
Contributor

@roji @smitpatel What I meant to do was remove the milestone and reopen the issue so we can discuss again in triage since it was already reported as a break.

@ajcvickers ajcvickers added this to the 5.0.0-preview7 milestone Jul 31, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview7, 5.0.0 Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@roji roji

Labels
area-perf breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Milestone
5.0.0
Development

Successfully merging a pull request may close this issue.

Return ValueTask in loggers and interceptors dotnet/efcore
3 participants
@ajcvickers @smitpatel @roji