Remove locking while computing our metadata indices for FAR and Add-Using#51747
Remove locking while computing our metadata indices for FAR and Add-Using#517475 commits merged intodotnet:mainfrom
Conversation
| // AsyncLazy would normally be an ok choice here. However, in the case where all clients | ||
| // cancel their request, we don't want ot keep the AsyncLazy around. It may capture a lot | ||
| // of immutable state (like a Solution) that we don't want kept around indefinitely. So we | ||
| // only cache results (the symbol tree infos) if they successfully compute to completion. |
There was a problem hiding this comment.
this comment is very relevant. we switched off of AsyncLazy because we woudl have AsyncLazys holding onto SOlution instances. However, in the new code, we do not hold onto solution instances. Instead, we hold onto a Workspace+SolutionKey. This is possible as the SOlution was only used to acquire an IPersistentStorage instance associated with that solution. But we don't need a full Solution object for that anymore with the current IPersistentStorage apis.
Note: if holding onto a workspace here is problematic, we could always key this first off of Workspace.
src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Metadata.cs
Outdated
Show resolved
Hide resolved
| createAsync: () => CreateSourceSymbolTreeInfoAsync(project, checksum, cancellationToken), | ||
| keySuffix: "_Source_" + project.FilePath, | ||
| tryReadObject: reader => TryReadSymbolTreeInfo(reader, checksum, nodes => GetSpellCheckerAsync(project.Solution, checksum, project.FilePath, nodes)), | ||
| tryReadObject: reader => TryReadSymbolTreeInfo(reader, checksum, nodes => GetSpellCheckerAsync(workspace, solutionKey, checksum, project.FilePath, nodes)), |
There was a problem hiding this comment.
Solution via project. If that is not desired, read project.FilePath to a local variable before this call.
There was a problem hiding this comment.
that's a good idea. it's not hugely important here (the task is kicked off immediately. but it's still good for hygeiene.
Fixes AB#1288263