From 447efabe6c9b5c176ee41625dfd05057f026defd Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Mon, 23 Jun 2025 12:33:53 +1000 Subject: [PATCH 1/5] Improve perf of accessing configured value Fixes #2707 Lifeng reported that calling `GetActiveConfiguredProjectExports` on the base class was taking 0.5 seconds of CPU while loading Roslyn.sln. This change uses `IActiveConfiguredValue<>` to cache the instance, to improve performance while building dependencies trees across the solution during solution load. --- .../Dependencies/DependenciesTreeProvider.cs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs index a7f5c3b21d0..17569a8eacd 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs @@ -47,6 +47,7 @@ internal sealed partial class DependenciesTreeProvider : ProjectTreeProviderBase private readonly OrderPrecedenceImportCollection _removalActionHandlers; private readonly DependenciesSnapshotProvider _dependenciesSnapshotProvider; + private readonly IActiveConfiguredValue _activeConfiguredProjectExports; private readonly CancellationSeries _treeUpdateCancellationSeries; private readonly IProjectAccessor _projectAccessor; private readonly ITaskDelayScheduler _debounce; @@ -68,11 +69,13 @@ public DependenciesTreeProvider( IProjectThreadingService threadingService, IUnconfiguredProjectTasksService tasksService, IProjectAccessor projectAccessor, - DependenciesSnapshotProvider dependenciesSnapshotProvider) + DependenciesSnapshotProvider dependenciesSnapshotProvider, + IActiveConfiguredValue activeConfiguredProjectExports) : base(threadingService, unconfiguredProject) { _dependenciesSnapshotProvider = dependenciesSnapshotProvider; _projectAccessor = projectAccessor; + _activeConfiguredProjectExports = activeConfiguredProjectExports; _removalActionHandlers = new OrderPrecedenceImportCollection( ImportOrderPrecedenceComparer.PreferenceOrder.PreferredComesFirst, @@ -377,7 +380,7 @@ await _projectAccessor.OpenProjectForWriteAsync(ActiveConfiguredProject, project protected override ConfiguredProjectExports GetActiveConfiguredProjectExports(ConfiguredProject newActiveConfiguredProject) { - return GetActiveConfiguredProjectExports(newActiveConfiguredProject); + return (ConfiguredProjectExports)_activeConfiguredProjectExports.Value; } #region ITreeConstruction @@ -479,14 +482,12 @@ IProjectItemTree2 IProjectTreeOperations.NewTree(string caption, IProjectPropert /// /// Describes services collected from the active configured project. /// - [Export] - private sealed class MyConfiguredProjectExports : ConfiguredProjectExports + [Export(typeof(IConfiguredProjectExports))] + [method: ImportingConstructor] + private sealed class MyConfiguredProjectExports(ConfiguredProject configuredProject) + : ConfiguredProjectExports(configuredProject), + IConfiguredProjectExports { - [ImportingConstructor] - public MyConfiguredProjectExports(ConfiguredProject configuredProject) - : base(configuredProject) - { - } } /// @@ -504,4 +505,7 @@ public ProjectDependencyTreeRemovalActionHandlerContext(IProjectTreeProvider tre TreeProvider = treeProvider; } } + + // NOTE this interface is needed to work around accessiblity issues when making MyConfiguredProjectExports non-private + internal interface IConfiguredProjectExports { } } From 8ec603a2ffc8aca1a465c57b128a49b590279a9c Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Mon, 23 Jun 2025 12:34:12 +1000 Subject: [PATCH 2/5] Use primary constructor --- .../Tree/Dependencies/DependenciesTreeProvider.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs index 17569a8eacd..a7d1b8d0926 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs @@ -494,16 +494,11 @@ private sealed class MyConfiguredProjectExports(ConfiguredProject configuredProj /// A private implementation of for use with /// exports. /// - private sealed class ProjectDependencyTreeRemovalActionHandlerContext : IProjectTreeActionHandlerContext + private sealed class ProjectDependencyTreeRemovalActionHandlerContext(IProjectTreeProvider treeProvider) : IProjectTreeActionHandlerContext { - public IProjectTreeProvider TreeProvider { get; } + public IProjectTreeProvider TreeProvider { get; } = treeProvider; public IProjectTreeActionHandler SuccessorHandlerDelegator => throw new NotImplementedException(); - - public ProjectDependencyTreeRemovalActionHandlerContext(IProjectTreeProvider treeProvider) - { - TreeProvider = treeProvider; - } } // NOTE this interface is needed to work around accessiblity issues when making MyConfiguredProjectExports non-private From ddba0f948567b5da625ccaa74489eb55d262b619 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Mon, 23 Jun 2025 12:36:11 +1000 Subject: [PATCH 3/5] Use concrete type The IDE suggests this. --- .../ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs index a7d1b8d0926..7f1857c26ba 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs @@ -50,7 +50,7 @@ internal sealed partial class DependenciesTreeProvider : ProjectTreeProviderBase private readonly IActiveConfiguredValue _activeConfiguredProjectExports; private readonly CancellationSeries _treeUpdateCancellationSeries; private readonly IProjectAccessor _projectAccessor; - private readonly ITaskDelayScheduler _debounce; + private readonly TaskDelayScheduler _debounce; [Import] private DependenciesTreeBuilder TreeBuilder { get; set; } = null!; From c80c4cf7333a98722ce8838a49f66d3ee9063e72 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Mon, 23 Jun 2025 12:36:37 +1000 Subject: [PATCH 4/5] Document use of property import This style is uncommon in this repo, so document that/why it's needed here. --- .../Tree/Dependencies/DependenciesTreeProvider.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs index 7f1857c26ba..4d875737753 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/Dependencies/DependenciesTreeProvider.cs @@ -52,6 +52,9 @@ internal sealed partial class DependenciesTreeProvider : ProjectTreeProviderBase private readonly IProjectAccessor _projectAccessor; private readonly TaskDelayScheduler _debounce; + // NOTE we use a property import here as importing this via the constructor creates a loop between + // DependenciesTreeProvider and DependenciesTreeBuilder. A property import allows MEF to break that + // circular dependency. [Import] private DependenciesTreeBuilder TreeBuilder { get; set; } = null!; From dfdc9d7528eb2157652fedcf59f38ac85c69f289 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Mon, 23 Jun 2025 13:04:19 +1000 Subject: [PATCH 5/5] Apply similar optimisation to ImportTreeProvider This caches the configured value in the ImportTreeProvider, as recently done for the DependenciesTreeProvider. --- .../Tree/ProjectImports/ImportTreeProvider.cs | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/ProjectImports/ImportTreeProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/ProjectImports/ImportTreeProvider.cs index e0867e82a2e..7c3aa01103b 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/ProjectImports/ImportTreeProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Tree/ProjectImports/ImportTreeProvider.cs @@ -45,6 +45,7 @@ internal sealed class ImportTreeProvider : ProjectTreeProviderBase, IProjectTree private readonly UnconfiguredProject _project; private readonly IActiveConfiguredProjectSubscriptionService _projectSubscriptionService; private readonly IUnconfiguredProjectTasksService _unconfiguredProjectTasksService; + private readonly IActiveConfiguredValue _activeConfiguredProjectExports; private DisposableBag? _subscriptions; private bool _showAllFiles; @@ -54,12 +55,14 @@ internal ImportTreeProvider( IProjectThreadingService threadingService, UnconfiguredProject project, IActiveConfiguredProjectSubscriptionService projectSubscriptionService, - IUnconfiguredProjectTasksService unconfiguredProjectTasksService) + IUnconfiguredProjectTasksService unconfiguredProjectTasksService, + IActiveConfiguredValue activeConfiguredProjectExports) : base(threadingService, project, useDisplayOrdering: true) { _project = project; _projectSubscriptionService = projectSubscriptionService; _unconfiguredProjectTasksService = unconfiguredProjectTasksService; + _activeConfiguredProjectExports = activeConfiguredProjectExports; } public override string? GetPath(IProjectTree node) @@ -346,9 +349,7 @@ void TearDownTree() protected override ConfiguredProjectExports GetActiveConfiguredProjectExports(ConfiguredProject newActiveConfiguredProject) { - Requires.NotNull(newActiveConfiguredProject); - - return GetActiveConfiguredProjectExports(newActiveConfiguredProject); + return (ConfiguredProjectExports)_activeConfiguredProjectExports.Value; } protected override void Dispose(bool disposing) @@ -361,13 +362,14 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } - [Export] - private sealed class MyConfiguredProjectExports : ConfiguredProjectExports + [Export(typeof(IConfiguredProjectExports))] + [method: ImportingConstructor] + private sealed class MyConfiguredProjectExports(ConfiguredProject configuredProject) + : ConfiguredProjectExports(configuredProject), + IConfiguredProjectExports { - [ImportingConstructor] - public MyConfiguredProjectExports(ConfiguredProject configuredProject) - : base(configuredProject) - { - } } + + // NOTE this interface is needed to work around accessiblity issues when making MyConfiguredProjectExports non-private + internal interface IConfiguredProjectExports { } }