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.

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

Expose async entrypoint for IWorkspaceProjectContextFactory #51138

Merged
merged 34 commits into from
Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
29bb62b
Expose async entrypoint for IWorkspaceProjectContextFactory
CyrusNajmabadi Feb 10, 2021
2171687
Update tesst
CyrusNajmabadi Feb 11, 2021
88a6598
Merge remote-tracking branch 'upstream/master' into asyncProjectFactory
CyrusNajmabadi Feb 11, 2021
cdf3f83
More JTF
CyrusNajmabadi Feb 11, 2021
fe8ca83
Cancellation
CyrusNajmabadi Feb 11, 2021
5b7bea8
Fix tests
CyrusNajmabadi Feb 12, 2021
539dded
Merge remote-tracking branch 'upstream/master' into asyncProjectFactory
CyrusNajmabadi Feb 13, 2021
392393a
Switch to async loading.
CyrusNajmabadi Feb 13, 2021
d46ea35
Add comment
CyrusNajmabadi Feb 13, 2021
82409be
More async
CyrusNajmabadi Feb 13, 2021
20670a5
pull solution code up
CyrusNajmabadi Feb 13, 2021
fb0c5dd
Inline
CyrusNajmabadi Feb 13, 2021
fd039ca
Updatecomment
CyrusNajmabadi Feb 13, 2021
7cd7d84
Update comment
CyrusNajmabadi Feb 13, 2021
7179d1a
Add comment
CyrusNajmabadi Feb 13, 2021
d3f9833
Remove methods
CyrusNajmabadi Feb 13, 2021
173f96f
more async
CyrusNajmabadi Feb 13, 2021
04fa8d3
null check
CyrusNajmabadi Feb 14, 2021
c31c935
reentrancy'
CyrusNajmabadi Feb 14, 2021
07d2183
Switch to ValueTask
CyrusNajmabadi Feb 14, 2021
d043723
Use CT.None
CyrusNajmabadi Feb 14, 2021
766fab9
Use CT.None
CyrusNajmabadi Feb 14, 2021
52039d9
Async service provider.
CyrusNajmabadi Feb 14, 2021
a2c5ca4
Async service provider.
CyrusNajmabadi Feb 14, 2021
ce53c66
Switch back to the UI thread.
CyrusNajmabadi Feb 14, 2021
56a0fc9
Switch back to UIthread
CyrusNajmabadi Feb 18, 2021
5f3c213
reorder
CyrusNajmabadi Feb 18, 2021
c73e66b
Update src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualS…
CyrusNajmabadi Feb 18, 2021
c701b66
rename
CyrusNajmabadi Feb 18, 2021
c7d4372
Move comment
CyrusNajmabadi Feb 18, 2021
6285480
Tweak
CyrusNajmabadi Feb 18, 2021
4bc6c1c
Docs
CyrusNajmabadi Feb 18, 2021
b1e2a6b
Revert
CyrusNajmabadi Feb 18, 2021
a5f1b03
REvise
CyrusNajmabadi Feb 18, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@
namespace Microsoft.VisualStudio.LanguageServices.ProjectSystem
{
/// <summary>
/// Project context to initialize properties and items of a Workspace project created with <see cref="IWorkspaceProjectContextFactory.CreateProjectContext(string, string, string, Guid, object, string)"/>.
/// Project context to initialize properties and items of a Workspace project created with <see
/// cref="IWorkspaceProjectContextFactory.CreateProjectContext(string, string, string, Guid, object, string)"/>.
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <remarks>
/// <see cref="IDisposable.Dispose"/> is safe to call on instances of this type on any thread.
Copy link
Member

Choose a reason for hiding this comment

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

❔ Can it be called while asynchronous operations are scheduled and/or in progress?

Copy link
Member Author

Choose a reason for hiding this comment

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

IWorkspaceProjectContext.cs itself doesn't do anything asynchronously. There is async work to create it, but once you have it, it's a sync object.

Copy link
Member

Choose a reason for hiding this comment

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

And yeah we'll be putting more async operations onto it at some point; so we'll need to sort that out...later.

Copy link
Member

Choose a reason for hiding this comment

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

Actually we should just make a proper ExternalAccess setup for CPS at that point.

/// </remarks>
internal interface IWorkspaceProjectContext : IDisposable
{
// Project properties.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#nullable disable

using System;
using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList;
using System.Threading.Tasks;

namespace Microsoft.VisualStudio.LanguageServices.ProjectSystem
{
Expand All @@ -14,6 +14,10 @@ namespace Microsoft.VisualStudio.LanguageServices.ProjectSystem
/// </summary>
internal interface IWorkspaceProjectContextFactory
{
/// <inheritdoc cref="CreateProjectContextAsync(string, string, string, Guid, object, string)"/>
[Obsolete("Use CreateProjectContextAsync instead")]
IWorkspaceProjectContext CreateProjectContext(string languageName, string projectUniqueName, string projectFilePath, Guid projectGuid, object hierarchy, string binOutputPath);

/// <summary>
/// Creates and initializes a new Workspace project and returns a <see cref="IWorkspaceProjectContext"/> to lazily initialize the properties and items for the project.
/// </summary>
Expand All @@ -23,6 +27,6 @@ internal interface IWorkspaceProjectContextFactory
/// <param name="projectGuid">Project guid.</param>
/// <param name="hierarchy">Obsolete. The argument is ignored.</param>
Copy link
Member

Choose a reason for hiding this comment

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

❔ Why keep this if it's just going to be ignored?

Copy link
Member Author

Choose a reason for hiding this comment

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

i can't tell i the comment is a lie or not. the value does seem to be used. i'd rather not change something that could subtly break things.

/// <param name="binOutputPath">Initial project binary output path.</param>
IWorkspaceProjectContext CreateProjectContext(string languageName, string projectUniqueName, string projectFilePath, Guid projectGuid, object hierarchy, string binOutputPath);
Task<IWorkspaceProjectContext> CreateProjectContextAsync(string languageName, string projectUniqueName, string projectFilePath, Guid projectGuid, object hierarchy, string binOutputPath);
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@
using System.Collections.Immutable;
using System.ComponentModel.Composition;
using System.IO;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.VisualStudio.LanguageServices.ExternalAccess.VSTypeScript.Api;
using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList;
using Microsoft.VisualStudio.Shell.Interop;
using Microsoft.VisualStudio.Telemetry;
using Microsoft.VisualStudio.Threading;

namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem
{
Expand All @@ -23,18 +26,20 @@ internal sealed class VisualStudioProjectFactory : IVsTypeScriptVisualStudioProj
{
private const string SolutionContextName = "Solution";
private const string SolutionSessionIdPropertyName = "SolutionSessionID";

private readonly IThreadingContext _threadingContext;
private readonly VisualStudioWorkspaceImpl _visualStudioWorkspaceImpl;
private readonly HostDiagnosticUpdateSource _hostDiagnosticUpdateSource;
private readonly ImmutableArray<Lazy<IDynamicFileInfoProvider, FileExtensionsMetadata>> _dynamicFileInfoProviders;

[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public VisualStudioProjectFactory(
IThreadingContext threadingContext,
VisualStudioWorkspaceImpl visualStudioWorkspaceImpl,
[ImportMany] IEnumerable<Lazy<IDynamicFileInfoProvider, FileExtensionsMetadata>> fileInfoProviders,
HostDiagnosticUpdateSource hostDiagnosticUpdateSource)
{
_threadingContext = threadingContext;
_visualStudioWorkspaceImpl = visualStudioWorkspaceImpl;
_dynamicFileInfoProviders = fileInfoProviders.AsImmutableOrEmpty();
_hostDiagnosticUpdateSource = hostDiagnosticUpdateSource;
Expand All @@ -44,12 +49,19 @@ public VisualStudioProject CreateAndAddToWorkspace(string projectSystemName, str
=> CreateAndAddToWorkspace(projectSystemName, language, new VisualStudioProjectCreationInfo());

public VisualStudioProject CreateAndAddToWorkspace(string projectSystemName, string language, VisualStudioProjectCreationInfo creationInfo)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
=> _threadingContext.JoinableTaskFactory.Run(async () => await CreateAndAddToWorkspaceAsync(
projectSystemName, language, creationInfo).ConfigureAwait(false));

public async Task<VisualStudioProject> CreateAndAddToWorkspaceAsync(string projectSystemName, string language, VisualStudioProjectCreationInfo creationInfo)
{
// HACK: Fetch this service to ensure it's still created on the UI thread; once this is moved off we'll need to fix up it's constructor to be free-threaded.
_visualStudioWorkspaceImpl.Services.GetRequiredService<VisualStudioMetadataReferenceManager>();
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

// HACK: since we're on the UI thread, ensure we initialize our options provider which depends on a UI-affinitized experimentation service
// HACK: switch to the UI thread, ensure we initialize our options provider which depends on a
// UI-affinitized experimentation service
await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync();
_visualStudioWorkspaceImpl.EnsureDocumentOptionProvidersInitialized();
await TaskScheduler.Default;

var id = ProjectId.CreateNewId(projectSystemName);
var assemblyName = creationInfo.AssemblyName ?? projectSystemName;
Expand Down Expand Up @@ -89,16 +101,19 @@ public VisualStudioProject CreateAndAddToWorkspace(string projectSystemName, str
if (w.CurrentSolution.ProjectIds.Count == 0)
{
// Fetch the current solution path. Since we're on the UI thread right now, we can do that.
string? solutionFilePath = null;
var solution = (IVsSolution)Shell.ServiceProvider.GlobalProvider.GetService(typeof(SVsSolution));
if (solution != null)
var solutionFilePath = _threadingContext.JoinableTaskFactory.Run(async () =>
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
if (ErrorHandler.Failed(solution.GetSolutionInfo(out _, out solutionFilePath, out _)))
await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync();
var solution = (IVsSolution)Shell.ServiceProvider.GlobalProvider.GetService(typeof(SVsSolution));
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
if (solution != null)
{
// Paranoia: if the call failed, we definitely don't want to use any stuff that was set
solutionFilePath = null;
if (ErrorHandler.Succeeded(solution.GetSolutionInfo(out _, out var filePath, out _)))
return filePath;
}
}

// Paranoia: if the call failed, we definitely don't want to use any stuff that was set
return null;
});

var solutionSessionId = GetSolutionSessionId();
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
21 changes: 18 additions & 3 deletions src/VisualStudio/Core/Impl/ProjectSystem/CPS/CPSProjectFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
using System.Collections.Immutable;
using System.ComponentModel.Composition;
using System.IO;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel;
using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList;
Expand All @@ -22,35 +24,47 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem.C
[Export(typeof(IWorkspaceProjectContextFactory))]
internal partial class CPSProjectFactory : IWorkspaceProjectContextFactory
{
private readonly IThreadingContext _threadingContext;
private readonly VisualStudioProjectFactory _projectFactory;
private readonly VisualStudioWorkspaceImpl _workspace;
private readonly IProjectCodeModelFactory _projectCodeModelFactory;

[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public CPSProjectFactory(
IThreadingContext threadingContext,
VisualStudioProjectFactory projectFactory,
VisualStudioWorkspaceImpl workspace,
IProjectCodeModelFactory projectCodeModelFactory)
{
_threadingContext = threadingContext;
_projectFactory = projectFactory;
_workspace = workspace;
_projectCodeModelFactory = projectCodeModelFactory;
}

IWorkspaceProjectContext IWorkspaceProjectContextFactory.CreateProjectContext(
IWorkspaceProjectContext IWorkspaceProjectContextFactory.CreateProjectContext(string languageName, string projectUniqueName, string projectFilePath, Guid projectGuid, object hierarchy, string binOutputPath)
{
return _threadingContext.JoinableTaskFactory.Run(async () =>
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
await ((IWorkspaceProjectContextFactory)this).CreateProjectContextAsync(
languageName, projectUniqueName, projectFilePath, projectGuid, hierarchy, binOutputPath).ConfigureAwait(false));
}

async Task<IWorkspaceProjectContext> IWorkspaceProjectContextFactory.CreateProjectContextAsync(
string languageName,
string projectUniqueName,
string projectFilePath,
Guid projectGuid,
object hierarchy,
string binOutputPath)
{
var visualStudioProject = CreateVisualStudioProject(languageName, projectUniqueName, projectFilePath, hierarchy as IVsHierarchy, projectGuid);
var visualStudioProject = await CreateVisualStudioProjectAsync(
languageName, projectUniqueName, projectFilePath, hierarchy as IVsHierarchy, projectGuid).ConfigureAwait(false);
return new CPSProject(visualStudioProject, _workspace, _projectCodeModelFactory, projectGuid, binOutputPath);
}

private VisualStudioProject CreateVisualStudioProject(string languageName, string projectUniqueName, string projectFilePath, IVsHierarchy hierarchy, Guid projectGuid)
private async Task<VisualStudioProject> CreateVisualStudioProjectAsync(
string languageName, string projectUniqueName, string projectFilePath, IVsHierarchy hierarchy, Guid projectGuid)
{
var creationInfo = new VisualStudioProjectCreationInfo
{
Expand All @@ -63,6 +77,7 @@ private VisualStudioProject CreateVisualStudioProject(string languageName, strin

if (languageName == LanguageNames.FSharp)
{
await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync();
var shell = (IVsShell)ServiceProvider.GlobalProvider.GetService(typeof(SVsShell));
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

// Force the F# package to load; this is necessary because the F# package listens to WorkspaceChanged to
Expand Down