From 56f870ff9eae0cd56f576b0601cb0bdb61ff5aa8 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 15:12:52 +1100 Subject: [PATCH 01/21] Use file-scoped namespace --- ...formTargetBuildPropertyPageEnumProvider.cs | 71 +++++++++---------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs index 69d475bd493..2f916ad172b 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs @@ -5,53 +5,52 @@ using Microsoft.VisualStudio.Threading; using EnumCollection = System.Collections.Generic.ICollection; -namespace Microsoft.VisualStudio.ProjectSystem.Properties +namespace Microsoft.VisualStudio.ProjectSystem.Properties; + +/// +/// Responsible for producing valid values for the TargetPlatform property from a design-time build. +/// +[ExportDynamicEnumValuesProvider("PlatformTargetEnumProvider")] +[AppliesTo(ProjectCapability.DotNet)] +internal class PlatformTargetBuildPropertyPageEnumProvider : IDynamicEnumValuesProvider, IDynamicEnumValuesGenerator { - /// - /// Responsible for producing valid values for the TargetPlatform property from a design-time build. - /// - [ExportDynamicEnumValuesProvider("PlatformTargetEnumProvider")] - [AppliesTo(ProjectCapability.DotNet)] - internal class PlatformTargetBuildPropertyPageEnumProvider : IDynamicEnumValuesProvider, IDynamicEnumValuesGenerator - { - private const string AnyCpuPlatformName = "AnyCPU"; - private const string AnyCpuDisplayName = "Any CPU"; - - private readonly ProjectProperties _properties; - - [ImportingConstructor] - public PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties properties) - { - _properties = properties; - } + private const string AnyCpuPlatformName = "AnyCPU"; + private const string AnyCpuDisplayName = "Any CPU"; - public bool AllowCustomValues => false; + private readonly ProjectProperties _properties; - public async Task GetListedValuesAsync() - { - var result = new List(); + [ImportingConstructor] + public PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties properties) + { + _properties = properties; + } - ConfigurationGeneral configuration = await _properties.GetConfigurationGeneralPropertiesAsync(); + public bool AllowCustomValues => false; - string availablePlatformsTargets = await configuration.AvailablePlatforms.GetDisplayValueAsync(); + public async Task GetListedValuesAsync() + { + var result = new List(); - foreach (string platformTarget in new LazyStringSplit(availablePlatformsTargets, ',')) - { - result.Add(new PageEnumValue(new EnumValue() { Name = platformTarget, DisplayName = platformTarget.Equals(AnyCpuPlatformName, StringComparisons.ConfigurationDimensionValues) ? AnyCpuDisplayName : platformTarget })); - } + ConfigurationGeneral configuration = await _properties.GetConfigurationGeneralPropertiesAsync(); - return result; - } + string availablePlatformsTargets = await configuration.AvailablePlatforms.GetDisplayValueAsync(); - public Task GetProviderAsync(IList? options) + foreach (string platformTarget in new LazyStringSplit(availablePlatformsTargets, ',')) { - return Task.FromResult(this); + result.Add(new PageEnumValue(new EnumValue() { Name = platformTarget, DisplayName = platformTarget.Equals(AnyCpuPlatformName, StringComparisons.ConfigurationDimensionValues) ? AnyCpuDisplayName : platformTarget })); } - public Task TryCreateEnumValueAsync(string userSuppliedValue) - { - return TaskResult.Null(); - } + return result; + } + + public Task GetProviderAsync(IList? options) + { + return Task.FromResult(this); + } + + public Task TryCreateEnumValueAsync(string userSuppliedValue) + { + return TaskResult.Null(); } } From dc26a024cd51f2dcf0daec46ba15bd9698b86772 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 15:13:18 +1100 Subject: [PATCH 02/21] Use primary constructor --- .../PlatformTargetBuildPropertyPageEnumProvider.cs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs index 2f916ad172b..914691590d0 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs @@ -12,26 +12,19 @@ namespace Microsoft.VisualStudio.ProjectSystem.Properties; /// [ExportDynamicEnumValuesProvider("PlatformTargetEnumProvider")] [AppliesTo(ProjectCapability.DotNet)] -internal class PlatformTargetBuildPropertyPageEnumProvider : IDynamicEnumValuesProvider, IDynamicEnumValuesGenerator +[method: ImportingConstructor] +internal class PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties properties) : IDynamicEnumValuesProvider, IDynamicEnumValuesGenerator { private const string AnyCpuPlatformName = "AnyCPU"; private const string AnyCpuDisplayName = "Any CPU"; - private readonly ProjectProperties _properties; - - [ImportingConstructor] - public PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties properties) - { - _properties = properties; - } - public bool AllowCustomValues => false; public async Task GetListedValuesAsync() { var result = new List(); - ConfigurationGeneral configuration = await _properties.GetConfigurationGeneralPropertiesAsync(); + ConfigurationGeneral configuration = await properties.GetConfigurationGeneralPropertiesAsync(); string availablePlatformsTargets = await configuration.AvailablePlatforms.GetDisplayValueAsync(); From 89fc46196938fcc834bce45e06eddabb4524fdfc Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 15:14:29 +1100 Subject: [PATCH 03/21] Add local function --- .../PlatformTargetBuildPropertyPageEnumProvider.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs index 914691590d0..3050c159bb6 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs @@ -15,9 +15,6 @@ namespace Microsoft.VisualStudio.ProjectSystem.Properties; [method: ImportingConstructor] internal class PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties properties) : IDynamicEnumValuesProvider, IDynamicEnumValuesGenerator { - private const string AnyCpuPlatformName = "AnyCPU"; - private const string AnyCpuDisplayName = "Any CPU"; - public bool AllowCustomValues => false; public async Task GetListedValuesAsync() @@ -30,10 +27,18 @@ public async Task GetListedValuesAsync() foreach (string platformTarget in new LazyStringSplit(availablePlatformsTargets, ',')) { - result.Add(new PageEnumValue(new EnumValue() { Name = platformTarget, DisplayName = platformTarget.Equals(AnyCpuPlatformName, StringComparisons.ConfigurationDimensionValues) ? AnyCpuDisplayName : platformTarget })); + result.Add(new PageEnumValue(new EnumValue() { Name = platformTarget, DisplayName = GetDisplayName(platformTarget) })); } return result; + + static string GetDisplayName(string platformTarget) + { + const string AnyCpuPlatformName = "AnyCPU"; + const string AnyCpuDisplayName = "Any CPU"; + + return platformTarget.Equals(AnyCpuPlatformName, StringComparisons.ConfigurationDimensionValues) ? AnyCpuDisplayName : platformTarget; + } } public Task GetProviderAsync(IList? options) From 24d0daf71cbad1c29a28c865143dbf6056123415 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 15:15:04 +1100 Subject: [PATCH 04/21] Inline type alias --- .../Properties/PlatformTargetBuildPropertyPageEnumProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs index 3050c159bb6..25283574825 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs @@ -3,7 +3,6 @@ using Microsoft.Build.Framework.XamlTypes; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Threading; -using EnumCollection = System.Collections.Generic.ICollection; namespace Microsoft.VisualStudio.ProjectSystem.Properties; @@ -17,7 +16,7 @@ internal class PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties pro { public bool AllowCustomValues => false; - public async Task GetListedValuesAsync() + public async Task> GetListedValuesAsync() { var result = new List(); From 1b2ffea5641671d37429f1cec96bf72e3828d033 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 15:17:33 +1100 Subject: [PATCH 05/21] Use target-typed new and collection expression --- .../Properties/PlatformTargetBuildPropertyPageEnumProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs index 25283574825..d1410142b95 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs @@ -18,7 +18,7 @@ internal class PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties pro public async Task> GetListedValuesAsync() { - var result = new List(); + List result = []; ConfigurationGeneral configuration = await properties.GetConfigurationGeneralPropertiesAsync(); From bf950127b870b9e99db07b6db74a64ea9ef4e435 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 15:17:48 +1100 Subject: [PATCH 06/21] Improve API doc --- .../PlatformTargetBuildPropertyPageEnumProvider.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs index d1410142b95..7d7503b1341 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs @@ -7,8 +7,11 @@ namespace Microsoft.VisualStudio.ProjectSystem.Properties; /// -/// Responsible for producing valid values for the TargetPlatform property from a design-time build. +/// Responsible for producing valid values for the TargetPlatform MSBuild property. /// +/// +/// Candidate values from the AvailablePlatforms MSBuild property. +/// [ExportDynamicEnumValuesProvider("PlatformTargetEnumProvider")] [AppliesTo(ProjectCapability.DotNet)] [method: ImportingConstructor] From 2d77b264eb4341fb5b54963de795feabbad802c7 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 15:20:06 +1100 Subject: [PATCH 07/21] Prevent duplicate target platform values --- .../PlatformTargetBuildPropertyPageEnumProvider.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs index 7d7503b1341..09a15e6a6d1 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs @@ -21,18 +21,23 @@ internal class PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties pro public async Task> GetListedValuesAsync() { - List result = []; - ConfigurationGeneral configuration = await properties.GetConfigurationGeneralPropertiesAsync(); string availablePlatformsTargets = await configuration.AvailablePlatforms.GetDisplayValueAsync(); + List enumValues = []; + HashSet targets = new(StringComparers.ConfigurationDimensionValues); + foreach (string platformTarget in new LazyStringSplit(availablePlatformsTargets, ',')) { - result.Add(new PageEnumValue(new EnumValue() { Name = platformTarget, DisplayName = GetDisplayName(platformTarget) })); + // Prevent duplicates. + if (targets.Add(platformTarget)) + { + enumValues.Add(new PageEnumValue(new EnumValue() { Name = platformTarget, DisplayName = GetDisplayName(platformTarget) })); + } } - return result; + return enumValues; static string GetDisplayName(string platformTarget) { From c7c72cc28ac0ab2ab9f3ad71bc468678637c3c7e Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 15:20:31 +1100 Subject: [PATCH 08/21] Seal class --- .../Properties/PlatformTargetBuildPropertyPageEnumProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs index 09a15e6a6d1..3d45bb9664d 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/PlatformTargetBuildPropertyPageEnumProvider.cs @@ -15,7 +15,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.Properties; [ExportDynamicEnumValuesProvider("PlatformTargetEnumProvider")] [AppliesTo(ProjectCapability.DotNet)] [method: ImportingConstructor] -internal class PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties properties) : IDynamicEnumValuesProvider, IDynamicEnumValuesGenerator +internal sealed class PlatformTargetBuildPropertyPageEnumProvider(ProjectProperties properties) : IDynamicEnumValuesProvider, IDynamicEnumValuesGenerator { public bool AllowCustomValues => false; From f609dbb35c57d8bd38b49dbdde5aebfa6f5f42e6 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 21:32:09 +1100 Subject: [PATCH 09/21] Remove commented using directive --- .../ProjectSystem/VS/Rename/RenamerTestsBase.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Rename/RenamerTestsBase.cs b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Rename/RenamerTestsBase.cs index aed1ccfcadb..2c790feef5f 100644 --- a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Rename/RenamerTestsBase.cs +++ b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Rename/RenamerTestsBase.cs @@ -1,6 +1,5 @@ // Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. -//using System; using EnvDTE; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Text; From a85583c8c367b2a72d3118bdb6dd80ced448caed Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 21:47:41 +1100 Subject: [PATCH 10/21] Protect concurrent dictionary access --- .../DesignTimeBuildLoggerProvider.cs | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs index 4479cddb01e..693a546c4ed 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs @@ -54,6 +54,8 @@ private sealed class DesignTimeTelemetryLogger(ITelemetryService telemetryServic /// /// The initial capacity here is based on an empty .NET Console App's DTB, which has /// around 120 targets, and aims to avoid resizing the dictionary during the build. + /// + /// Targets can run concurrently, so use of this collection must be protected by a lock. /// private readonly Dictionary _targetRecordById = new(capacity: 140); @@ -93,7 +95,10 @@ void OnTargetStarted(object sender, TargetStartedEventArgs e) e.TargetFile, file => ProjectFileClassifier.IsShippedByMicrosoft(e.TargetFile)); - _targetRecordById[id] = new TargetRecord(e.TargetName, IsMicrosoft: isMicrosoft, e.Timestamp); + lock (_targetRecordById) + { + _targetRecordById[id] = new TargetRecord(e.TargetName, IsMicrosoft: isMicrosoft, e.Timestamp); + } } } @@ -123,9 +128,12 @@ void OnBuildFinished(object sender, BuildFinishedEventArgs e) bool TryGetTargetRecord(BuildEventContext? context, [NotNullWhen(returnValue: true)] out TargetRecord? record) { - if (context is { TargetId: int id } && _targetRecordById.TryGetValue(id, out record)) + lock (_targetRecordById) { - return true; + if (context is { TargetId: int id } && _targetRecordById.TryGetValue(id, out record)) + { + return true; + } } record = null; @@ -141,14 +149,19 @@ void ILogger.Shutdown() void SendTelemetry() { - // Filter out very fast targets (to reduce the cost of ordering) then take the top ten by elapsed time. - // Note that targets can run multiple times, so the same target may appear more than once in the results. - object[][] targetDurations = _targetRecordById.Values - .Where(static record => record.Elapsed > new TimeSpan(ticks: 5 * TimeSpan.TicksPerMillisecond)) - .OrderByDescending(record => record.Elapsed) - .Take(10) - .Select(record => new object[] { GetHashedTargetName(record), record.Elapsed.Milliseconds }) - .ToArray(); + object[][] targetDurations; + + lock (_targetRecordById) + { + // Filter out very fast targets (to reduce the cost of ordering) then take the top ten by elapsed time. + // Note that targets can run multiple times, so the same target may appear more than once in the results. + targetDurations = _targetRecordById.Values + .Where(static record => record.Elapsed > new TimeSpan(ticks: 5 * TimeSpan.TicksPerMillisecond)) + .OrderByDescending(record => record.Elapsed) + .Take(10) + .Select(record => new object[] { GetHashedTargetName(record), record.Elapsed.Milliseconds }) + .ToArray(); + } telemetryService.PostProperties( TelemetryEventName.DesignTimeBuildComplete, From 3474213ac71c979a62a678c586151b8f6824f947 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 21:48:05 +1100 Subject: [PATCH 11/21] Show info bars on build failures, not target errors --- .../VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs index 693a546c4ed..a9565a10a34 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs @@ -175,8 +175,11 @@ void SendTelemetry() void ReportBuildErrors() { - if (_errorCount is 0) + if (_succeeded is not false) { + // Only report a failure if the build failed. Specific targets can have + // errors, yet if ContinueOnError is set accordingly they won't fail the + // build. We don't want to report those to the user. return; } From 2790bb5020a249c85758c451ecb562bdaa7fe7db Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 21:52:32 +1100 Subject: [PATCH 12/21] Protect concurrent list access --- .../DesignTimeBuildLoggerProvider.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs index a9565a10a34..87d731519b7 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs @@ -63,7 +63,10 @@ private sealed class DesignTimeTelemetryLogger(ITelemetryService telemetryServic /// The names of any targets that reported errors. Names may be hashed. /// May be empty even when errors exist, as not all errors come from a target. /// - private List? _errorTargets; + /// + /// Targets can run concurrently, so use of this collection must be protected by a lock. + /// + private readonly List _errorTargets = []; /// /// Whether the build succeeded. @@ -116,8 +119,10 @@ void OnErrorRaised(object sender, BuildErrorEventArgs e) if (TryGetTargetRecord(e.BuildEventContext, out TargetRecord? record)) { - _errorTargets ??= []; - _errorTargets.Add(GetHashedTargetName(record)); + lock (_errorTargets) + { + _errorTargets.Add(GetHashedTargetName(record)); + } } } @@ -150,6 +155,7 @@ void ILogger.Shutdown() void SendTelemetry() { object[][] targetDurations; + string[] errorTargets; lock (_targetRecordById) { @@ -161,6 +167,8 @@ void SendTelemetry() .Take(10) .Select(record => new object[] { GetHashedTargetName(record), record.Elapsed.Milliseconds }) .ToArray(); + + errorTargets = _errorTargets.ToArray(); } telemetryService.PostProperties( @@ -169,7 +177,7 @@ void SendTelemetry() (TelemetryPropertyName.DesignTimeBuildComplete.Succeeded, _succeeded), (TelemetryPropertyName.DesignTimeBuildComplete.Targets, new ComplexPropertyValue(targetDurations)), (TelemetryPropertyName.DesignTimeBuildComplete.ErrorCount, _errorCount), - (TelemetryPropertyName.DesignTimeBuildComplete.ErrorTargets, _errorTargets), + (TelemetryPropertyName.DesignTimeBuildComplete.ErrorTargets, errorTargets), ]); } From 63540409482dc11f78dd48e4cfb7cdf8f20dd7d1 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 10 Dec 2024 23:34:03 +1100 Subject: [PATCH 13/21] Add error code to DTB error telemetry --- .../DesignTimeBuildLoggerProvider.cs | 41 +++++++++++-------- .../Telemetry/TelemetryPropertyName.cs | 4 +- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs index 87d731519b7..2f6bdbf4a0d 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/DesignTimeBuilds/DesignTimeBuildLoggerProvider.cs @@ -60,24 +60,22 @@ private sealed class DesignTimeTelemetryLogger(ITelemetryService telemetryServic private readonly Dictionary _targetRecordById = new(capacity: 140); /// - /// The names of any targets that reported errors. Names may be hashed. - /// May be empty even when errors exist, as not all errors come from a target. + /// Data about any errors that occurred during the build. Note that design-time build + /// targets are supposed to be authored to respect ContinueOnError, so errors might + /// not fail the build. However it's still useful to know which targets reported errors. + /// + /// Similarly, this collection may be empty even when the build fails. /// /// /// Targets can run concurrently, so use of this collection must be protected by a lock. /// - private readonly List _errorTargets = []; + private readonly List _errors = []; /// /// Whether the build succeeded. /// private bool? _succeeded; - /// - /// The number of errors reported during the build. - /// - private int _errorCount; - LoggerVerbosity ILogger.Verbosity { get; set; } string? ILogger.Parameters { get; set; } @@ -115,14 +113,16 @@ void OnTargetFinished(object sender, TargetFinishedEventArgs e) void OnErrorRaised(object sender, BuildErrorEventArgs e) { - _errorCount++; + string? targetName = null; if (TryGetTargetRecord(e.BuildEventContext, out TargetRecord? record)) { - lock (_errorTargets) - { - _errorTargets.Add(GetHashedTargetName(record)); - } + targetName = GetHashedTargetName(record); + } + + lock (_errors) + { + _errors.Add(new(targetName, e.Code)); } } @@ -155,7 +155,7 @@ void ILogger.Shutdown() void SendTelemetry() { object[][] targetDurations; - string[] errorTargets; + ErrorData[] errors; lock (_targetRecordById) { @@ -168,7 +168,7 @@ void SendTelemetry() .Select(record => new object[] { GetHashedTargetName(record), record.Elapsed.Milliseconds }) .ToArray(); - errorTargets = _errorTargets.ToArray(); + errors = _errors.ToArray(); } telemetryService.PostProperties( @@ -176,8 +176,8 @@ void SendTelemetry() [ (TelemetryPropertyName.DesignTimeBuildComplete.Succeeded, _succeeded), (TelemetryPropertyName.DesignTimeBuildComplete.Targets, new ComplexPropertyValue(targetDurations)), - (TelemetryPropertyName.DesignTimeBuildComplete.ErrorCount, _errorCount), - (TelemetryPropertyName.DesignTimeBuildComplete.ErrorTargets, errorTargets), + (TelemetryPropertyName.DesignTimeBuildComplete.ErrorCount, errors.Length), + (TelemetryPropertyName.DesignTimeBuildComplete.Errors, new ComplexPropertyValue(errors)), ]); } @@ -232,5 +232,12 @@ private sealed record class TargetRecord(string TargetName, bool IsMicrosoft, Da public TimeSpan Elapsed => Ended - Started; } + + /// + /// Data about errors reported during design-time builds. + /// + /// Names of the targets that reported the error. May be hashed. if the target name could not be identified. + /// An error code that identifies the type of error. + private sealed record class ErrorData(string? TargetName, string ErrorCode); } } diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/TelemetryPropertyName.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/TelemetryPropertyName.cs index 5ae6d623431..0b9ff4e26e2 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/TelemetryPropertyName.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/Telemetry/TelemetryPropertyName.cs @@ -199,9 +199,9 @@ public static class DesignTimeBuildComplete public const string ErrorCount = "vs.projectsystem.managed.designtimebuildcomplete.errorcount"; /// - /// The names of targets that failed during the design-time build. + /// Data about errors that occur during design-time builds. /// - public const string ErrorTargets = "vs.projectsystem.managed.designtimebuildcomplete.errortargets"; + public const string Errors = "vs.projectsystem.managed.designtimebuildcomplete.errors"; } public static class SDKVersion From 28761421aca49d9c235542bf5ab6a795122d1841 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Thu, 12 Dec 2024 22:37:17 +1100 Subject: [PATCH 14/21] Fix "collection modified" exception When signalling project close to an info bar entry, if there are no other remaining projects associated with that info bar, it will be removed from the collection. That triggers an exception. The fix is to make a defensive copy before enumerating, so that the original can be safely modified while the copy is enumerated. --- .../ProjectSystem/VS/UI/InfoBars/VsInfoBarService.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/UI/InfoBars/VsInfoBarService.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/UI/InfoBars/VsInfoBarService.cs index 076103eebd7..80548f0c896 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/UI/InfoBars/VsInfoBarService.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/UI/InfoBars/VsInfoBarService.cs @@ -47,7 +47,10 @@ public VsInfoBarService( { lock (s_entries) { - foreach (InfoBar entry in s_entries) + // Copy the list to avoid "collection modified during enumeration" exceptions, + // as when the last project associated with an info bar is closed, we will + // remove that entry from the list. + foreach (InfoBar entry in s_entries.ToList()) { entry.OnProjectClosed(_project); } From b0c1b13ad64f47e3fb6646f7bde9e9ebea99d343 Mon Sep 17 00:00:00 2001 From: emilyfischer Date: Wed, 11 Dec 2024 16:01:17 -0600 Subject: [PATCH 15/21] changes required for launch.json UI to work properly --- .../VS/Query/PropertyPages/ContextAndRuleProviderState.cs | 2 +- .../ProjectSystem/VS/Query/PropertyPages/IProjectState.cs | 2 +- .../VS/Query/PropertyPages/QueryProjectPropertiesContext.cs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/ContextAndRuleProviderState.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/ContextAndRuleProviderState.cs index 70cc832d2d6..786d537998d 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/ContextAndRuleProviderState.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/ContextAndRuleProviderState.cs @@ -10,7 +10,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.VS.Query; /// to pass the state their child producers will need, but allows the actual binding /// of the to be delayed until needed. /// -internal sealed class ContextAndRuleProviderState +public sealed class ContextAndRuleProviderState { public ContextAndRuleProviderState(IProjectState projectState, QueryProjectPropertiesContext propertiesContext, Rule rule) { diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/IProjectState.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/IProjectState.cs index b5044898e83..8cfe7a00d98 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/IProjectState.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/IProjectState.cs @@ -32,7 +32,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.VS.Query; /// significantly reduce the amount of work we need to do. /// /// -internal interface IProjectState +public interface IProjectState { /// /// Binds the specified schema to a particular context within the given project configuration. diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/QueryProjectPropertiesContext.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/QueryProjectPropertiesContext.cs index 6383525c0ab..453a789967b 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/QueryProjectPropertiesContext.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/QueryProjectPropertiesContext.cs @@ -16,7 +16,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.VS.Query; /// manner that can be passed from one provider to another and is also suitable as a /// key into a cache (such as the ). /// -internal sealed class QueryProjectPropertiesContext : IProjectPropertiesContext, IEquatable +public sealed class QueryProjectPropertiesContext : IProjectPropertiesContext, IEquatable { /// /// A well-known context representing the project file as a whole. @@ -84,7 +84,7 @@ public override int GetHashCode() /// Creates a from a Project Query API /// . /// - public static bool TryCreateFromEntityId(EntityIdentity id, [NotNullWhen(true)] out QueryProjectPropertiesContext? propertiesContext) + internal static bool TryCreateFromEntityId(EntityIdentity id, [NotNullWhen(true)] out QueryProjectPropertiesContext? propertiesContext) { if (id.TryGetValue(ProjectModelIdentityKeys.ProjectPath, out string? projectPath)) { From f22999a3cde0ba7ca1c90996b5542bb59ca2b603 Mon Sep 17 00:00:00 2001 From: emilyfischer Date: Thu, 12 Dec 2024 15:30:55 -0600 Subject: [PATCH 16/21] small updates --- .../PublicAPI.Unshipped.txt | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/PublicAPI.Unshipped.txt b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/PublicAPI.Unshipped.txt index e69de29bb2d..b5f0076f9e2 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/PublicAPI.Unshipped.txt +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/PublicAPI.Unshipped.txt @@ -0,0 +1,22 @@ +Microsoft.VisualStudio.ProjectSystem.VS.Query.ContextAndRuleProviderState +Microsoft.VisualStudio.ProjectSystem.VS.Query.ContextAndRuleProviderState.ContextAndRuleProviderState(Microsoft.VisualStudio.ProjectSystem.VS.Query.IProjectState! projectState, Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext! propertiesContext, Microsoft.Build.Framework.XamlTypes.Rule! rule) -> void +Microsoft.VisualStudio.ProjectSystem.VS.Query.ContextAndRuleProviderState.ProjectState.get -> Microsoft.VisualStudio.ProjectSystem.VS.Query.IProjectState! +Microsoft.VisualStudio.ProjectSystem.VS.Query.ContextAndRuleProviderState.PropertiesContext.get -> Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext! +Microsoft.VisualStudio.ProjectSystem.VS.Query.ContextAndRuleProviderState.Rule.get -> Microsoft.Build.Framework.XamlTypes.Rule! +Microsoft.VisualStudio.ProjectSystem.VS.Query.IProjectState +Microsoft.VisualStudio.ProjectSystem.VS.Query.IProjectState.BindToRuleAsync(Microsoft.VisualStudio.ProjectSystem.ProjectConfiguration! projectConfiguration, string! schemaName, Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext! propertiesContext) -> System.Threading.Tasks.Task! +Microsoft.VisualStudio.ProjectSystem.VS.Query.IProjectState.GetDataVersionAsync(Microsoft.VisualStudio.ProjectSystem.ProjectConfiguration! configuration) -> System.Threading.Tasks.Task<(string! versionKey, long versionNumber)?>! +Microsoft.VisualStudio.ProjectSystem.VS.Query.IProjectState.GetKnownConfigurationsAsync() -> System.Threading.Tasks.Task?>! +Microsoft.VisualStudio.ProjectSystem.VS.Query.IProjectState.GetMetadataVersionAsync() -> System.Threading.Tasks.Task<(string! versionKey, long versionNumber)?>! +Microsoft.VisualStudio.ProjectSystem.VS.Query.IProjectState.GetSuggestedConfigurationAsync() -> System.Threading.Tasks.Task! +Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext +Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.Equals(Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext? other) -> bool +Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.File.get -> string! +Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.IsProjectFile.get -> bool +Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.ItemName.get -> string? +Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.ItemType.get -> string? +Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.QueryProjectPropertiesContext(bool isProjectFile, string! file, string? itemType, string? itemName) -> void +override Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.Equals(object! obj) -> bool +override Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.GetHashCode() -> int +static Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.TryCreateFromEntityId(Microsoft.VisualStudio.ProjectSystem.Query.EntityIdentity! id, out Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext? propertiesContext) -> bool +static readonly Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext.ProjectFile -> Microsoft.VisualStudio.ProjectSystem.VS.Query.QueryProjectPropertiesContext! \ No newline at end of file From d0e1f7a9efc6ca4bc0e59ef3069aea72f2f632d3 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Mon, 16 Dec 2024 11:22:20 +1100 Subject: [PATCH 17/21] Avoid extra JTF collection/factory `LanguageServiceHost` uses its own JTF collection and factory to track its work. This class derives from `OnceInitializedOnceDisposedAsync`, which also has its own collection and factory. @lifengl recently noticed this while investigating a hang and suggested that we reuse the base class's objects to reduce allocations and potentially even help avoid deadlocks. --- .../VS/LanguageServices/LanguageServiceHost.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs index 1a8f7b59e81..4a4dea99197 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs @@ -45,8 +45,6 @@ internal sealed class LanguageServiceHost : OnceInitializedOnceDisposedAsync, IP private readonly IUnconfiguredProjectTasksService _tasksService; private readonly ISafeProjectGuidService _projectGuidService; private readonly IProjectFaultHandlerService _projectFaultHandler; - private readonly JoinableTaskCollection _joinableTaskCollection; - private readonly JoinableTaskFactory _joinableTaskFactory; private readonly AsyncLazy _isEnabled; private DisposableBag? _disposables; @@ -88,10 +86,6 @@ public LanguageServiceHost( || await vsShell.IsPopulateSolutionCacheModeAsync(); }, threadingService.JoinableTaskFactory); - - _joinableTaskCollection = threadingService.JoinableTaskContext.CreateCollection(); - _joinableTaskCollection.DisplayName = "LanguageServiceHostTasks"; - _joinableTaskFactory = new JoinableTaskFactory(_joinableTaskCollection); } public Task LoadAsync() @@ -175,7 +169,7 @@ protected override async Task InitializeCoreAsync(CancellationToken cancellation linkOptions: DataflowOption.PropagateCompletion, cancellationToken: cancellationToken), - ProjectDataSources.JoinUpstreamDataSources(_joinableTaskFactory, _projectFaultHandler, _activeConfiguredProjectProvider, _activeConfigurationGroupSubscriptionService), + ProjectDataSources.JoinUpstreamDataSources(JoinableFactory, _projectFaultHandler, _activeConfiguredProjectProvider, _activeConfigurationGroupSubscriptionService), new DisposableDelegate(() => { @@ -211,7 +205,7 @@ async Task OnSlicesChangedAsync(IProjectVersionedValue<(ConfiguredProject Active Guid projectGuid = await _projectGuidService.GetProjectGuidAsync(cancellationToken); // New slice. Create a workspace for it. - workspace = _workspaceFactory.Create(source, slice, _joinableTaskCollection, _joinableTaskFactory, projectGuid, cancellationToken); + workspace = _workspaceFactory.Create(source, slice, JoinableCollection, JoinableFactory, projectGuid, cancellationToken); if (workspace is null) { @@ -282,7 +276,7 @@ public async Task WhenInitialized(CancellationToken token) { await ValidateEnabledAsync(token); - using (_joinableTaskCollection.Join()) + using (JoinableCollection.Join()) { await _firstPrimaryWorkspaceSet.Task.WithCancellation(token); } @@ -350,7 +344,7 @@ public async Task AfterLoadInitialConfigurationAsync() // Ensure the project is not considered loaded until our first publication. Task result = _tasksService.PrioritizedProjectLoadedInHostAsync(async () => { - using (_joinableTaskCollection.Join()) + using (JoinableCollection.Join()) { await WhenInitialized(_tasksService.UnloadCancellationToken); } From 67f60bff0d9cbac7084ebb4f3c275b56a30e6405 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 17 Dec 2024 13:33:20 +1100 Subject: [PATCH 18/21] Make "isEnabled" static This value can be shared across all projects throughout the lifetime of VS. Making it static reduces allocations and causes the factory logic to run once, rather than per-project. --- .../VS/LanguageServices/LanguageServiceHost.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs index 4a4dea99197..5742c617908 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs @@ -36,6 +36,11 @@ namespace Microsoft.VisualStudio.ProjectSystem.LanguageServices; [AppliesTo(ProjectCapability.DotNetLanguageService)] internal sealed class LanguageServiceHost : OnceInitializedOnceDisposedAsync, IProjectDynamicLoadComponent, IWorkspaceWriter { + /// + /// Singleton instance across all projects, initialized once. + /// + private static AsyncLazy? s_isEnabled; + private readonly TaskCompletionSource _firstPrimaryWorkspaceSet = new(); private readonly UnconfiguredProject _unconfiguredProject; @@ -45,7 +50,6 @@ internal sealed class LanguageServiceHost : OnceInitializedOnceDisposedAsync, IP private readonly IUnconfiguredProjectTasksService _tasksService; private readonly ISafeProjectGuidService _projectGuidService; private readonly IProjectFaultHandlerService _projectFaultHandler; - private readonly AsyncLazy _isEnabled; private DisposableBag? _disposables; @@ -76,7 +80,9 @@ public LanguageServiceHost( _projectGuidService = projectGuidService; _projectFaultHandler = projectFaultHandler; - _isEnabled = new( + // We initialize this once across all instances. Note that we don't need any synchronization here. + // If more than one thread initializes this, it's not a big deal. + s_isEnabled ??= new( async () => { // If VS is running in command line mode (e.g. "devenv.exe /build my.sln"), @@ -268,8 +274,10 @@ async Task OnSlicesChangedAsync(IProjectVersionedValue<(ConfiguredProject Active public Task IsEnabledAsync(CancellationToken cancellationToken) { + Assumes.NotNull(s_isEnabled); + // Defer to the host environment to determine if we're enabled. - return _isEnabled.GetValueAsync(cancellationToken); + return s_isEnabled.GetValueAsync(cancellationToken); } public async Task WhenInitialized(CancellationToken token) From ca6820a91050c3baba094a859fae494cec731117 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 17 Dec 2024 13:35:25 +1100 Subject: [PATCH 19/21] Set AsyncLazy.SuppressRecursiveFactoryDetection true This avoids the overhead associated with the instance's re-entrancy prevention. --- .../ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs index 5742c617908..6954c644725 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/LanguageServices/LanguageServiceHost.cs @@ -91,7 +91,10 @@ public LanguageServiceHost( return !await vsShell.IsCommandLineModeAsync() || await vsShell.IsPopulateSolutionCacheModeAsync(); }, - threadingService.JoinableTaskFactory); + threadingService.JoinableTaskFactory) + { + SuppressRecursiveFactoryDetection = true + }; } public Task LoadAsync() From b2606eacf192ffdd7be79c40d6087b0d3cf71ee5 Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 17 Dec 2024 14:42:56 +1100 Subject: [PATCH 20/21] Set AsyncLazy.SuppressRecursiveFactoryDetection to true This avoids the overhead associated with the type's re-entrancy prevention in cases where we the code cannot call itself. See https://github.com/microsoft/vs-threading/pull/1265 for more information. --- .../Build/LanguageServiceErrorListProvider.cs | 5 +- .../PropertyPages/AbstractProjectState.cs | 62 +++++++++---------- .../Build/PublishItemsOutputGroupProvider.cs | 5 +- .../Debug/DebugProfileEnumValuesGenerator.cs | 10 +-- 4 files changed, 43 insertions(+), 39 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Build/LanguageServiceErrorListProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Build/LanguageServiceErrorListProvider.cs index 09e911bc85d..c8ad691bb5b 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Build/LanguageServiceErrorListProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Build/LanguageServiceErrorListProvider.cs @@ -36,7 +36,10 @@ public LanguageServiceErrorListProvider( _isLspPullDiagnosticsEnabled = new AsyncLazy( async () => await projectSystemOptions.IsLspPullDiagnosticsEnabledAsync(CancellationToken.None), - joinableTaskContext.Factory); + joinableTaskContext.Factory) + { + SuppressRecursiveFactoryDetection = true + }; } public void SuspendRefresh() diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/AbstractProjectState.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/AbstractProjectState.cs index 6b7db0c3b37..fb29bd75efe 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/AbstractProjectState.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Query/PropertyPages/AbstractProjectState.cs @@ -9,8 +9,8 @@ internal abstract class AbstractProjectState : IProjectState { protected readonly UnconfiguredProject Project; - private readonly Dictionary _catalogCache; - private readonly Dictionary<(ProjectConfiguration, string, QueryProjectPropertiesContext), IRule?> _ruleCache; + private readonly Dictionary _catalogCache = []; + private readonly Dictionary<(ProjectConfiguration, string, QueryProjectPropertiesContext), IRule?> _ruleCache = []; private readonly AsyncLazy?> _knownProjectConfigurations; private readonly AsyncLazy _defaultProjectConfiguration; @@ -20,10 +20,34 @@ protected AbstractProjectState(UnconfiguredProject project) Project = project; JoinableTaskFactory joinableTaskFactory = project.Services.ThreadingPolicy.JoinableTaskFactory; - _knownProjectConfigurations = new AsyncLazy?>(CreateKnownConfigurationsAsync, joinableTaskFactory); - _defaultProjectConfiguration = new AsyncLazy(CreateDefaultConfigurationAsync, joinableTaskFactory); - _catalogCache = new Dictionary(); - _ruleCache = new Dictionary<(ProjectConfiguration, string, QueryProjectPropertiesContext), IRule?>(); + _knownProjectConfigurations = new AsyncLazy?>(CreateKnownConfigurationsAsync, joinableTaskFactory) + { + SuppressRecursiveFactoryDetection = true + }; + + _defaultProjectConfiguration = new AsyncLazy(CreateDefaultConfigurationAsync, joinableTaskFactory) + { + SuppressRecursiveFactoryDetection = true + }; + + async Task?> CreateKnownConfigurationsAsync() + { + return Project.Services.ProjectConfigurationsService switch + { + IProjectConfigurationsService configurationsService => await configurationsService.GetKnownProjectConfigurationsAsync(), + _ => null + }; + } + + async Task CreateDefaultConfigurationAsync() + { + return Project.Services.ProjectConfigurationsService switch + { + IProjectConfigurationsService2 configurationsService2 => await configurationsService2.GetSuggestedProjectConfigurationAsync(), + IProjectConfigurationsService configurationsService => configurationsService.SuggestedProjectConfiguration, + _ => null + }; + } } /// @@ -60,32 +84,6 @@ protected AbstractProjectState(UnconfiguredProject project) /// public Task GetSuggestedConfigurationAsync() => _defaultProjectConfiguration.GetValueAsync(); - private async Task CreateDefaultConfigurationAsync() - { - if (Project.Services.ProjectConfigurationsService is IProjectConfigurationsService2 configurationsService2) - { - return await configurationsService2.GetSuggestedProjectConfigurationAsync(); - } - else if (Project.Services.ProjectConfigurationsService is IProjectConfigurationsService configurationsService) - { - return configurationsService.SuggestedProjectConfiguration; - } - else - { - return null; - } - } - - private async Task?> CreateKnownConfigurationsAsync() - { - if (Project.Services.ProjectConfigurationsService is IProjectConfigurationsService configurationsService) - { - return await configurationsService.GetKnownProjectConfigurationsAsync(); - } - - return null; - } - /// /// Retrieves the set of property pages that apply to the project level for the given . diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Build/PublishItemsOutputGroupProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Build/PublishItemsOutputGroupProvider.cs index 864ffad252f..9fe632a2f2b 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Build/PublishItemsOutputGroupProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Build/PublishItemsOutputGroupProvider.cs @@ -34,7 +34,10 @@ internal PublishItemsOutputGroupProvider( { _outputGroups = new AsyncLazy>( GetOutputGroupMetadataAsync, - projectThreadingService.JoinableTaskFactory); + projectThreadingService.JoinableTaskFactory) + { + SuppressRecursiveFactoryDetection = true + }; async Task> GetOutputGroupMetadataAsync() { diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Debug/DebugProfileEnumValuesGenerator.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Debug/DebugProfileEnumValuesGenerator.cs index df7017b83cc..970f66de0fd 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Debug/DebugProfileEnumValuesGenerator.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Debug/DebugProfileEnumValuesGenerator.cs @@ -19,21 +19,21 @@ internal DebugProfileEnumValuesGenerator( ILaunchSettingsProvider profileProvider, IProjectThreadingService threadingService) { - Requires.NotNull(profileProvider); - Requires.NotNull(threadingService); - _listedValues = new AsyncLazy>( () => { ILaunchSettings? snapshot = profileProvider.CurrentSnapshot; ICollection values = snapshot is null - ? Array.Empty() + ? [] : GetEnumeratorEnumValues(snapshot); return Task.FromResult(values); }, - threadingService.JoinableTaskFactory); + threadingService.JoinableTaskFactory) + { + SuppressRecursiveFactoryDetection = true + }; } public Task> GetListedValuesAsync() From bda7ef3f032501a0a023b4ce39112c304b2c3cef Mon Sep 17 00:00:00 2001 From: Drew Noakes Date: Tue, 17 Dec 2024 21:31:59 +1100 Subject: [PATCH 21/21] Remove unused GuidSymbol from VSCT file Seen while debugging something ealier in the day with Zewditu. --- .../Menus.vsct | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/Menus.vsct b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/Menus.vsct index 07bac97f04b..981668c671d 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/Menus.vsct +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/Menus.vsct @@ -227,7 +227,7 @@ - + @@ -270,8 +270,6 @@ - -