Improve ContinueWith perf with NotOn* options#35575
Merged
stephentoub merged 5 commits intodotnet:masterfrom May 2, 2020
Merged
Improve ContinueWith perf with NotOn* options#35575stephentoub merged 5 commits intodotnet:masterfrom
stephentoub merged 5 commits intodotnet:masterfrom
Conversation
|
Tagging subscribers to this area: @tarekgh |
tarekgh
approved these changes
Apr 28, 2020
Member
Author
|
I removed the commit that removed the scheduler field from the continuation task; turns out our synchronous Wait inlining logic relied on that. I'll look at removing it again separately. |
The bCancelNonExecutingOnly argument was always false.
There's no point in these being separate, with one being small and only and always called from the other.
We can avoid unnecessarily instantiating the contingent properties bag, as well as a few interlocked operations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This came about while looking at allocations and CPU costs in some HttpClient code. When ContinueWith is used with TaskContinuationOptions.NotOn* options, when the antecedent task completes we compare the state of that antecedent task against the NotOn* options: if the options permit it, the continuation is queued/invoked, and if they don't, the continuation is canceled. That cancellation then ends up being common in cases where a ContinueWith is used, for example, to log exceptions that result from a faulted task, e.g.
We can handle that cancellation much more efficiently than we are today. Today that's resulting in us expanding the ContingentProperties inside of the task, in order to store that cancellation was requested internally, but that's not actually necessary. It's also resulting in us doing several atomic transitions via interlockeds, but that's only necessary if the task could have transitioned in any way, which is only possible if a cancelable token is provided.
This PR removes that overhead. It also shrinks the size of the object created by ContinueWith by a field, seals and renames it, and removes some dead code from related code paths.
cc: @kouvel, @tarekgh