diff --git a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs index 9c633d14b8c..60d3fc4285d 100644 --- a/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs +++ b/src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs @@ -1128,8 +1128,8 @@ private void IssueBuildRequests(BuildRequestEntry issuingEntry, FullyQualifiedBu // to the entry rather than a series of them. lock (issuingEntry.GlobalLock) { - var existingResultsToReport = new List(); - var unresolvedConfigurationsAdded = new HashSet(); + List existingResultsToReport = null; + HashSet unresolvedConfigurationsAdded = new HashSet(); foreach (FullyQualifiedBuildRequest request in newRequests) { @@ -1232,6 +1232,7 @@ private void IssueBuildRequests(BuildRequestEntry issuingEntry, FullyQualifiedBu // Can't report the result directly here, because that could cause the request to go from // Waiting to Ready. + existingResultsToReport ??= new List(); existingResultsToReport.Add(response.Results); } else @@ -1243,9 +1244,12 @@ private void IssueBuildRequests(BuildRequestEntry issuingEntry, FullyQualifiedBu } // If we have any results we had to report, do so now. - foreach (BuildResult existingResult in existingResultsToReport) + if (existingResultsToReport is not null) { - issuingEntry.ReportResult(existingResult); + foreach (BuildResult existingResult in existingResultsToReport) + { + issuingEntry.ReportResult(existingResult); + } } // Issue any configuration requests we may still need. diff --git a/src/Build/BackEnd/Components/RequestBuilder/BatchingEngine.cs b/src/Build/BackEnd/Components/RequestBuilder/BatchingEngine.cs index 40b58682006..8962679602e 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/BatchingEngine.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/BatchingEngine.cs @@ -259,7 +259,7 @@ private static Dictionary> GetItemLists if (items != null) { // Loop through all the items in the BuildItemGroup. - foreach (ProjectItemInstance item in items) + foreach (ProjectItemInstance item in items.GetStructEnumerable()) { ProjectErrorUtilities.VerifyThrowInvalidProject( item.HasMetadata(consumedMetadataReference.MetadataName), @@ -319,7 +319,7 @@ private static List BucketConsumedItems( if (items != null) { - foreach (ProjectItemInstance item in items) + foreach (ProjectItemInstance item in items.GetStructEnumerable()) { // Get this item's values for all the metadata consumed by the batchable object. Dictionary itemMetadataValues = GetItemMetadataValues(item, consumedMetadataReferences, elementLocation); diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs index 423679a1f6e..f0d9d5f8693 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs @@ -486,10 +486,12 @@ private List ExpandItemIntoItems( { foreach (var item in items) { - var metadataToRemove = item.MetadataNames.Where(name => !keepMetadata.Contains(name)); - foreach (var metadataName in metadataToRemove) + foreach (var metadataName in item.MetadataNames) { - item.RemoveMetadata(metadataName); + if (!keepMetadata.Contains(metadataName)) + { + item.RemoveMetadata(metadataName); + } } } } @@ -497,10 +499,12 @@ private List ExpandItemIntoItems( { foreach (var item in items) { - var metadataToRemove = item.MetadataNames.Where(name => removeMetadata.Contains(name)); - foreach (var metadataName in metadataToRemove) + foreach (var metadataName in item.MetadataNames) { - item.RemoveMetadata(metadataName); + if (removeMetadata.Contains(metadataName)) + { + item.RemoveMetadata(metadataName); + } } } } @@ -517,7 +521,7 @@ private List ExpandItemIntoItems( /// A list of matching items private HashSet EvaluateExcludePaths(IReadOnlyList excludes, ElementLocation excludeLocation) { - HashSet excludesUnescapedForComparison = new HashSet(StringComparer.OrdinalIgnoreCase); + HashSet excludesUnescapedForComparison = new HashSet(excludes.Count, StringComparer.OrdinalIgnoreCase); foreach (string excludeSplit in excludes) { string[] excludeSplitFiles = EngineFileUtilities.GetFileListUnescaped( @@ -595,7 +599,7 @@ private List FindItemsMatchingSpecification( // filename in the remove list. List itemsRemoved = new List(); - foreach (ProjectItemInstance item in items) + foreach (ProjectItemInstance item in items.GetStructEnumerable()) { // Even if the case for the excluded files is different, they // will still get excluded, as expected. However, if the excluded path diff --git a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs index 6a882eb87f0..2d3ab506dd6 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs @@ -481,8 +481,14 @@ public ICollection GetItems(string itemType) ICollection adds = scope.Adds[itemType]; if (adds.Count != 0) { - allAdds ??= new List(adds.Count); - allAdds.AddRange(adds); + if (allAdds == null) + { + allAdds = new List(adds); + } + else + { + allAdds.AddRange(adds); + } } } @@ -492,8 +498,14 @@ public ICollection GetItems(string itemType) ICollection removes = scope.Removes[itemType]; if (removes.Count != 0) { - allRemoves ??= new List(removes.Count); - allRemoves.AddRange(removes); + if (allRemoves == null) + { + allRemoves = new List(removes); + } + else + { + allRemoves.AddRange(removes); + } } } @@ -715,7 +727,7 @@ internal void AddNewItem(ProjectItemInstance item) /// internal void RemoveItems(IEnumerable items) { - foreach (ProjectItemInstance item in items) + foreach (ProjectItemInstance item in items.GetStructEnumerable()) { RemoveItem(item); } diff --git a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs index 74adf85158f..52401fbc033 100644 --- a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs +++ b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs @@ -699,7 +699,7 @@ private void ScheduleUnassignedRequests(List responses) // Resume any work available which has already been assigned to specific nodes. ResumeRequiredWork(responses); - HashSet idleNodes = new HashSet(); + HashSet idleNodes = new HashSet(_availableNodes.Count); foreach (int availableNodeId in _availableNodes.Keys) { if (!_schedulingData.IsNodeWorking(availableNodeId)) @@ -992,7 +992,8 @@ private void AssignUnscheduledRequestsToInProcNode(List respon { if (idleNodes.Contains(InProcNodeId)) { - List unscheduledRequests = new List(_schedulingData.UnscheduledRequestsWhichCanBeScheduled); + List unscheduledRequests = new List(_schedulingData.UnscheduledRequestsCount); + unscheduledRequests.AddRange(_schedulingData.UnscheduledRequestsWhichCanBeScheduled); foreach (SchedulableRequest request in unscheduledRequests) { if (CanScheduleRequestToNode(request, InProcNodeId) && shouldBeScheduled(request)) @@ -1250,7 +1251,7 @@ private void AssignUnscheduledRequestsWithMaxWaitingRequests2(List responses, HashSet idleNodes) { // Assign requests on a first-come/first-serve basis - foreach (int nodeId in idleNodes) + foreach (SchedulableRequest unscheduledRequest in _schedulingData.UnscheduledRequestsWhichCanBeScheduled) { // Don't overload the system. if (AtSchedulingLimit()) @@ -1259,7 +1260,7 @@ private void AssignUnscheduledRequestsFIFO(List responses, Has return; } - foreach (SchedulableRequest unscheduledRequest in _schedulingData.UnscheduledRequestsWhichCanBeScheduled) + foreach (int nodeId in idleNodes) { if (CanScheduleRequestToNode(unscheduledRequest, nodeId)) { @@ -1972,21 +1973,13 @@ private bool CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot(int nodeF bool logComment = ((isolateProjects == ProjectIsolationMode.True || isolateProjects == ProjectIsolationMode.MessageUponIsolationViolation) && request.SkipStaticGraphIsolationConstraints); if (logComment) { - // retrieving the configs is not quite free, so avoid computing them eagerly - var configs = GetConfigurations(); - - emitNonErrorLogs = ls => ls.LogComment( - NewBuildEventContext(), - MessageImportance.Normal, - "SkippedConstraintsOnRequest", - configs.ParentConfig.ProjectFullPath, - configs.RequestConfig.ProjectFullPath); + emitNonErrorLogs = GetLoggingServiceAction(configCache, request, _schedulingData); } return true; } - (BuildRequestConfiguration requestConfig, BuildRequestConfiguration parentConfig) = GetConfigurations(); + (BuildRequestConfiguration requestConfig, BuildRequestConfiguration parentConfig) = GetConfigurations(configCache, request, _schedulingData); // allow self references (project calling the msbuild task on itself, potentially with different global properties) if (parentConfig.ProjectFullPath.Equals(requestConfig.ProjectFullPath, StringComparison.OrdinalIgnoreCase)) @@ -2014,7 +2007,7 @@ private bool CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot(int nodeF return false; - BuildEventContext NewBuildEventContext() + static BuildEventContext NewBuildEventContext(BuildRequest request) { return new BuildEventContext( request.SubmissionId, @@ -2025,13 +2018,33 @@ BuildEventContext NewBuildEventContext() BuildEventContext.InvalidTaskId); } - (BuildRequestConfiguration RequestConfig, BuildRequestConfiguration ParentConfig) GetConfigurations() + static (BuildRequestConfiguration RequestConfig, BuildRequestConfiguration ParentConfig) GetConfigurations(IConfigCache configCache, BuildRequest request, SchedulingData schedulingData) { BuildRequestConfiguration buildRequestConfiguration = configCache[request.ConfigurationId]; // Need the parent request. It might be blocked or executing; check both. - SchedulableRequest parentRequest = _schedulingData.BlockedRequests.FirstOrDefault(r => r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId) - ?? _schedulingData.ExecutingRequests.FirstOrDefault(r => r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId); + SchedulableRequest parentRequest = null; + + foreach (var r in schedulingData.BlockedRequests) + { + if (r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId) + { + parentRequest = r; + break; + } + } + + if (parentRequest is null) + { + foreach (var r in schedulingData.ExecutingRequests) + { + if (r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId) + { + parentRequest = r; + break; + } + } + } ErrorUtilities.VerifyThrowInternalNull(parentRequest); ErrorUtilities.VerifyThrow( @@ -2044,7 +2057,7 @@ BuildEventContext NewBuildEventContext() string ConcatenateGlobalProperties(BuildRequestConfiguration configuration) { - return string.Join("; ", configuration.GlobalProperties.Select(p => $"{p.Name}={p.EvaluatedValue}")); + return string.Join("; ", configuration.GlobalProperties.Select(static p => $"{p.Name}={p.EvaluatedValue}")); } bool SkipNonexistentTargetsIfExistentTargetsHaveResults(BuildRequest buildRequest) @@ -2080,6 +2093,20 @@ bool SkipNonexistentTargetsIfExistentTargetsHaveResults(BuildRequest buildReques // to skip nonexistent targets. return true; } + static Action GetLoggingServiceAction(IConfigCache configCache, BuildRequest request, SchedulingData schedulingData) + { + Action emitNonErrorLogs; + // retrieving the configs is not quite free, so avoid computing them eagerly + var configs = GetConfigurations(configCache, request, schedulingData); + + emitNonErrorLogs = ls => ls.LogComment( + NewBuildEventContext(request), + MessageImportance.Normal, + "SkippedConstraintsOnRequest", + configs.ParentConfig.ProjectFullPath, + configs.RequestConfig.ProjectFullPath); + return emitNonErrorLogs; + } } /// diff --git a/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs b/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs index a9ea6b37548..22f52492c45 100644 --- a/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs +++ b/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs @@ -48,12 +48,12 @@ internal class SdkResolverService : ISdkResolverService /// /// Stores the list of manifests of specific SDK resolvers which could be loaded. /// - protected IReadOnlyList _specificResolversManifestsRegistry; + private List _specificResolversManifestsRegistry; /// /// Stores the list of manifests of general SDK resolvers which could be loaded. /// - protected IReadOnlyList _generalResolversManifestsRegistry; + private List _generalResolversManifestsRegistry; /// /// Stores an which can load registered SDK resolvers. @@ -226,7 +226,7 @@ private SdkResult ResolveSdkUsingResolversWithPatternsFirst(int submissionId, Sd resolvers = GetResolvers( _generalResolversManifestsRegistry, loggingContext, - sdkReferenceLocation).ToList(); + sdkReferenceLocation); if (TryResolveSdkUsingSpecifiedResolvers( resolvers, @@ -259,22 +259,20 @@ private SdkResult ResolveSdkUsingResolversWithPatternsFirst(int submissionId, Sd return new SdkResult(sdk, null, null); } - private List GetResolvers(IReadOnlyList resolversManifests, LoggingContext loggingContext, ElementLocation sdkReferenceLocation) + private List GetResolvers(List resolversManifests, LoggingContext loggingContext, ElementLocation sdkReferenceLocation) { // Create a sorted by priority list of resolvers. Load them if needed. List resolvers = new List(); foreach (var resolverManifest in resolversManifests) { - if (!_manifestToResolvers.TryGetValue(resolverManifest, out IReadOnlyList newResolvers)) + IReadOnlyList newResolvers; + lock (_lockObject) { - lock (_lockObject) + if (!_manifestToResolvers.TryGetValue(resolverManifest, out newResolvers)) { - if (!_manifestToResolvers.TryGetValue(resolverManifest, out newResolvers)) - { - // Loading of the needed resolvers. - newResolvers = _sdkResolverLoader.LoadResolversFromManifest(resolverManifest, sdkReferenceLocation); - _manifestToResolvers[resolverManifest] = newResolvers; - } + // Loading of the needed resolvers. + newResolvers = _sdkResolverLoader.LoadResolversFromManifest(resolverManifest, sdkReferenceLocation); + _manifestToResolvers[resolverManifest] = newResolvers; } } @@ -423,8 +421,8 @@ internal virtual void InitializeForTests(SdkResolverLoader resolverLoader = null SdkResolverManifest sdkResolverManifest = new SdkResolverManifest(DisplayName: "TestResolversManifest", Path: null, ResolvableSdkRegex: null); generalResolversManifestsRegistry.Add(sdkResolverManifest); _manifestToResolvers[sdkResolverManifest] = resolvers; - _generalResolversManifestsRegistry = generalResolversManifestsRegistry.AsReadOnly(); - _specificResolversManifestsRegistry = specificResolversManifestsRegistry.AsReadOnly(); + _generalResolversManifestsRegistry = generalResolversManifestsRegistry; + _specificResolversManifestsRegistry = specificResolversManifestsRegistry; } } @@ -521,8 +519,8 @@ private void RegisterResolversManifests(ElementLocation location) // Then it will wait at the lock and return after we release it since the collections we have filled them before releasing the lock. // The collections are never modified after this point. // So I've made them ReadOnly - _specificResolversManifestsRegistry = specificResolversManifestsRegistry.AsReadOnly(); - _generalResolversManifestsRegistry = generalResolversManifestsRegistry.AsReadOnly(); + _specificResolversManifestsRegistry = specificResolversManifestsRegistry; + _generalResolversManifestsRegistry = generalResolversManifestsRegistry; } } diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 0e4c160336f..071d97154be 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -15,6 +15,7 @@ using System.Threading; using Microsoft.Build.BackEnd.Logging; +using Microsoft.Build.Collections; using Microsoft.Build.Construction; using Microsoft.Build.Evaluation; using Microsoft.Build.Exceptions; @@ -1416,9 +1417,26 @@ private void GatherTaskItemOutputs(bool outputTargetIsItem, string outputTargetN static IEnumerable> EnumerateMetadata(IDictionary customMetadata) { - foreach (DictionaryEntry de in customMetadata) + if (customMetadata is CopyOnWriteDictionary copyOnWriteDictionary) { - yield return new KeyValuePair((string)de.Key, EscapingUtilities.Escape((string)de.Value)); + foreach (KeyValuePair kvp in copyOnWriteDictionary) + { + yield return new KeyValuePair(kvp.Key, EscapingUtilities.Escape(kvp.Value)); + } + } + else if (customMetadata is Dictionary dictionary) + { + foreach (KeyValuePair kvp in dictionary) + { + yield return new KeyValuePair(kvp.Key, EscapingUtilities.Escape(kvp.Value)); + } + } + else + { + foreach (DictionaryEntry de in customMetadata) + { + yield return new KeyValuePair((string)de.Key, EscapingUtilities.Escape((string)de.Value)); + } } } } diff --git a/src/Build/Collections/CopyOnReadEnumerable.cs b/src/Build/Collections/CopyOnReadEnumerable.cs index 6ed48e0eed1..d0be1421a6b 100644 --- a/src/Build/Collections/CopyOnReadEnumerable.cs +++ b/src/Build/Collections/CopyOnReadEnumerable.cs @@ -72,6 +72,10 @@ public IEnumerator GetEnumerator() #endif list = new List(count); } + else if (_backingEnumerable is IReadOnlyCollection readOnlyCollection) + { + list = new List(readOnlyCollection.Count); + } else { list = new List(); diff --git a/src/Build/Collections/CopyOnWritePropertyDictionary.cs b/src/Build/Collections/CopyOnWritePropertyDictionary.cs index 41dd414b5cf..930438e2b47 100644 --- a/src/Build/Collections/CopyOnWritePropertyDictionary.cs +++ b/src/Build/Collections/CopyOnWritePropertyDictionary.cs @@ -137,12 +137,18 @@ public void Clear() /// Gets an enumerator over all the properties in the collection /// Enumeration is in undefined order /// - public IEnumerator GetEnumerator() => _backing.Values.GetEnumerator(); + public ImmutableDictionary.Enumerator GetEnumerator() => _backing.GetEnumerator(); + + /// + /// Gets an enumerator over all the properties in the collection + /// Enumeration is in undefined order + /// + IEnumerator IEnumerable.GetEnumerator() => new Enumerator(this); /// /// Get an enumerator over entries /// - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => new Enumerator(this); #region IEquatable> Members @@ -347,9 +353,24 @@ public void Set(T projectProperty) /// An enumerator over the properties to add. public void ImportProperties(IEnumerable other) { - _backing = _backing.SetItems(Items()); + if (other is CopyOnWritePropertyDictionary copyOnWriteDictionary) + { + _backing = _backing.SetItems(DictionaryItems(copyOnWriteDictionary)); + } + else + { + _backing = _backing.SetItems(Items(other)); + } - IEnumerable> Items() + static IEnumerable> DictionaryItems(CopyOnWritePropertyDictionary copyOnWriteDictionary) + { + foreach (KeyValuePair kvp in copyOnWriteDictionary) + { + yield return new(kvp.Value.Key, kvp.Value); + } + } + + static IEnumerable> Items(IEnumerable other) { foreach (T property in other) { @@ -366,5 +387,36 @@ public ICopyOnWritePropertyDictionary DeepClone() { return new CopyOnWritePropertyDictionary(this); } + + public struct Enumerator : IEnumerator + { + private ImmutableDictionary.Enumerator _dictionaryEnumerator; + public Enumerator(CopyOnWritePropertyDictionary dictionary) + { + _dictionaryEnumerator = dictionary._backing.GetEnumerator(); + } + + public T Current { get; private set; } + + readonly object IEnumerator.Current => Current; + + public void Dispose() => _dictionaryEnumerator.Dispose(); + + public readonly Enumerator GetEnumerator() => this; + + public bool MoveNext() + { + if (_dictionaryEnumerator.MoveNext()) + { + Current = _dictionaryEnumerator.Current.Value; + + return true; + } + + return false; + } + + public void Reset() => _dictionaryEnumerator.Reset(); + } } } diff --git a/src/Build/Collections/ItemDictionary.cs b/src/Build/Collections/ItemDictionary.cs index 39834b4dafa..fb9f71ceb4f 100644 --- a/src/Build/Collections/ItemDictionary.cs +++ b/src/Build/Collections/ItemDictionary.cs @@ -29,7 +29,7 @@ namespace Microsoft.Build.Collections /// /// Item class type to store [DebuggerDisplay("#Item types={ItemTypes.Count} #Items={Count}")] - internal sealed class ItemDictionary : IItemDictionary + internal sealed class ItemDictionary : IItemDictionary, IReadOnlyCollection where T : class, IKeyed, IItem { /// @@ -333,7 +333,7 @@ public void ImportItemsOfType(string itemType, IEnumerable items) _itemLists[itemType] = list; } - foreach (T item in items) + foreach (T item in items.GetStructEnumerable()) { #if DEBUG // Debug only: hot code path @@ -351,7 +351,7 @@ public void ImportItemsOfType(string itemType, IEnumerable items) /// An enumerator over the items to remove. public void RemoveItems(IEnumerable other) { - foreach (T item in other) + foreach (T item in other.GetStructEnumerable()) { Remove(item); } diff --git a/src/Build/Collections/PropertyDictionary.cs b/src/Build/Collections/PropertyDictionary.cs index 657b92a9f2e..d33ce400ddf 100644 --- a/src/Build/Collections/PropertyDictionary.cs +++ b/src/Build/Collections/PropertyDictionary.cs @@ -551,19 +551,37 @@ internal void Enumerate(Action keyValueCallback) internal IEnumerable Filter(Func filter, Func selector) { - List result = new(); lock (_properties) { - foreach (T property in (ICollection)_properties) + ICollection propertiesCollection = (ICollection)_properties; + List result = new(propertiesCollection.Count); + + // PERF: Prefer using struct enumerators from the concrete types to avoid allocations. + // RetrievableValuedEntryHashSet implements a struct enumerator. + if (_properties is RetrievableValuedEntryHashSet hashSet) { - if (filter(property)) + foreach (T property in hashSet) { - result.Add(selector(property)); + if (filter(property)) + { + result.Add(selector(property)); + } } } - } + else + { + foreach (T property in propertiesCollection) + { + if (filter(property)) + { + result.Add(selector(property)); + } + } + } + - return result; + return result; + } } } } diff --git a/src/Build/Construction/ProjectElementContainer.cs b/src/Build/Construction/ProjectElementContainer.cs index c8fabf3559e..356b36b91b3 100644 --- a/src/Build/Construction/ProjectElementContainer.cs +++ b/src/Build/Construction/ProjectElementContainer.cs @@ -858,7 +858,9 @@ public bool Remove(T item) return false; } - public IEnumerator GetEnumerator() => new Enumerator(_initial, _forwards); + public Enumerator GetEnumerator() => new Enumerator(_initial, _forwards); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); @@ -948,13 +950,18 @@ internal ProjectElementSiblingEnumerable(ProjectElement initial, bool forwards = /// /// Get enumerator /// - public readonly IEnumerator GetEnumerator() => _enumerator; + public readonly Enumerator GetEnumerator() => _enumerator; /// /// Get non generic enumerator /// IEnumerator IEnumerable.GetEnumerator() => _enumerator; + /// + /// Get enumerator + /// + IEnumerator IEnumerable.GetEnumerator() => _enumerator; + /// /// Enumerator over a series of sibling ProjectElement objects /// diff --git a/src/Build/Evaluation/ExpressionShredder.cs b/src/Build/Evaluation/ExpressionShredder.cs index fd102dff143..3c6209e0cb4 100644 --- a/src/Build/Evaluation/ExpressionShredder.cs +++ b/src/Build/Evaluation/ExpressionShredder.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using Microsoft.Build.Collections; +using Microsoft.Build.Construction; using Microsoft.Build.Shared; #nullable disable @@ -68,7 +69,7 @@ internal static ItemsAndMetadataPair GetReferencedItemNamesAndMetadata(IEnumerab { ItemsAndMetadataPair pair = new ItemsAndMetadataPair(null, null); - foreach (string expression in expressions) + foreach (string expression in expressions.GetStructEnumerable()) { GetReferencedItemNamesAndMetadata(expression, 0, expression.Length, ref pair, ShredderOptions.All); } diff --git a/src/Build/Instance/ProjectItemInstance.cs b/src/Build/Instance/ProjectItemInstance.cs index 8dd5127eba5..9d68904c9df 100644 --- a/src/Build/Instance/ProjectItemInstance.cs +++ b/src/Build/Instance/ProjectItemInstance.cs @@ -608,7 +608,7 @@ internal static void SetMetadata(IEnumerable> metad IEnumerable projectMetadataInstances = metadataList.Select(metadatum => new ProjectMetadataInstance(metadatum.Key, metadatum.Value)); metadata.ImportProperties(projectMetadataInstances); - foreach (ProjectItemInstance item in items) + foreach (ProjectItemInstance item in items.GetStructEnumerable()) { item._taskItem.SetMetadata(metadata); // Potential copy on write } @@ -1109,11 +1109,36 @@ private IEnumerable> EnumerateMetadataEager(ICopyOn private IEnumerable> EnumerateMetadata(ICopyOnWritePropertyDictionary list) { - foreach (var projectMetadataInstance in list.Values) + if (list is CopyOnWritePropertyDictionary copyOnWritePropertyDictionary) { - if (projectMetadataInstance != null) + return EnumerateCopyOnWritePropertyDictionary(copyOnWritePropertyDictionary); + } + else + { + return EnumerateDictionary(list); + } + + static IEnumerable> EnumerateCopyOnWritePropertyDictionary(CopyOnWritePropertyDictionary copyOnWritePropertyDictionary) + { + foreach (var kvp in copyOnWritePropertyDictionary) { - yield return new KeyValuePair(projectMetadataInstance.Name, projectMetadataInstance.EvaluatedValue); + var projectMetadataInstance = kvp.Value; + if (projectMetadataInstance != null) + { + yield return new KeyValuePair(projectMetadataInstance.Name, projectMetadataInstance.EvaluatedValue); + } + } + } + + static IEnumerable> EnumerateDictionary(ICopyOnWritePropertyDictionary list) + { + foreach (var kvp in (IDictionary)list) + { + var projectMetadataInstance = kvp.Value; + if (projectMetadataInstance != null) + { + yield return new KeyValuePair(projectMetadataInstance.Name, projectMetadataInstance.EvaluatedValue); + } } } } @@ -1447,9 +1472,7 @@ public void CopyMetadataTo(ITaskItem destinationItem, bool addOriginalItemSpec) { // The destination implements IMetadataContainer so we can use the ImportMetadata bulk-set operation. IEnumerable metadataEnumerable = MetadataCollection; - IEnumerable> metadataToImport = metadataEnumerable - .Where(metadatum => string.IsNullOrEmpty(destinationItem.GetMetadata(metadatum.Name))) - .Select(metadatum => new KeyValuePair(metadatum.Name, GetMetadataEscaped(metadatum.Name))); + IEnumerable> metadataToImport = GetMetadataToImportEnumerable(destinationItem, metadataEnumerable); #if FEATURE_APPDOMAIN if (RemotingServices.IsTransparentProxy(destinationItem)) @@ -1484,6 +1507,10 @@ public void CopyMetadataTo(ITaskItem destinationItem, bool addOriginalItemSpec) destinationItem.SetMetadata("OriginalItemSpec", _includeEscaped); } } + + IEnumerable> GetMetadataToImportEnumerable(ITaskItem destinationItem, IEnumerable metadataEnumerable) => metadataEnumerable + .Where(metadatum => string.IsNullOrEmpty(destinationItem.GetMetadata(metadatum.Name))) + .Select(metadatum => new KeyValuePair(metadatum.Name, GetMetadataEscaped(metadatum.Name))); } /// @@ -1496,10 +1523,21 @@ public IDictionary CloneCustomMetadata() var metadata = MetadataCollection; Dictionary clonedMetadata = new Dictionary(metadata.Count, MSBuildNameIgnoreCaseComparer.Default); - foreach (ProjectMetadataInstance metadatum in (IEnumerable)metadata) + if (metadata is CopyOnWritePropertyDictionary metadataDictionary) + { + foreach (KeyValuePair metadatum in metadataDictionary) + { + clonedMetadata[metadatum.Value.Name] = metadatum.Value.EvaluatedValue; + } + } + else { - clonedMetadata[metadatum.Name] = metadatum.EvaluatedValue; + foreach (ProjectMetadataInstance metadatum in (IEnumerable)metadata) + { + clonedMetadata[metadatum.Name] = metadatum.EvaluatedValue; + } } + return clonedMetadata; } @@ -1768,12 +1806,20 @@ internal void TranslateWithInterning(ITranslator translator, LookasideStringInte { int count = temp.Count; translator.Writer.Write(count); - foreach (ProjectMetadataInstance metadatum in (IEnumerable)temp) + + if (temp is CopyOnWritePropertyDictionary copyOnWriteDictionary) { - int key = interner.Intern(metadatum.Name); - int value = interner.Intern(metadatum.EvaluatedValueEscaped); - translator.Writer.Write(key); - translator.Writer.Write(value); + foreach (KeyValuePair metadatum in copyOnWriteDictionary) + { + TranslateMetadata(translator, interner, metadatum.Value); + } + } + else + { + foreach (ProjectMetadataInstance metadatum in (IEnumerable)temp) + { + TranslateMetadata(translator, interner, metadatum); + } } } } @@ -1786,13 +1832,7 @@ internal void TranslateWithInterning(ITranslator translator, LookasideStringInte int count = translator.Reader.ReadInt32(); if (count > 0) { - IEnumerable metaData = - Enumerable.Range(0, count).Select(_ => - { - int key = translator.Reader.ReadInt32(); - int value = translator.Reader.ReadInt32(); - return new ProjectMetadataInstance(interner.GetString(key), interner.GetString(value), allowItemSpecModifiers: true); - }); + IEnumerable metaData = GetMetadataEnumerable(translator, interner, count); _directMetadata = new CopyOnWritePropertyDictionary(); _directMetadata.ImportProperties(metaData); } @@ -1802,6 +1842,24 @@ internal void TranslateWithInterning(ITranslator translator, LookasideStringInte } } } + + static IEnumerable GetMetadataEnumerable(ITranslator translator, LookasideStringInterner interner, int count) + { + for (int i = 0; i < count; ++i) + { + int key = translator.Reader.ReadInt32(); + int value = translator.Reader.ReadInt32(); + yield return new ProjectMetadataInstance(interner.GetString(key), interner.GetString(value), allowItemSpecModifiers: true); + } + } + + static void TranslateMetadata(ITranslator translator, LookasideStringInterner interner, ProjectMetadataInstance metadatum) + { + int key = interner.Intern(metadatum.Name); + int value = interner.Intern(metadatum.EvaluatedValueEscaped); + translator.Writer.Write(key); + translator.Writer.Write(value); + } } /// diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 66d6c38d007..db1650a1840 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -83,6 +83,9 @@ IConstrainedEqualityComparer.cs + + IEnumerableExtensions.cs + BackEnd\Components\RequestBuilder\IntrinsicTasks\PropertyParser.cs diff --git a/src/Framework/BinaryTranslator.cs b/src/Framework/BinaryTranslator.cs index 41a5914dad6..d82637116b0 100644 --- a/src/Framework/BinaryTranslator.cs +++ b/src/Framework/BinaryTranslator.cs @@ -299,7 +299,11 @@ public void Translate(ref HashSet set) } int count = _reader.ReadInt32(); +#if NET472_OR_GREATER || NET9_0_OR_GREATER + set = new HashSet(count); +#else set = new HashSet(); +#endif for (int i = 0; i < count; i++) { diff --git a/src/Shared/CopyOnWriteDictionary.cs b/src/Shared/CopyOnWriteDictionary.cs index b48401b964f..28797141eba 100644 --- a/src/Shared/CopyOnWriteDictionary.cs +++ b/src/Shared/CopyOnWriteDictionary.cs @@ -294,13 +294,26 @@ public bool Remove(KeyValuePair item) return initial != _backing; // whether the removal occured } +#if NET472_OR_GREATER || NETCOREAPP /// /// Implementation of generic IEnumerable.GetEnumerator() /// + public ImmutableDictionary.Enumerator GetEnumerator() + { + return _backing.GetEnumerator(); + } + + IEnumerator> IEnumerable>.GetEnumerator() + { + ImmutableDictionary.Enumerator enumerator = _backing.GetEnumerator(); + return _backing.GetEnumerator(); + } +#else public IEnumerator> GetEnumerator() { return _backing.GetEnumerator(); } +#endif /// /// Implementation of IEnumerable.GetEnumerator() diff --git a/src/Shared/IEnumerableExtensions.cs b/src/Shared/IEnumerableExtensions.cs new file mode 100644 index 00000000000..f0d55d6bfc3 --- /dev/null +++ b/src/Shared/IEnumerableExtensions.cs @@ -0,0 +1,237 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; + +namespace Microsoft.Build.Collections +{ + /// + /// A set of extension methods for working with immutable dictionaries. + /// + internal static class IEnumerableExtensions + { + /// + /// Avoids allocating an enumerator when enumerating an in many cases. + /// + /// + /// + /// The statement enumerates types that implement , + /// but will also enumerate any type that has the required methods. Several collection types take advantage of this + /// to avoid allocating an enumerator on the heap when used with by returning a + /// enumerator. This is in contrast to the interface-based enumerator + /// which will always be allocated on the heap. + /// + /// + /// This extension method attempts to create a non-allocating struct enumerator to enumerate + /// . It checks the concrete type of the collection and provides a + /// non-allocating path in several cases. + /// + /// + /// Types that can be enumerated without allocation are: + /// + /// + /// (and by extension and other popular implementations) + /// + /// + /// + /// or having zero count + /// + /// + /// If is not one of the supported types, the returned enumerator falls back to the + /// interface-based enumerator, which will heap allocate. Benchmarking shows the overhead in such cases is low enough + /// to be within the measurement error. + /// + /// + /// + /// + /// collection = ...; + /// + /// foreach (string item in collection.GetStructEnumerable()) + /// { + /// // ... + /// }]]> + /// + /// + /// The item type that is enumerated. + /// The collections that will be enumerated. + /// The enumerator for the collection. + public static Enumerable GetStructEnumerable(this IEnumerable collection) + { + if (collection is null) + { + throw new ArgumentNullException(nameof(collection)); + } + + return Enumerable.Create(collection); + } + + /// + /// Provides a struct-based enumerator for use with . + /// Do not use this type directly. Use instead. + /// + /// The item type that is enumerated. + public readonly ref struct Enumerable + { + private readonly IEnumerable collection; + + private Enumerable(IEnumerable collection) => this.collection = collection; + + /// + /// Constructs an that can be enumerated. + /// + /// The collections that will be enumerated. + /// . + public static Enumerable Create(IEnumerable collection) + { + return new Enumerable(collection); + } + + /// + /// Gets the Enumerator. + /// + /// . + public Enumerator GetEnumerator() => new(this.collection); + + /// + /// A struct-based enumerator for use with . + /// Do not use this type directly. Use instead. + /// + public struct Enumerator : IDisposable + { + private readonly Type enumeratorType; + private readonly IEnumerator? fallbackEnumerator; + private readonly IList? iList; + private int listIndex; + + private LinkedList.Enumerator concreteLinkedListEnumerator; + private ImmutableHashSet.Enumerator concreteImmutableHashSetEnumerator; + private ImmutableList.Enumerator concreteImmutableListEnumerator; + private HashSet.Enumerator concreteHashSetEnumerator; + + /// + /// Initializes a new instance of the struct. + /// + /// The collection that will be enumerated. + internal Enumerator(IEnumerable collection) + { + this.concreteLinkedListEnumerator = default; + this.concreteImmutableHashSetEnumerator = default; + this.concreteImmutableListEnumerator = default; + this.concreteHashSetEnumerator = default; + this.fallbackEnumerator = null; + this.iList = null; + this.listIndex = -1; + + switch (collection) + { + case ICollection sizedCollection when sizedCollection.Count == 0: + case IReadOnlyCollection readOnlyCollection when readOnlyCollection.Count == 0: + // The collection is empty, just return false from MoveNext. + this.enumeratorType = Type.Empty; + break; + case LinkedList concreteLinkedList: + this.enumeratorType = Type.LinkedList; + this.concreteLinkedListEnumerator = concreteLinkedList.GetEnumerator(); + break; + case ImmutableHashSet concreteImmutableHashSet: + this.enumeratorType = Type.ImmutableHashSet; + this.concreteImmutableHashSetEnumerator = concreteImmutableHashSet.GetEnumerator(); + break; + case ImmutableList concreteImmutableList: + this.enumeratorType = Type.ImmutableList; + this.concreteImmutableListEnumerator = concreteImmutableList.GetEnumerator(); + break; + case HashSet concreteHashSet: + this.enumeratorType = Type.HashSet; + this.concreteHashSetEnumerator = concreteHashSet.GetEnumerator(); + break; + case IList list: + this.enumeratorType = Type.IList; + this.iList = list; + break; + default: + this.enumeratorType = Type.Fallback; + this.fallbackEnumerator = collection.GetEnumerator(); + break; + } + } + + private enum Type : byte + { + Empty, + IList, + LinkedList, + ImmutableHashSet, + ImmutableList, + HashSet, + Fallback, + } + + /// + /// Gets the element in the at the current position of the enumerator. + /// + public T Current => + this.enumeratorType switch + { + Type.IList => (uint)this.listIndex < this.iList!.Count ? this.iList![this.listIndex] : default!, + Type.LinkedList => this.concreteLinkedListEnumerator.Current, + Type.ImmutableHashSet => this.concreteImmutableHashSetEnumerator.Current, + Type.ImmutableList => this.concreteImmutableListEnumerator.Current, + Type.HashSet => this.concreteHashSetEnumerator.Current, + Type.Fallback => this.fallbackEnumerator!.Current, + _ => default!, + }; + + /// + /// Advances the enumerator to the next element of the . + /// + /// if the enumerator was successfully advanced to the next element; if + /// the enumerator has passed the end of the . + public bool MoveNext() => + this.enumeratorType switch + { + Type.IList => ++this.listIndex < this.iList!.Count, + Type.LinkedList => this.concreteLinkedListEnumerator.MoveNext(), + Type.ImmutableHashSet => this.concreteImmutableHashSetEnumerator.MoveNext(), + Type.ImmutableList => this.concreteImmutableListEnumerator.MoveNext(), + Type.HashSet => this.concreteHashSetEnumerator.MoveNext(), + Type.Fallback => this.fallbackEnumerator!.MoveNext(), + _ => false, + }; + + /// + /// Disposes the underlying enumerator. + /// + public void Dispose() + { + switch (this.enumeratorType) + { + case Type.LinkedList: + this.concreteLinkedListEnumerator.Dispose(); + break; + + case Type.ImmutableHashSet: + this.concreteImmutableHashSetEnumerator.Dispose(); + break; + + case Type.ImmutableList: + this.concreteImmutableListEnumerator.Dispose(); + break; + + case Type.HashSet: + this.concreteHashSetEnumerator.Dispose(); + break; + + case Type.Fallback: + this.fallbackEnumerator!.Dispose(); + break; + } + } + } + } + } +}