Remove dispatcher from DocumentVersionCache #9026
Conversation
There was a problem hiding this comment.
In isolation this makes sense, so I'm clicking approve, but something that isn't clear to me is the impacts of moving this to have its own lock, versus previously when it would share the dispatcher (and the implied "lock" that gave) thread with all other project operations. Every single integration test failing does not fill me with confidence either 😛
Should the snapshot manager expose its lock for others to use?
src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultDocumentVersionCache.cs
Show resolved
Hide resolved
…tDocumentVersionCache.cs Co-authored-by: David Wengier <[email protected]>
I think not for now. The final straw would be if we got |
| private void TrackDocumentVersion(IDocumentSnapshot documentSnapshot, int version, string filePath, ReadWriterLocker.UpgradeableReadLock upgradeableReadLock) | ||
| { | ||
| // Need to ensure the write lock covers all uses of documentEntries, not just DocumentLookup | ||
| using (upgradeableReadLock.EnterWriteLock()) |
There was a problem hiding this comment.
This might just be me not understanding locks properly, but is there a reason why we're using using () syntax instead of using var _ syntax as we do in most other parts of this file? Is the intention to just be more explicit about the scope of the write lock?
There was a problem hiding this comment.
For write locks we're being a bit more explicit about where it is held, since they block all other operations.
ghost
added this to the
* upstream/main: (188 commits) Rename CSHTML file reference. (dotnet#8969) Remove Omnisharp logic from main branch (dotnet#9027) Update dependencies from https://github.com/dotnet/arcade build 20230726.1 Fixes CVE-2023-33127 and CVE-2023-33170 (dotnet#9032) Remove Async Keyword For Generate Async Method Code Action (dotnet#9030) Remove dispatcher from DocumentVersionCache (dotnet#9026) Restore perf work. (dotnet#8995) Implement priority trigger support Change implementations and references Rename ProjectSnapshotChangeTrigger and convert to interface Updates after merge Fix nullability Use pattern matching Convert to record struct Move CloseTextTagOnAutoInsertProvider to FindToken (dotnet#9025) Move GenerateMethodCodeActionProvider to FindToken (dotnet#8988) Add comment describing when ProjectRazorJson.Version should be incremented Some more violations, after the merge Remove TryResolveDocument method [Infra] 17.8 P1 snap PRs (dotnet#9021) ...
4 participants
Takes document version tracking off the dispatcher thread requirement and uses a locking mechanism instead.