Move MiscellaneousFilesWorkspace construction to background thread #78051
Conversation
… the main thread. Added comments about https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2441480 and created a tracking bug for us to clean this up once it's fixed (dotnet#78050)
ghost
added
Area-IDE
untriaged
ToddGrun
changed the title
|
I guess this is fine but we don't have clean main insertions yet, so maybe wait with this? cc @dibarbet as current Tiger |
|
@jasonmalinowski -- any remaining concerns? |
| private readonly IVsService<IVsTextManager> _textManagerService; | ||
| private readonly OpenTextBufferProvider _openTextBufferProvider; | ||
| private readonly IMetadataAsSourceFileService _fileTrackingMetadataAsSourceService; | ||
| private readonly Lazy<IMetadataAsSourceFileService> _fileTrackingMetadataAsSourceService; |
There was a problem hiding this comment.
Was this necessary because that code also has affinity or just to avoid paying extra cost during creation?
There was a problem hiding this comment.
This was also asked in the original PR. Generally, I'd prefer for lazy args for data not used immediately to delay construction.
There was a problem hiding this comment.
Agreed. I'm not sure if we should have some sort of team habit to add comments to that effect. Historically we've used Lazy to mostly mean "there's deep magic here good luck" but if we were all better about being lazy then it's really those special cases that should be commented.
src/VisualStudio/Core/Def/ProjectSystem/MiscellaneousFilesWorkspace.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/ProjectSystem/OpenTextBufferProvider.cs
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
4 participants
This PR restores behavior introduced in #77768 that were reverted in PR #77983.
The revert occurred as it was flagged for causing ddrit regressions in some of the WinformsVS64.Designer tests. I was unable to reproduce this scenario locally,
but the RPS tests are able to reliably hit this issue.
Digging into it a bit indicated that creating the OpenTextBufferProvider on a bg thread somehow a thread affinity that caused
microsoft.visualstudio.shell.design.ni!DocData.CreateNativeDocData's call to msenv!CEditorManager::RegisterInvisibleEditor's to require an RPC main->bg thread switch.
This breaks the assumptions of RegisterInvisibleEditor and weird things start happening. Jason helped track down the likely culprit, and logged
https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2441480 to track that work.
This PR works around that issue by obtaining the RDT while on the main thread. I've logged #78050 to track the Roslyn cleanup
necessary once obtaining the rdt can be done from a background thread.
Compare by commit:
Commit 1:
Restore changes from PR #77768 that were reverted in PR #77983
Commit 2:
a) Ensure OpenTextBufferProvider._runningDocumentTable is initialized on the main thread.
b) Added comments about https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2441480 and created a tracking bug for us to clean this up once it's fixed (#78050)