Optimize Match-on-metadata

src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs

@@ -249,7 +249,18 @@ private void ExecuteRemove(ProjectItemGroupTaskItemInstance child, ItemBucket bu
             }
             else
             {
-                itemsToRemove = FindItemsUsingMatchOnMetadata(group, child, bucket, matchOnMetadata, matchingOptions);
+                ImmutableList<string> metadataList = matchOnMetadata.ToImmutableList();
+                ItemSpec<ProjectPropertyInstance, ProjectItemInstance> itemSpec = new(child.Remove, bucket.Expander, child.RemoveLocation, Project.Directory, true);
+                ProjectFileErrorUtilities.VerifyThrowInvalidProjectFile(
+                    itemSpec.Fragments.Count == 1
+                    && itemSpec.Fragments.First() is ItemSpec<ProjectPropertyInstance, ProjectItemInstance>.ItemExpressionFragment
+                    && matchOnMetadata.Count == 1,
+                    new BuildEventFileInfo(string.Empty),
+                    "OM_MatchOnMetadataIsRestrictedToOnlyOneReferencedItem",
+                    child.RemoveLocation,
+                    child.Remove);
+                MetadataSet<ProjectPropertyInstance, ProjectItemInstance> metadataSet = new(matchingOptions, metadataList, itemSpec);
+                itemsToRemove = group.Where(item => metadataSet.Contains(metadataList.Select(m => item.GetMetadataValue(m)))).ToList();
             }
 
             if (itemsToRemove != null)
@@ -268,29 +279,6 @@ private void ExecuteRemove(ProjectItemGroupTaskItemInstance child, ItemBucket bu
             }
         }
 
-        private List<ProjectItemInstance> FindItemsUsingMatchOnMetadata(
-            ICollection<ProjectItemInstance> items,
-            ProjectItemGroupTaskItemInstance child,
-            ItemBucket bucket,
-            HashSet<string> matchOnMetadata,
-            MatchOnMetadataOptions options)
-        {
-            ErrorUtilities.VerifyThrowArgumentNull(matchOnMetadata, nameof(matchOnMetadata));
-
-            var itemSpec = new ItemSpec<ProjectPropertyInstance, ProjectItemInstance>(child.Remove, bucket.Expander, child.RemoveLocation, Project.Directory, true);
-
-            ProjectFileErrorUtilities.VerifyThrowInvalidProjectFile(
-                itemSpec.Fragments.Count == 1
-                && itemSpec.Fragments.First() is ItemSpec<ProjectPropertyInstance, ProjectItemInstance>.ItemExpressionFragment
-                && matchOnMetadata.Count == 1,
-                new BuildEventFileInfo(string.Empty),
-                "OM_MatchOnMetadataIsRestrictedToOnlyOneReferencedItem",
-                child.RemoveLocation,
-                child.Remove);
-
-            return items.Where(item => itemSpec.MatchesItemOnMetadata(item, matchOnMetadata, options)).ToList();
-        }
-
         /// <summary>
         /// Modifies items in the world - specifically, changes their metadata. Changes to items that are part of the project manifest are backed up, so 
         /// they can be reverted when the project is reset after the end of the build.

src/Build/Evaluation/ItemSpec.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 using Microsoft.Build.Globbing;
 using Microsoft.Build.Internal;
@@ -85,24 +86,6 @@ public override bool IsMatch(string itemToMatch)
                 return ReferencedItems.Any(v => v.ItemAsValueFragment.IsMatch(itemToMatch));
             }
 
-            public override bool IsMatchOnMetadata(IItem item, IEnumerable<string> metadata, MatchOnMetadataOptions options)
-            {
-                return ReferencedItems.Any(referencedItem =>
-                        metadata.All(m => !item.GetMetadataValue(m).Equals(string.Empty) && MetadataComparer(options, item.GetMetadataValue(m), referencedItem.Item.GetMetadataValue(m))));
-            }
-
-            private bool MetadataComparer(MatchOnMetadataOptions options, string itemMetadata, string referencedItemMetadata)
-            {
-                if (options.Equals(MatchOnMetadataOptions.PathLike))
-                {
-                    return FileUtilities.ComparePathsNoThrow(itemMetadata, referencedItemMetadata, ProjectDirectory);
-                }
-                else 
-                {
-                    return String.Equals(itemMetadata, referencedItemMetadata, options.Equals(MatchOnMetadataOptions.CaseInsensitive) ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal);
-                }
-            }
-
             public override IMSBuildGlob ToMSBuildGlob()
             {
                 return MsBuildGlob;
@@ -310,26 +293,6 @@ public bool MatchesItem(I item)
             return false;
         }
 
-        /// <summary>
-        ///     Return true if any of the given <paramref name="metadata" /> matches the metadata on <paramref name="item" />
-        /// </summary>
-        /// <param name="item">The item to attempt to find a match for based on matching metadata</param>
-        /// <param name="metadata">Names of metadata to look for matches for</param>
-        /// <param name="options">metadata option matching</param>
-        /// <returns></returns>
-        public bool MatchesItemOnMetadata(IItem item, IEnumerable<string> metadata, MatchOnMetadataOptions options)
-        {
-            foreach (var fragment in Fragments)
-            {
-                if (fragment.IsMatchOnMetadata(item, metadata, options))
-                {
-                    return true;
-                }
-            }
-
-            return false;
-        }
-
         /// <summary>
         ///     Return the fragments that match against the given <paramref name="itemToMatch" />
         /// </summary>
@@ -456,14 +419,6 @@ public virtual bool IsMatch(string itemToMatch)
             return FileMatcher.IsMatch(itemToMatch);
         }
 
-        /// <summary>
-        /// Returns true if <paramref name="itemToMatch" /> matches any ReferencedItems based on <paramref name="metadata" /> and <paramref name="options" />.
-        /// </summary>
-        public virtual bool IsMatchOnMetadata(IItem itemToMatch, IEnumerable<string> metadata, MatchOnMetadataOptions options)
-        {
-            return false;
-        }
-
         public virtual IMSBuildGlob ToMSBuildGlob()
         {
             return MsBuildGlob;
@@ -504,4 +459,78 @@ public GlobFragment(string textFragment, string projectDirectory)
             && TextFragment[2] == '*'
             && FileUtilities.IsAnySlash(TextFragment[3]);
     }
+
+    internal class MetadataSet<P, I> where P : class, IProperty where I : class, IItem, IMetadataTable
+    {
+        private Dictionary<string, MetadataSet<P, I>> children;
+        Func<string, string> normalize;
+
+        internal MetadataSet(MatchOnMetadataOptions options, ImmutableList<string> metadata, ItemSpec<P, I> itemSpec)
+        {
+            StringComparer comparer = options == MatchOnMetadataOptions.CaseSensitive ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase;
+            children = new Dictionary<string, MetadataSet<P, I>>(comparer);
+            normalize = options == MatchOnMetadataOptions.PathLike ? p => FileUtilities.NormalizePathForComparisonNoThrow(p, Environment.CurrentDirectory) : p => p;
+            foreach (ItemSpec<P, I>.ItemExpressionFragment frag in itemSpec.Fragments)
+            {
+                foreach (ItemSpec<P, I>.ReferencedItem referencedItem in frag.ReferencedItems)
+                {
+                    this.Add(metadata.Select(m => referencedItem.Item.GetMetadataValue(m)), comparer);
+                }
+            }
+        }
+
+        private MetadataSet(StringComparer comparer)
+        {
+            children = new Dictionary<string, MetadataSet<P, I>>(comparer);
+        }
+
+        // Relies on IEnumerable returning the metadata in a reasonable order. Reasonable?
+        private void Add(IEnumerable<string> metadata, StringComparer comparer)
+        {
+            MetadataSet<P, I> current = this;
+            foreach (string m in metadata)
+            {
+                string normalizedString = normalize(m);
+                if (current.children.TryGetValue(normalizedString, out MetadataSet<P, I> child))
+                {
+                    current = child;
+                }
+                else
+                {
+                    current.children.Add(normalizedString, new MetadataSet<P, I>(comparer));
+                    current = current.children[normalizedString];
+                }
+            }
+        }
+
+        internal bool Contains(IEnumerable<string> metadata)
+        {
+            bool nonEmptyFound = false;
+            MetadataSet<P, I> curr = this;
+            foreach (string m in metadata)
+            {
+                if (!String.IsNullOrEmpty(m))
+                {
+                    nonEmptyFound = true;
+                }
+                if (!curr.children.TryGetValue(normalize(m), out curr))
+                {
+                    return false;
+                }
+            }
+            return nonEmptyFound;
+        }
+    }
+
+    public enum MatchOnMetadataOptions
+    {
+        CaseSensitive,
+        CaseInsensitive,
+        PathLike
+    }
+
+    public static class MatchOnMetadataConstants
+    {
+        public const MatchOnMetadataOptions MatchOnMetadataOptionsDefaultValue = MatchOnMetadataOptions.CaseSensitive;
+    }
 }

src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs

@@ -14,6 +14,7 @@ class RemoveOperation : LazyItemOperation
         {
             readonly ImmutableList<string> _matchOnMetadata;
             readonly MatchOnMetadataOptions _matchOnMetadataOptions;
+            private MetadataSet<P, I> metadataSet;
 
             public RemoveOperation(RemoveOperationBuilder builder, LazyItemEvaluator<P, I, M, D> lazyEvaluator)
                 : base(builder, lazyEvaluator)
@@ -55,13 +56,23 @@ protected override ImmutableList<I> SelectItems(ImmutableList<ItemData>.Builder
                 var items = ImmutableHashSet.CreateBuilder<I>();
                 foreach (ItemData item in listBuilder)
                 {
-                    if (_matchOnMetadata.IsEmpty ? _itemSpec.MatchesItem(item.Item) : _itemSpec.MatchesItemOnMetadata(item.Item, _matchOnMetadata, _matchOnMetadataOptions))
+                    if (_matchOnMetadata.IsEmpty ? _itemSpec.MatchesItem(item.Item) : MatchesItemOnMetadata(item.Item))
                         items.Add(item.Item);
                 }
 
                 return items.ToImmutableList();
             }
 
+            private bool MatchesItemOnMetadata(I item)
+            {
+                if (metadataSet == null)
+                {
+                    metadataSet = new MetadataSet<P, I>(_matchOnMetadataOptions, _matchOnMetadata, _itemSpec);
+                }
+
+                return metadataSet.Contains(_matchOnMetadata.Select(m => item.GetMetadataValue(m)));
+            }
+
             protected override void SaveItems(ImmutableList<I> items, ImmutableList<ItemData>.Builder listBuilder)
             {
                 if (!_conditionResult)
@@ -100,15 +111,4 @@ public RemoveOperationBuilder(ProjectItemElement itemElement, bool conditionResu
             }
         }
     }
-
-    public enum MatchOnMetadataOptions
-    {
-        CaseSensitive,
-        CaseInsensitive,
-        PathLike
-    }
-
-    public static class MatchOnMetadataConstants {
-        public const MatchOnMetadataOptions MatchOnMetadataOptionsDefaultValue = MatchOnMetadataOptions.CaseSensitive;
-    }
 }