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

Add Task.ConfigureAwaitRunInline extension method #292

Merged
merged 1 commit into from
Jun 7, 2018

Conversation

AArnott
Copy link
Member

@AArnott AArnott commented Jun 6, 2018

This makes it easy to avoid a dependency on the UI thread or the threadpool to complete async methods that have very little work to do after a Task completes.

.ConfigureAwait(false) does not inline continuations when SynchronizationContext.Current != null (when the antecedent is completed).
.ConfigureAwait(true) does not inline continuations unless SynchronizationContext.Current is the same instance from when the continuation was created to when the antecedent completes.

Yet .ConfigureAwaitRunInline() will always inline the continuation so long as the TPL feels there is enough stack space available. Otherwise it will fallback to the threadpool.

@AArnott AArnott added this to the v15.8 milestone Jun 6, 2018
@AArnott AArnott self-assigned this Jun 6, 2018
@AArnott AArnott requested review from sharwell and lifengl June 6, 2018 00:45
@AArnott
Copy link
Member Author

AArnott commented Jun 6, 2018

My expectation is that this makes #291 a much simpler change.

/// <summary>
/// A Task awaitable that has affinity to executing callbacks synchronously on the completing callstack.
/// </summary>
public struct ExecuteContinuationSynchronouslyAwaitable
Copy link
Member

Choose a reason for hiding this comment

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

💡 readonly struct can apply to all these structures

Copy link
Member Author

Choose a reason for hiding this comment

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

That's an interesting idea. Since we have many awaiters in this library and I presume all of them would benefit, I'm going to save that for a different PR and apply after some testing.

/// <summary>
/// The task whose completion will execute the continuation.
/// </summary>
private readonly Task<T> antecedent;
Copy link
Member

Choose a reason for hiding this comment

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

❔ Why hold the Task<T> here instead of just the awaiter?

Copy link
Member Author

Choose a reason for hiding this comment

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

I could. I don't see the difference though. It couldn't possibly be smaller than holding a Task, but it could theoretically be bigger. I might research that, but it would seem I have nothing to gain by switching it. Are you aware of something that I'm missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I couldn't just use the awaiter. The key point of these awaiters is to override the behavior of TaskAwaiter. I can't do that if I'm tunneling through TaskAwaiter. I have to actually schedule a task continuation explicitly in order to set TaskContinuationOptions.ExecuteSynchronously, which I cannot do via TaskAwaiter.OnCompleted.

@lifengl
Copy link
Member

lifengl commented Jun 6, 2018 via email

This makes it easy to avoid a dependency on the UI thread or the threadpool to complete async methods that have very little work to do after a Task completes.

.ConfigureAwait(false) does *not* inline continuations when SynchronizationContext.Current != null (when the antecedent is completed).
.ConfigureAwait(true) does *not* inline continuations unless SynchronizationContext.Current is the same instance from when the continuation was created to when the antecedent completes.

Yet .ConfigureAwaitRunInline() will *always* inline the continuation so long as the TPL feels there is enough stack space available. Otherwise it will fallback to the threadpool.
@AArnott
Copy link
Member Author

AArnott commented Jun 6, 2018

@viktorveis @lifengl This is to assist with #291. Is this an approved fix for 15.8? Please let me know when it is and we can complete this.

@AArnott AArnott merged commit 996db06 into microsoft:master Jun 7, 2018
@AArnott AArnott deleted the InlineContinuation branch June 7, 2018 02:46
AArnott pushed a commit that referenced this pull request Oct 30, 2024
Bumps [dotnet-coverage](https://github.com/microsoft/codecoverage) from 17.12.4 to 17.12.5.
- [Commits](https://github.com/microsoft/codecoverage/commits)

---
updated-dependencies:
- dependency-name: dotnet-coverage
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants