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

Provide a synchronous path for outlining so it won't block hte UI thread waiting for a threadpool thread. #11705

Merged
merged 1 commit into from
Jun 3, 2016

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@CyrusNajmabadi
Copy link
Member Author

Tagging @dotnet/roslyn-ide . This helps address a regression where we are being asked to synchronously compute outlining spans on the UI thread, but we end up blocking the UI thread waiting for a threadpool thread to get around to processing the operation for us.

@CyrusNajmabadi
Copy link
Member Author

retest vsi please

@CyrusNajmabadi
Copy link
Member Author

@jasonmalinowski Plz review this for Ask mode.

@@ -227,6 +257,22 @@ public bool TryGetSyntaxRoot(out SyntaxNode root)
}

/// <summary>
/// Only for features that absolutely must run synchronously (probably because they're
/// on the UI thread). Right now, the only feature this is for is Outlining as VS will
Copy link
Member

Choose a reason for hiding this comment

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

Should we not have the "right now..." comment, since I can imagine that quickly getting out of sync? Is it worth auditing our codebase for a .GetSyntaxTreeAsync().Result and moving others over? I could imagine this would also have improvements for some automatic formatting stuff which might be worth getting the benefit of.

@jasonmalinowski
Copy link
Member

Overall, 👍. The caching of SyntaxTree tasks you've observed seems terribly fishy to me and I think might warrant further investigation, but clearly you didn't make that part worse.

Is it worth going through and seeing what other taggers can cheaply be moved over to this?

@CyrusNajmabadi CyrusNajmabadi merged commit d606041 into dotnet:stabilization Jun 3, 2016
@CyrusNajmabadi CyrusNajmabadi deleted the syncOutlining branch June 3, 2016 18:19
@CyrusNajmabadi CyrusNajmabadi restored the syncOutlining branch June 6, 2016 20:16
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