Skip to content

Improve performance in metadata-to-project-reference conversion #79163

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

Merged
merged 1 commit into from
Jun 27, 2025
Merged

Improve performance in metadata-to-project-reference conversion #79163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ static ProjectUpdateState UpdateMetadataReferences(
foreach (var (path, properties) in metadataReferencesAddedInBatch)
{
projectUpdateState = TryCreateConvertedProjectReference_NoLock(
projectId, path, properties, projectUpdateState, solutionChanges.Solution, out var projectReference);
projectBeforeMutation.State, path, properties, projectUpdateState, solutionChanges.Solution, out var projectReference);

if (projectReference != null)
{
Expand Down
57 changes: 28 additions & 29 deletions src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProjectFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
Expand Down Expand Up @@ -548,14 +549,20 @@ private static ProjectUpdateState ConvertMetadataReferencesToProjectReferences_N
string outputPath,
ProjectUpdateState projectUpdateState)
{
foreach (var projectIdToRetarget in solutionChanges.Solution.ProjectIds)
// PERF: call GetRequiredProjectState instead of GetRequiredProject, otherwise creating a new project
// might force all Project instances to get created.
var candidateProjectState = solutionChanges.Solution.GetRequiredProjectState(projectIdToReference);

foreach (var projectToRetarget in solutionChanges.Solution.SortedProjectStates)
{
if (CanConvertMetadataReferenceToProjectReference(solutionChanges.Solution, projectIdToRetarget, referencedProjectId: projectIdToReference))
// PERF: If we don't even have any metadata references yet, then don't even call CanConvertMetadataReferenceToProjectReference.
// This optimizes the early parts of solution load, where projects may be created with their output paths right away,
// but metadata references come in later. CanConvertMetadataReferenceToProjectReference isn't terribly expensive
// but when called enough times things can start to add up.
if (projectToRetarget.MetadataReferences.Count > 0 &&
CanConvertMetadataReferenceToProjectReference(solutionChanges.Solution, projectToRetarget, candidateProjectState))
{
// PERF: call GetRequiredProjectState instead of GetRequiredProject, otherwise creating a new project
// might force all Project instances to get created.
var projectState = solutionChanges.Solution.GetRequiredProjectState(projectIdToRetarget);
foreach (var reference in projectState.MetadataReferences)
foreach (var reference in projectToRetarget.MetadataReferences)
{
if (reference is PortableExecutableReference peReference
&& string.Equals(peReference.FilePath, outputPath, StringComparison.OrdinalIgnoreCase))
Expand All @@ -564,13 +571,13 @@ private static ProjectUpdateState ConvertMetadataReferencesToProjectReferences_N

var projectReference = new ProjectReference(projectIdToReference, peReference.Properties.Aliases, peReference.Properties.EmbedInteropTypes);
var newSolution = solutionChanges.Solution
.RemoveMetadataReference(projectIdToRetarget, peReference)
.AddProjectReference(projectIdToRetarget, projectReference);
.RemoveMetadataReference(projectToRetarget.Id, peReference)
.AddProjectReference(projectToRetarget.Id, projectReference);

solutionChanges.UpdateSolutionForProjectAction(projectIdToRetarget, newSolution);
solutionChanges.UpdateSolutionForProjectAction(projectToRetarget.Id, newSolution);

projectUpdateState = GetReferenceInformation(projectIdToRetarget, projectUpdateState, out var projectInfo);
projectUpdateState = projectUpdateState.WithProjectReferenceInfo(projectIdToRetarget,
projectUpdateState = GetReferenceInformation(projectToRetarget.Id, projectUpdateState, out var projectInfo);
projectUpdateState = projectUpdateState.WithProjectReferenceInfo(projectToRetarget.Id,
projectInfo.WithConvertedProjectReference(peReference.FilePath!, projectReference));

// We have converted one, but you could have more than one reference with different aliases that
Expand All @@ -585,33 +592,25 @@ private static ProjectUpdateState ConvertMetadataReferencesToProjectReferences_N

[PerformanceSensitive("https://github.com/dotnet/roslyn/issues/31306",
Constraint = "Avoid calling " + nameof(CodeAnalysis.Solution.GetProject) + " to avoid realizing all projects.")]
private static bool CanConvertMetadataReferenceToProjectReference(Solution solution, ProjectId projectIdWithMetadataReference, ProjectId referencedProjectId)
private static bool CanConvertMetadataReferenceToProjectReference(Solution solution, ProjectState projectWithMetadataReference, ProjectState candidateProjectToReference)
{
// We can never make a project reference ourselves. This isn't a meaningful scenario, but if somebody does this by accident
// we do want to throw exceptions.
if (projectIdWithMetadataReference == referencedProjectId)
if (projectWithMetadataReference.Id == candidateProjectToReference.Id)
{
return false;
}

// PERF: call GetProjectState instead of GetProject, otherwise creating a new project might force all
// Project instances to get created.
var projectWithMetadataReference = solution.GetProjectState(projectIdWithMetadataReference);
var referencedProject = solution.GetProjectState(referencedProjectId);

Contract.ThrowIfNull(projectWithMetadataReference);
Contract.ThrowIfNull(referencedProject);

// We don't want to convert a metadata reference to a project reference if the project being referenced isn't
// something we can create a Compilation for. For example, if we have a C# project, and it's referencing a F#
// project via a metadata reference everything would be fine if we left it a metadata reference. Converting it
// to a project reference means we couldn't create a Compilation anymore in the IDE, since the C# compilation
// would need to reference an F# compilation. F# projects referencing other F# projects though do expect this to
// work, and so we'll always allow references through of the same language.
if (projectWithMetadataReference.Language != referencedProject.Language)
if (projectWithMetadataReference.Language != candidateProjectToReference.Language)
{
if (projectWithMetadataReference.LanguageServices.GetService<ICompilationFactoryService>() != null &&
referencedProject.LanguageServices.GetService<ICompilationFactoryService>() == null)
candidateProjectToReference.LanguageServices.GetService<ICompilationFactoryService>() == null)
{
// We're referencing something that we can't create a compilation from something that can, so keep the metadata reference
return false;
Expand All @@ -621,11 +620,11 @@ private static bool CanConvertMetadataReferenceToProjectReference(Solution solut
// Getting a metadata reference from a 'module' is not supported from the compilation layer. Nor is emitting a
// 'metadata-only' stream for it (a 'skeleton' reference). So converting a NetModule reference to a project
// reference won't actually help us out. Best to keep this as a plain metadata reference.
if (referencedProject.CompilationOptions?.OutputKind == OutputKind.NetModule)
if (candidateProjectToReference.CompilationOptions?.OutputKind == OutputKind.NetModule)
return false;

// If this is going to cause a circular reference, also disallow it
if (solution.GetProjectDependencyGraph().GetProjectsThatThisProjectTransitivelyDependsOn(referencedProjectId).Contains(projectIdWithMetadataReference))
if (solution.GetProjectDependencyGraph().DoesProjectTransitivelyDependOnProject(candidateProjectToReference.Id, projectWithMetadataReference.Id))
{
return false;
}
Expand Down Expand Up @@ -696,7 +695,7 @@ private static ProjectUpdateState ConvertProjectReferencesToMetadataReferences_N
/// during a workspace update (which will attempt to apply the update multiple times).
/// </summary>
public static ProjectUpdateState TryCreateConvertedProjectReference_NoLock(
ProjectId referencingProject,
ProjectState referencingProjectState,
string path,
MetadataReferenceProperties properties,
ProjectUpdateState projectUpdateState,
Expand All @@ -707,15 +706,15 @@ public static ProjectUpdateState TryCreateConvertedProjectReference_NoLock(
{
var projectIdToReference = ids.First();

if (CanConvertMetadataReferenceToProjectReference(currentSolution, referencingProject, projectIdToReference))
if (CanConvertMetadataReferenceToProjectReference(currentSolution, referencingProjectState, currentSolution.GetRequiredProjectState(projectIdToReference)))
{
projectReference = new ProjectReference(
projectIdToReference,
aliases: properties.Aliases,
embedInteropTypes: properties.EmbedInteropTypes);

projectUpdateState = GetReferenceInformation(referencingProject, projectUpdateState, out var projectReferenceInfo);
projectUpdateState = projectUpdateState.WithProjectReferenceInfo(referencingProject, projectReferenceInfo.WithConvertedProjectReference(path, projectReference));
projectUpdateState = GetReferenceInformation(referencingProjectState.Id, projectUpdateState, out var projectReferenceInfo);
projectUpdateState = projectUpdateState.WithProjectReferenceInfo(referencingProjectState.Id, projectReferenceInfo.WithConvertedProjectReference(path, projectReference));
return projectUpdateState;
}
else
Expand Down
Loading