Merge pull request #5350 from rainersigwald/optimized-remove-all

dotnet · May 15, 2020 · 31a1a34 · 31a1a34
2 parents fe0b082 + 811b648
commit 31a1a34
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 24 deletions.
diff --git a/src/Build/Evaluation/LazyItemEvaluator.LazyItemOperation.cs b/src/Build/Evaluation/LazyItemEvaluator.LazyItemOperation.cs
@@ -323,6 +323,27 @@ protected bool NeedToExpandMetadataForEachItem(ImmutableList<ProjectMetadataElem
 
                 return needToExpandMetadataForEachItem;
             }
+
+            protected static bool ItemspecContainsASingleItemReference(ItemSpec<P, I> itemSpec, string referencedItemType)
+            {
+                if (itemSpec.Fragments.Count != 1)
+                {
+                    return false;
+                }
+
+                var itemExpressionFragment = itemSpec.Fragments[0] as ItemSpec<P, I>.ItemExpressionFragment;
+                if (itemExpressionFragment == null)
+                {
+                    return false;
+                }
+
+                if (!itemExpressionFragment.Capture.ItemType.Equals(referencedItemType, StringComparison.OrdinalIgnoreCase))
+                {
+                    return false;
+                }
+
+                return true;
+            }
         }
     }
 }
diff --git a/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs b/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs
@@ -23,9 +23,14 @@ public RemoveOperation(RemoveOperationBuilder builder, LazyItemEvaluator<P, I, M
                 _matchOnMetadataOptions = builder.MatchOnMetadataOptions;
             }
 
-            // todo port the self referencing matching optimization (e.g. <I Remove="@(I)">) from Update to Remove as well. Ideally make one mechanism for both. https://github.com/Microsoft/msbuild/issues/2314
-            // todo Perf: do not match against the globs: https://github.com/Microsoft/msbuild/issues/2329
-            protected override ImmutableList<I> SelectItems(ImmutableList<ItemData>.Builder listBuilder, ImmutableHashSet<string> globsToIgnore)
+            /// <summary>
+            /// Apply the Remove operation.
+            /// </summary>
+            /// <remarks>
+            /// This operation is mostly implemented in terms of the default <see cref="LazyItemOperation.ApplyImpl(ImmutableList{ItemData}.Builder, ImmutableHashSet{string})"/>.
+            /// This override exists to apply the removing-everything short-circuit.
+            /// </remarks>
+            protected override void ApplyImpl(ImmutableList<ItemData>.Builder listBuilder, ImmutableHashSet<string> globsToIgnore)
             {
                 var matchOnMetadataValid = !_matchOnMetadata.IsEmpty && _itemSpec.Fragments.Count == 1
                     && _itemSpec.Fragments.First() is ItemSpec<ProjectProperty, ProjectItem>.ItemExpressionFragment;
@@ -34,6 +39,20 @@ protected override ImmutableList<I> SelectItems(ImmutableList<ItemData>.Builder
                     new BuildEventFileInfo(string.Empty),
                     "OM_MatchOnMetadataIsRestrictedToOnlyOneReferencedItem");
 
+                if (_matchOnMetadata.IsEmpty && ItemspecContainsASingleItemReference(_itemSpec, _itemElement.ItemType))
+                {
+                    // Perf optimization: If the Remove operation references itself (e.g. <I Remove="@(I)"/>)
+                    // then all items are removed and matching is not necessary
+                    listBuilder.Clear();
+                    return;
+                }
+
+                base.ApplyImpl(listBuilder, globsToIgnore);
+            }
+
+            // todo Perf: do not match against the globs: https://github.com/Microsoft/msbuild/issues/2329
+            protected override ImmutableList<I> SelectItems(ImmutableList<ItemData>.Builder listBuilder, ImmutableHashSet<string> globsToIgnore)
+            {
                 var items = ImmutableHashSet.CreateBuilder<I>();
                 foreach (ItemData item in listBuilder)
                 {

diff --git a/src/Build/Evaluation/LazyItemEvaluator.UpdateOperation.cs b/src/Build/Evaluation/LazyItemEvaluator.UpdateOperation.cs
@@ -134,27 +134,6 @@ private static bool ItemSpecContainsItemReferences(ItemSpec<P, I> itemSpec)
             {
                 return itemSpec.Fragments.Any(f => f is ItemSpec<P,I>.ItemExpressionFragment);
             }
-
-            private static bool ItemspecContainsASingleItemReference(ItemSpec<P, I> itemSpec, string referencedItemType)
-            {
-                if (itemSpec.Fragments.Count != 1)
-                {
-                    return false;
-                }
-
-                var itemExpressionFragment = itemSpec.Fragments[0] as ItemSpec<P,I>.ItemExpressionFragment;
-                if (itemExpressionFragment == null)
-                {
-                    return false;
-                }
-
-                if (!itemExpressionFragment.Capture.ItemType.Equals(referencedItemType, StringComparison.OrdinalIgnoreCase))
-                {
-                    return false;
-                }
-
-                return true;
-            }
         }
     }
 }