Skip to content

make transaction inner tasks use RunContinuationsAsynchronously - #950

Merged
mgravell merged 2 commits into
masterfrom
transaction-inner-sync
Sep 12, 2018
Merged

make transaction inner tasks use RunContinuationsAsynchronously#950
mgravell merged 2 commits into
masterfrom
transaction-inner-sync

Conversation

@mgravell

Copy link
Copy Markdown
Collaborator

For discussion, see issue #943

  • make transaction inner tasks use RunContinuationsAsynchronously
  • detect RunContinuationsAsynchronously in ResultBox completion, and set directly rather than deferred

…discussion, see issue #943); detect RunContinuationsAsynchronously in ResultBox completion, and set directly rather than deferred

@NickCraver NickCraver left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good since we’re on 46+ across the board now - IMO removing the TaskSource helper layer that’s not adding any logic would help code readability and reasoning, but no strong feelings about it either.

=> new TaskCompletionSource<T>(asyncState, TaskCreationOptions.None);
/// <param name="options">The options to apply to the task</param>
public static TaskCompletionSource<T> Create<T>(object asyncState, TaskCreationOptions options = TaskCreationOptions.None)
=> new TaskCompletionSource<T>(asyncState, options);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FWIW, we could delete this helper now that it’s a wrap with no logic over built-in.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@NickCraver I'd rather keep it all in one place even if it is just inlined by the JIT; I might add a JIT hint, but frankly I'd expect it to inline like a beast

@mgravell
mgravell merged commit 4949fc4 into master Sep 12, 2018
@mgravell
mgravell deleted the transaction-inner-sync branch September 12, 2018 10:11

@ZackaryJacobthereal ZackaryJacobthereal left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Z.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants