-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Make finding checksum async #40091
Make finding checksum async #40091
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20/27 files reviewed so far
src/VisualStudio/Core/Def/Implementation/Remote/RemotableDataJsonRpcEx.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/Core/Portable/Workspace/Solution/StateChecksums.cs
Outdated
Show resolved
Hide resolved
@sharwell Would we want |
The overwhelming use of this method enters through FWIW, |
That's true, but making TextDocumentState.GetTextAsync would at least avoid some allocations until we figure out the new public API. |
@dotnet/roslyn-ide PTAL |
When we retrieve an object for Text checksum in
DocumentStateChecksums
we used a trick to avoid making Find asynchronous and save Task allocations. We placedDocumentState
to the result instead of the expectedSourceText
and retrieved the actualSourceText
instance later on, when we were in async context. We can avoid theTask
allocations by usingValueTask
, remove this special casing and simplifySolutionAsset
implementation.Add nullable annotations.
Move methods from
Microsoft.CodeAnalysis.Remote.Shared.Extensions
to TestUtils as they are only used for testing.Remove SolutionAssetManager and RemoteHostPanel. They were only used for experimenting and are no longer needed.