Skip to content

Conversation

lifengl
Copy link
Member

@lifengl lifengl commented Mar 29, 2024

…CancellationToken) call.

Those appeared a lot during the solution open time, because some global AsyncLazy doesn't provide value until the blocking loading time ends. And cancellation token is often used to handle the case loading is aborted.

The reason why we didn't eliminate the duplication earlier is the JTF.JoinAsync and task.WithCancellation handles in-middle task continuation differently. The JTF one generally will capture the SynchorizationContext, while the task one would not. The exact performance charcter of the two choices is different. The JTF one prevent additional dependency to the thread pool in some cases, but may add unnecessary dependency to the UI thread in other cases, while the other one is on the reverse side. So eliminating the two chains could impact perf, because the accidental UI thread dependency is more likely to cause performance problems than the other one.

It is impossible to decide which behavior is better, because the best choice should be aligned to the ConfiguredAwaiter option to wait the task later. But provide this extra option would make API more complex. In this case, we try to make the behavior matching the exisiting GetValueAsync API. Capturing the context makes little sense for the task which is forgotten in the original code path.

This is an image of those extra tasks in a common solution loading session. We can see all of them waiting the same asyncLazy, and we have another set of almost same task chains to wait on the inner task. They are often chained to similar cancellation token, which bloats the cancellation token registration data structure.

image

…CancellationToken) call.

Those appeared a lot during the solution open time, because some global AsyncLazy doesn't provide value until the blocking loading time ends. And cancellation token is often used to handle the case loading is aborted.

The reason why we didn't eliminate the duplication earlier is the JTF.JoinAsync and task.WithCancellation handles in-middle task continuation differently. The JTF one generally will capture the SynchorizationContext, while the task one would not. The exact performance charcter of the two choices is different. The JTF one prevent additional dependency to the thread pool in some cases, but may add unnecessary dependency to the UI thread in other cases, while the other one is on the reverse side. So eliminating the two chains could impact perf, because the accidental UI thread dependency is more likely to cause performance problems than the other one.

It is impossible to decide which behavior is better, because the best choice should be aligned to the ConfiguredAwaiter option to wait the task later. But provide this extra option would make API more complex. In this case, we try to make the behavior matching the exisiting GetValueAsync API. Capturing the context makes little sense for the task which is forgotten in the original code path.
@lifengl lifengl requested a review from AArnott March 29, 2024 19:08
Copy link
Member

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Thank you for the well-written description and thoughtful change.

@AArnott AArnott added this to the v17.11 milestone Apr 6, 2024
@AArnott
Copy link
Member

AArnott commented Apr 6, 2024

@lifengl let's merge this after #1298.

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.

2 participants