Skip to content
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.

Sign up for GitHub

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

Move to instantiating and setting up the language service on a bg thread #73351

Merged
merged 13 commits into from
May 6, 2024
Merged

Move to instantiating and setting up the language service on a bg thread #73351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3fd4789
Defer instantiating debugger related feature components until needed
CyrusNajmabadi May 6, 2024
b70f2a1
Move to instantiating and setting up the language service on a bg thread
CyrusNajmabadi May 6, 2024
9bbab46
Remove teardown logic
CyrusNajmabadi May 6, 2024
410b47f
Move into constructor
CyrusNajmabadi May 6, 2024
987d373
Assert
CyrusNajmabadi May 6, 2024
92d23c2
Make private
CyrusNajmabadi May 6, 2024
2958760
rename
CyrusNajmabadi May 6, 2024
be8f1be
move props
CyrusNajmabadi May 6, 2024
bf83d82
lint
CyrusNajmabadi May 6, 2024
1d84147
Usings
CyrusNajmabadi May 6, 2024
945de16
inline
CyrusNajmabadi May 6, 2024
27c0bca
Merge branch 'removeUnusedType' into bgLanguageService2
CyrusNajmabadi May 6, 2024
3e35578
Merge remote-tracking branch 'upstream/main' into bgLanguageService
CyrusNajmabadi May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.IVsLanguageDebugInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ int IVsLanguageDebugInfo.GetLanguageID(IVsTextBuffer pBuffer, int iLine, int iCo
{
try
{
return LanguageDebugInfo.GetLanguageID(pBuffer, iLine, iCol, out pguidLanguageID);
return _languageDebugInfo.GetLanguageID(pBuffer, iLine, iCol, out pguidLanguageID);
}
catch (Exception e) when (FatalError.ReportAndPropagate(e))
{
Expand All @@ -32,7 +32,7 @@ int IVsLanguageDebugInfo.GetLocationOfName(string pszName, out string pbstrMkDoc
{
try
{
return LanguageDebugInfo.GetLocationOfName(pszName, out pbstrMkDoc, out pspanLocation);
return _languageDebugInfo.GetLocationOfName(pszName, out pbstrMkDoc, out pspanLocation);
}
catch (Exception e) when (FatalError.ReportAndPropagate(e))
{
Expand All @@ -44,7 +44,7 @@ int IVsLanguageDebugInfo.GetNameOfLocation(IVsTextBuffer pBuffer, int iLine, int
{
try
{
return LanguageDebugInfo.GetNameOfLocation(pBuffer, iLine, iCol, out pbstrName, out piLineOffset);
return _languageDebugInfo.GetNameOfLocation(pBuffer, iLine, iCol, out pbstrName, out piLineOffset);
}
catch (Exception e) when (FatalError.ReportAndPropagate(e))
{
Expand All @@ -56,7 +56,7 @@ int IVsLanguageDebugInfo.GetProximityExpressions(IVsTextBuffer pBuffer, int iLin
{
try
{
return LanguageDebugInfo.GetProximityExpressions(pBuffer, iLine, iCol, cLines, out ppEnum);
return _languageDebugInfo.GetProximityExpressions(pBuffer, iLine, iCol, cLines, out ppEnum);
}
catch (Exception e) when (FatalError.ReportAndPropagate(e))
{
Expand All @@ -68,7 +68,7 @@ int IVsLanguageDebugInfo.IsMappedLocation(IVsTextBuffer pBuffer, int iLine, int
{
try
{
return LanguageDebugInfo.IsMappedLocation(pBuffer, iLine, iCol);
return _languageDebugInfo.IsMappedLocation(pBuffer, iLine, iCol);
}
catch (Exception e) when (FatalError.ReportAndPropagate(e))
{
Expand All @@ -80,7 +80,7 @@ int IVsLanguageDebugInfo.ResolveName(string pszName, uint dwFlags, out IVsEnumDe
{
try
{
return LanguageDebugInfo.ResolveName(pszName, dwFlags, out ppNames);
return _languageDebugInfo.ResolveName(pszName, dwFlags, out ppNames);
}
catch (Exception e) when (FatalError.ReportAndPropagate(e))
{
Expand All @@ -92,7 +92,7 @@ int IVsLanguageDebugInfo.ValidateBreakpointLocation(IVsTextBuffer pBuffer, int i
{
try
{
return LanguageDebugInfo.ValidateBreakpointLocation(pBuffer, iLine, iCol, pCodeSpan);
return _languageDebugInfo.ValidateBreakpointLocation(pBuffer, iLine, iCol, pCodeSpan);
}
catch (Exception e) when (FatalError.ReportAndPropagate(e))
{
Expand Down
113 changes: 34 additions & 79 deletions src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using System;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
Expand All @@ -19,13 +21,12 @@
using Microsoft.CodeAnalysis.Workspaces.ProjectSystem;
using Microsoft.VisualStudio.Editor;
using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem;
using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList;
using Microsoft.VisualStudio.LanguageServices.Implementation.Venus;
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Shell.Interop;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Outlining;
using Microsoft.VisualStudio.TextManager.Interop;
using Microsoft.VisualStudio.Threading;
using Microsoft.VisualStudio.Utilities;
using Roslyn.Utilities;

Expand All @@ -36,7 +37,8 @@ internal abstract partial class AbstractLanguageService<TPackage, TLanguageServi
where TLanguageService : AbstractLanguageService<TPackage, TLanguageService>
{
internal TPackage Package { get; }
internal VsLanguageDebugInfo LanguageDebugInfo { get; private set; }

private readonly VsLanguageDebugInfo _languageDebugInfo;

// DevDiv 753309:
// We've redefined some VS interfaces that had incorrect PIAs. When
Expand All @@ -56,9 +58,9 @@ internal abstract partial class AbstractLanguageService<TPackage, TLanguageServi
// Note: The lifetime for state in this class is carefully managed. For every bit of state
// we set up, there is a corresponding tear down phase which deconstructs the state in the
// reverse order it was created in.
internal EditorOptionsService EditorOptionsService { get; private set; }
internal VisualStudioWorkspaceImpl Workspace { get; private set; }
internal IVsEditorAdaptersFactoryService EditorAdaptersFactoryService { get; private set; }
internal readonly EditorOptionsService EditorOptionsService;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal readonly EditorOptionsService EditorOptionsService;

by init'ing these in the ctor and not nulling them out, could this file be made nullable without too much effort now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. will do in followup.

internal readonly VisualStudioWorkspaceImpl Workspace;
internal readonly IVsEditorAdaptersFactoryService EditorAdaptersFactoryService;

/// <summary>
/// Whether or not we have been set up. This is set once everything is wired up and cleared once tear down has begun.
Expand All @@ -70,9 +72,21 @@ internal abstract partial class AbstractLanguageService<TPackage, TLanguageServi
/// </remarks>
private bool _isSetUp;

protected abstract string ContentTypeName { get; }
protected abstract string LanguageName { get; }
protected abstract string RoslynLanguageName { get; }
protected abstract Guid DebuggerLanguageId { get; }

protected AbstractLanguageService(TPackage package)
{
Package = package;

Debug.Assert(!this.Package.JoinableTaskFactory.Context.IsOnMainThread, "Language service should be instantiated on background thread");

this.EditorOptionsService = this.Package.ComponentModel.GetService<EditorOptionsService>();
this.Workspace = this.Package.ComponentModel.GetService<VisualStudioWorkspaceImpl>();
this.EditorAdaptersFactoryService = this.Package.ComponentModel.GetService<IVsEditorAdaptersFactoryService>();
this._languageDebugInfo = CreateLanguageDebugInfo();
sharwell marked this conversation as resolved.
Show resolved Hide resolved
}

public override IServiceProvider SystemServiceProvider
Expand All @@ -81,32 +95,28 @@ public override IServiceProvider SystemServiceProvider
/// <summary>
/// Setup and TearDown go in reverse order.
/// </summary>
internal void Setup()
public async Task SetupAsync(CancellationToken cancellationToken)
{
this.ComAggregate = CreateComAggregate();

// First, acquire any services we need throughout our lifetime.
this.GetServices();
// This method should only contain calls to acquire services off of the component model
// or service providers. Anything else which is more complicated should go in Initialize
// instead.

// TODO: Is the below access to component model required or can be removed?
_ = this.Package.ComponentModel;
// Start off a background task to prime some components we'll need for editing.
Task.Run(() =>
{
var formatter = this.Workspace.Services.GetLanguageServices(RoslynLanguageName).GetService<ISyntaxFormattingService>();
formatter?.GetDefaultFormattingRules();
}, cancellationToken).Forget();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to normal Task.Run here. Could switch back to VsTaskLibraryHelper. Need to understand the value of that type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fire and forget operations need to be tracked


// Start off a background task to prime some components we'll need for editing
VsTaskLibraryHelper.CreateAndStartTask(VsTaskLibraryHelper.ServiceInstance, VsTaskRunContext.BackgroundThread,
() => PrimeLanguageServiceComponentsOnBackground());
await this.Package.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);

// Finally, once our connections are established, set up any initial state that we need.
// Note: we may be instantiated at any time (including when the IDE is already
// debugging). We must not assume anything about our initial state and must instead
// query for all the information we need at this point.
this.Initialize();
// Creating the com aggregate has to happen on the UI thread.
this.ComAggregate = Interop.ComAggregate.CreateAggregatedObject(this);

_isSetUp = true;
}

private object CreateComAggregate()
=> Interop.ComAggregate.CreateAggregatedObject(this);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inlined this.


internal void TearDown()
{
if (!_isSetUp)
Expand All @@ -116,9 +126,6 @@ internal void TearDown()

_isSetUp = false;
GC.SuppressFinalize(this);

this.Uninitialize();
this.RemoveServices();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for these. all they did was null out managed objects. something unnecessary with managed types and disposal.

}

~AbstractLanguageService()
Expand All @@ -129,49 +136,6 @@ internal void TearDown()
}
}

protected virtual void GetServices()
{
// This method should only contain calls to acquire services off of the component model
// or service providers. Anything else which is more complicated should go in Initialize
// instead.
this.EditorOptionsService = this.Package.ComponentModel.GetService<EditorOptionsService>();
this.Workspace = this.Package.ComponentModel.GetService<VisualStudioWorkspaceImpl>();
this.EditorAdaptersFactoryService = this.Package.ComponentModel.GetService<IVsEditorAdaptersFactoryService>();
}

protected virtual void RemoveServices()
{
this.EditorAdaptersFactoryService = null;
this.Workspace = null;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't seem to serve any purpose. nulling out these simple managed types serves no purpose.


/// <summary>
/// Called right after we instantiate the language service. Used to set up any internal
/// state we need.
///
/// Try to keep this method fairly clean. Any complicated logic should go in methods called
/// from this one. Initialize and Uninitialize go in reverse order
/// </summary>
protected virtual void Initialize()
{
InitializeLanguageDebugInfo();
}

protected virtual void Uninitialize()
{
UninitializeLanguageDebugInfo();
}

private void PrimeLanguageServiceComponentsOnBackground()
{
var formatter = this.Workspace.Services.GetLanguageServices(RoslynLanguageName).GetService<ISyntaxFormattingService>();
formatter?.GetDefaultFormattingRules();
}

protected abstract string ContentTypeName { get; }
protected abstract string LanguageName { get; }
protected abstract string RoslynLanguageName { get; }

protected virtual void SetupNewTextView(IVsTextView textView)
{
Contract.ThrowIfNull(textView);
Expand Down Expand Up @@ -253,15 +217,9 @@ private void ConditionallyCollapseOutliningRegions(IVsTextView textView, IWpfTex
}
}

private void InitializeLanguageDebugInfo()
=> this.LanguageDebugInfo = this.CreateLanguageDebugInfo();

protected abstract Guid DebuggerLanguageId { get; }

private VsLanguageDebugInfo CreateLanguageDebugInfo()
{
var workspace = this.Workspace;
var languageServices = workspace.Services.GetLanguageServices(RoslynLanguageName);
var languageServices = this.Workspace.Services.GetLanguageServices(RoslynLanguageName);

return new VsLanguageDebugInfo(
this.DebuggerLanguageId,
Expand All @@ -271,9 +229,6 @@ private VsLanguageDebugInfo CreateLanguageDebugInfo()
this.Package.ComponentModel.GetService<IUIThreadOperationExecutor>());
}

private void UninitializeLanguageDebugInfo()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this uninitialize stuff doesn't appear to serve any purpose. nulling out simple managed types doesnt' buy us anything.

=> this.LanguageDebugInfo = null;

protected virtual IVsContainedLanguage CreateContainedLanguage(
IVsTextBufferCoordinator bufferCoordinator, ProjectSystemProject project,
IVsHierarchy hierarchy, uint itemid)
Expand Down
8 changes: 5 additions & 3 deletions src/VisualStudio/Core/Def/LanguageService/AbstractPackage`2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,16 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke
RegisterEditorFactory(editorFactory);
}

RegisterLanguageService(typeof(TLanguageService), async ct =>
RegisterLanguageService(typeof(TLanguageService), async cancellationToken =>
{
await JoinableTaskFactory.SwitchToMainThreadAsync(ct);
// Ensure we're on the BG when creating the language service.
await TaskScheduler.Default;

// Create the language service, tell it to set itself up, then store it in a field
// so we can notify it that it's time to clean up.
_languageService = CreateLanguageService();
_languageService.Setup();
await _languageService.SetupAsync(cancellationToken).ConfigureAwait(false);

return _languageService.ComAggregate;
});

Expand Down
Loading