Skip to content

Commit

Permalink
Make Remove-all-items O(1) in lazy evaluation
Browse files Browse the repository at this point in the history
In the special case where a remove operation removes all items like

```xml
<Compile Remove="@(Compile)" />
```

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.
  • Loading branch information
rainersigwald committed May 14, 2020
1 parent e8240b9 commit 4459ab7
Showing 1 changed file with 22 additions and 3 deletions.
25 changes: 22 additions & 3 deletions src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.RemoveAll((_) => true);
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)
{
Expand Down

0 comments on commit 4459ab7

Please sign in to comment.