From e8240b9e76b105e99f576fac61288e561a518986 Mon Sep 17 00:00:00 2001 From: Rainer Sigwald Date: Thu, 14 May 2020 16:49:11 -0500 Subject: [PATCH 1/2] Move ItemspecContainsASingleItemReference to LazyItemOperation I want to use this in another subclass. --- .../LazyItemEvaluator.LazyItemOperation.cs | 21 +++++++++++++++++++ .../LazyItemEvaluator.UpdateOperation.cs | 21 ------------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Build/Evaluation/LazyItemEvaluator.LazyItemOperation.cs b/src/Build/Evaluation/LazyItemEvaluator.LazyItemOperation.cs index 45866736036..21bdd300107 100644 --- a/src/Build/Evaluation/LazyItemEvaluator.LazyItemOperation.cs +++ b/src/Build/Evaluation/LazyItemEvaluator.LazyItemOperation.cs @@ -323,6 +323,27 @@ protected bool NeedToExpandMetadataForEachItem(ImmutableList itemSpec, string referencedItemType) + { + if (itemSpec.Fragments.Count != 1) + { + return false; + } + + var itemExpressionFragment = itemSpec.Fragments[0] as ItemSpec.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.UpdateOperation.cs b/src/Build/Evaluation/LazyItemEvaluator.UpdateOperation.cs index 24b3d9a375b..025f247cfec 100644 --- a/src/Build/Evaluation/LazyItemEvaluator.UpdateOperation.cs +++ b/src/Build/Evaluation/LazyItemEvaluator.UpdateOperation.cs @@ -134,27 +134,6 @@ private static bool ItemSpecContainsItemReferences(ItemSpec itemSpec) { return itemSpec.Fragments.Any(f => f is ItemSpec.ItemExpressionFragment); } - - private static bool ItemspecContainsASingleItemReference(ItemSpec itemSpec, string referencedItemType) - { - if (itemSpec.Fragments.Count != 1) - { - return false; - } - - var itemExpressionFragment = itemSpec.Fragments[0] as ItemSpec.ItemExpressionFragment; - if (itemExpressionFragment == null) - { - return false; - } - - if (!itemExpressionFragment.Capture.ItemType.Equals(referencedItemType, StringComparison.OrdinalIgnoreCase)) - { - return false; - } - - return true; - } } } } From 811b64821ea0430c20b82dc5f32ad3f6753c4d15 Mon Sep 17 00:00:00 2001 From: Rainer Sigwald Date: Thu, 14 May 2020 16:56:55 -0500 Subject: [PATCH 2/2] Make Remove-all-items O(1) in lazy evaluation In the special case where a remove operation removes all items like ```xml ``` We currently have poor behavior because we match ("everything matches") and then remove items one at a time. Instead, we can just empty out the list. Part of #2314. --- .../LazyItemEvaluator.RemoveOperation.cs | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs b/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs index fdd5b3e312d..fe2dcf43a85 100644 --- a/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs +++ b/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs @@ -23,9 +23,14 @@ public RemoveOperation(RemoveOperationBuilder builder, LazyItemEvaluator) 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 SelectItems(ImmutableList.Builder listBuilder, ImmutableHashSet globsToIgnore) + /// + /// Apply the Remove operation. + /// + /// + /// This operation is mostly implemented in terms of the default . + /// This override exists to apply the removing-everything short-circuit. + /// + protected override void ApplyImpl(ImmutableList.Builder listBuilder, ImmutableHashSet globsToIgnore) { var matchOnMetadataValid = !_matchOnMetadata.IsEmpty && _itemSpec.Fragments.Count == 1 && _itemSpec.Fragments.First() is ItemSpec.ItemExpressionFragment; @@ -34,6 +39,20 @@ protected override ImmutableList SelectItems(ImmutableList.Builder new BuildEventFileInfo(string.Empty), "OM_MatchOnMetadataIsRestrictedToOnlyOneReferencedItem"); + if (_matchOnMetadata.IsEmpty && ItemspecContainsASingleItemReference(_itemSpec, _itemElement.ItemType)) + { + // Perf optimization: If the Remove operation references itself (e.g. ) + // 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 SelectItems(ImmutableList.Builder listBuilder, ImmutableHashSet globsToIgnore) + { var items = ImmutableHashSet.CreateBuilder(); foreach (ItemData item in listBuilder) {