Skip to content

Conversation

@sharwell
Copy link
Contributor

@sharwell sharwell commented May 6, 2020

This change makes some tests faster locally; submitting this PR to see if it causes problems for any others.

@sharwell sharwell marked this pull request as ready for review May 6, 2020 19:55
@sharwell sharwell requested a review from a team as a code owner May 6, 2020 19:55
{
internal static Task WaitAllDispatcherOperationAndTasksAsync(this IAsynchronousOperationListenerProvider provider, params string[] featureNames)
=> ((AsynchronousOperationListenerProvider)provider).WaitAllAsync(featureNames, eventProcessingAction: () => Dispatcher.CurrentDispatcher.DoEvents());
=> ((AsynchronousOperationListenerProvider)provider).WaitAllAsync(featureNames);
Copy link
Member

Choose a reason for hiding this comment

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

The belief is the DoEvents isn't necessary because we should be yielding the thread no matter what given this is Task?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The DoEvents is not necessary because we're going through JTF which tracks UI thread work requests internally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

📝 I looked into simplifying this further, but I had less confidence in correctness as it fanned out. This seems like a reasonable intermediate perf win.

@sharwell sharwell merged commit 99317e2 into dotnet:master May 15, 2020
@ghost ghost added this to the Next milestone May 15, 2020
@sharwell sharwell deleted the rm-doevents branch May 15, 2020 16:12
@JoeRobich JoeRobich modified the milestones: Next, 16.7.P2 May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants