Move to instantiating and setting up the language service on a bg thread #73351
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| this.EditorOptionsService = this.Package.ComponentModel.GetService<EditorOptionsService>(); | ||
| this.Workspace = this.Package.ComponentModel.GetService<VisualStudioWorkspaceImpl>(); | ||
| this.EditorAdaptersFactoryService = this.Package.ComponentModel.GetService<IVsEditorAdaptersFactoryService>(); | ||
| this.AnalyzerFileWatcherService = this.Package.ComponentModel.GetService<AnalyzerFileWatcherService>(); |
There was a problem hiding this comment.
inlined GetServices
|
|
||
| // TODO: Is the below access to component model required or can be removed? | ||
| _ = this.Package.ComponentModel; | ||
| this.LanguageDebugInfo = CreateLanguageDebugInfo(); |
There was a problem hiding this comment.
moved this to here so it can happen on the BG, instead of on the UI thread when the debugger calls into us.
| { | ||
| var formatter = this.Workspace.Services.GetLanguageServices(RoslynLanguageName).GetService<ISyntaxFormattingService>(); | ||
| formatter?.GetDefaultFormattingRules(); | ||
| }, cancellationToken).Forget(); |
There was a problem hiding this comment.
moved to normal Task.Run here. Could switch back to VsTaskLibraryHelper. Need to understand the value of that type.
There was a problem hiding this comment.
Fire and forget operations need to be tracked
|
|
||
| _isSetUp = true; | ||
| } | ||
|
|
||
| private object CreateComAggregate() | ||
| => Interop.ComAggregate.CreateAggregatedObject(this); |
CyrusNajmabadi
changed the title
| @@ -273,9 +232,6 @@ private VsLanguageDebugInfo CreateLanguageDebugInfo() | |||
| this.Package.ComponentModel.GetService<IUIThreadOperationExecutor>()); | |||
| } | |||
|
|
|||
| private void UninitializeLanguageDebugInfo() | |||
There was a problem hiding this comment.
this uninitialize stuff doesn't appear to serve any purpose. nulling out simple managed types doesnt' buy us anything.
| { | ||
| this.EditorAdaptersFactoryService = null; | ||
| this.Workspace = null; | ||
| } |
There was a problem hiding this comment.
this doesn't seem to serve any purpose. nulling out these simple managed types serves no purpose.
| this.EditorOptionsService = this.Package.ComponentModel.GetService<EditorOptionsService>(); | ||
| this.Workspace = this.Package.ComponentModel.GetService<VisualStudioWorkspaceImpl>(); | ||
| this.EditorAdaptersFactoryService = this.Package.ComponentModel.GetService<IVsEditorAdaptersFactoryService>(); | ||
| this.AnalyzerFileWatcherService = this.Package.ComponentModel.GetService<AnalyzerFileWatcherService>(); |
There was a problem hiding this comment.
inlined into constructor.
| @@ -117,9 +128,6 @@ internal void TearDown() | |||
|
|
|||
| _isSetUp = false; | |||
| GC.SuppressFinalize(this); | |||
|
|
|||
| this.Uninitialize(); | |||
| this.RemoveServices(); | |||
There was a problem hiding this comment.
no need for these. all they did was null out managed objects. something unnecessary with managed types and disposal.
| internal VisualStudioWorkspaceImpl Workspace { get; private set; } | ||
| internal IVsEditorAdaptersFactoryService EditorAdaptersFactoryService { get; private set; } | ||
| internal AnalyzerFileWatcherService AnalyzerFileWatcherService { get; private set; } | ||
| internal readonly EditorOptionsService EditorOptionsService; |
There was a problem hiding this comment.
yes. will do in followup.
|
@jasonmalinowski For review when you get back. |
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1812707