Skip to content

Commit

Permalink
Improve performance of projects with large numbers of consecutive ite…
Browse files Browse the repository at this point in the history 
…m updates without wildcards Fixes #5776 (#5853)

This PR changed the process of applying consecutive UpdateOperations on distinct fragments without wildcards.

Previously, each one was applied individually, so if there were a large number of items and a large number of updates, this would take time proportional to the product of the two. Now, those update operations are batched together in a dictionary such that we only have to make one pass to apply all the update operations at once. In other words, time would then be proportional to the number of items but not the number of updates.

Note that this is very specific to update operations in sequence without wildcards or other characters indicating that we might need to expand them. Interleaving RemoveOperations, for instance, would effectively run the older, unoptimized code. This is because the UpdateOperations are added to the batch in sequence, but if an operation that cannot be added to the batch is next, it pauses to evaluate all the operations in the batch. This ensures we respect ordering of operations.

Updating the same fragment twice isn't permitted in a single batch because then we would have to keep track of whether the second update would overwrite the first or not, and if so, apply them in the correct order.

Time to add/delete a class decreased to 1-3 seconds, thus fixing #5776
  • Loading branch information
@Forgind
Forgind committed Jan 9, 2021
1 parent 0ec390a commit 74eb87c
Show file tree
Hide file tree
Showing 8 changed files with 265 additions and 43 deletions.
105 changes: 105 additions & 0 deletions src/Build.OM.UnitTests/Definition/ProjectItem_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2604,6 +2604,92 @@ public void LastUpdateWins()
ObjectModelHelpers.AssertItemHasMetadata(expectedUpdate, items[0]);
}

[Theory]
[InlineData("abc", "def", "abc")]
[InlineData("abc", "de*", "abc")]
[InlineData("a*c", "def", "abc")]
[InlineData("abc", "def", "*bc")]
[InlineData("abc", "d*f", "*bc")]
[InlineData("*c", "d*f", "*bc")]
[InlineData("a*", "d*", "abc")]
public void UpdatesProceedInOrder(string first, string second, string third)
{
string contents = $@"
<i Include='abc'>
<m1>m1_contents</m1>
</i>
<j Include='def'>
<m1>m1_contents</m1>
</j>
<i Update='{first}'>
<m1>first</m1>
</i>
<j Update='{second}'>
<m1>second</m1>
</j>
<i Update='{third}'>
<m1>third</m1>
</i>
";
IList<ProjectItem> items = ObjectModelHelpers.GetItemsFromFragment(contents, allItems: true);
Dictionary<string, string> expectedUpdatei = new Dictionary<string, string>
{
{"m1", "third" }
};
Dictionary<string, string> expectedUpdatej = new Dictionary<string, string>
{
{"m1", "second" }
};

ObjectModelHelpers.AssertItemHasMetadata(expectedUpdatei, items[0]);
ObjectModelHelpers.AssertItemHasMetadata(expectedUpdatej, items[1]);
}

[Fact]
public void UpdatingIndividualItemsProceedsInOrder()
{
string contents = @"
<i Include='a;b;c'>
<m1>m1_contents</m1>
</i>
<i Update='a'>
<m1>second</m1>
</i>
<i Update='b'>
<m1>third</m1>
</i>
<i Update='c'>
<m1>fourth</m1>
</i>
<afterFirst Include='@(i)' />
<i Update='*'>
<m1>sixth</m1>
</i>
<afterSecond Include='@(i)' />
<i Update='b'>
<m1>seventh</m1>
</i>
<afterThird Include='@(i)' />
<i Update='c'>
<m1>eighth</m1>
</i>
<afterFourth Include='@(i)' />
";
IList<ProjectItem> items = ObjectModelHelpers.GetItemsFromFragment(contents, allItems: true);
ObjectModelHelpers.AssertItemHasMetadata("m1", "second", items[3]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "third", items[4]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "fourth", items[5]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "sixth", items[6]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "sixth", items[7]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "sixth", items[8]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "sixth", items[9]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "seventh", items[10]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "sixth", items[11]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "sixth", items[12]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "seventh", items[13]);
ObjectModelHelpers.AssertItemHasMetadata("m1", "eighth", items[14]);
}

[Fact]
public void UpdateWithNoMetadataShouldNotAffectItems()
{
Expand Down Expand Up @@ -2850,6 +2936,25 @@ public void UpdateFromReferencedItemShouldBeCaseInsensitive()
ObjectModelHelpers.AssertItemHasMetadata(expectedMetadataA, items[1]);
}

[Fact]
public void UpdateMetadataWithoutItemReferenceShouldBeCaseInsensitive()
{
string content = @"
<to Include='a' />
<to Update='A' m='m1_contents' />";

IList<ProjectItem> items = ObjectModelHelpers.GetItemsFromFragment(content, true);

var expectedMetadataA = new Dictionary<string, string>
{
{"m", "m1_contents"},
};

items[0].ItemType.ShouldBe("to");
ObjectModelHelpers.AssertItemHasMetadata(expectedMetadataA, items[0]);
}

[Fact]
public void UndeclaredQualifiedMetadataReferencesInUpdateShouldResolveToEmptyStrings()
{
Expand Down
2 changes: 1 addition & 1 deletion src/Build/Evaluation/ItemSpec.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ internal abstract class ItemSpecFragment
/// <summary>
/// Path of the project the itemspec is coming from
/// </summary>
protected string ProjectDirectory { get; }
internal string ProjectDirectory { get; }

// not a Lazy to reduce memory
private ref FileSpecMatcherTester FileMatcher
Expand Down
3 changes: 2 additions & 1 deletion src/Build/Evaluation/LazyItemEvaluator.LazyItemOperation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ private abstract class LazyItemOperation : IItemOperation
// This is used only when evaluating an expression, which instantiates
// the items and then removes them
protected readonly IItemFactory<I, I> _itemFactory;

internal ItemSpec<P, I> Spec => _itemSpec;

protected LazyItemOperation(OperationBuilder builder, LazyItemEvaluator<P, I, M, D> lazyEvaluator)
{
_itemElement = builder.ItemElement;
Expand Down
95 changes: 65 additions & 30 deletions src/Build/Evaluation/LazyItemEvaluator.UpdateOperation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ internal partial class LazyItemEvaluator<P, I, M, D>
class UpdateOperation : LazyItemOperation
{
private readonly ImmutableList<ProjectMetadataElement> _metadata;
private ImmutableList<ItemBatchingContext>.Builder _itemsToUpdate = null;
private ItemSpecMatchesItem _matchItemSpec = null;
private bool? _needToExpandMetadataForEachItem = null;

public UpdateOperation(OperationBuilderWithMetadata builder, LazyItemEvaluator<P, I, M, D> lazyEvaluator)
: base(builder, lazyEvaluator)
Expand Down Expand Up @@ -43,23 +46,77 @@ protected override void ApplyImpl(ImmutableList<ItemData>.Builder listBuilder, I
return;
}

ItemSpecMatchesItem matchItemspec;
bool? needToExpandMetadataForEachItem = null;
SetMatchItemSpec();
_itemsToUpdate ??= ImmutableList.CreateBuilder<ItemBatchingContext>();
_itemsToUpdate.Clear();

for (int i = 0; i < listBuilder.Count; i++)
{
var itemData = listBuilder[i];

var matchResult = _matchItemSpec(_itemSpec, itemData.Item);

if (matchResult.IsMatch)
{
listBuilder[i] = UpdateItem(listBuilder[i], matchResult.CapturedItemsFromReferencedItemTypes);
}
}

DecorateItemsWithMetadata(_itemsToUpdate.ToImmutableList(), _metadata, _needToExpandMetadataForEachItem);
}

/// <summary>
/// Apply the Update operation to the item if it matches.
/// </summary>
/// <param name="item">The item to check for a match.</param>
/// <returns>The updated item.</returns>
internal ItemData UpdateItem(ItemData item)
{
if (_conditionResult)
{
SetMatchItemSpec();
_itemsToUpdate ??= ImmutableList.CreateBuilder<ItemBatchingContext>();
_itemsToUpdate.Clear();
MatchResult matchResult = _matchItemSpec(_itemSpec, item.Item);
if (matchResult.IsMatch)
{
ItemData clonedData = UpdateItem(item, matchResult.CapturedItemsFromReferencedItemTypes);
DecorateItemsWithMetadata(_itemsToUpdate.ToImmutableList(), _metadata, _needToExpandMetadataForEachItem);
return clonedData;
}
}
return item;
}

private ItemData UpdateItem(ItemData item, Dictionary<string, I> capturedItemsFromReferencedItemTypes)
{
// items should be deep immutable, so clone and replace items before mutating them
// otherwise, with GetItems caching enabled, the mutations would leak into the cache causing
// future operations to mutate the state of past operations
ItemData clonedData = item.Clone(_itemFactory, _itemElement);
_itemsToUpdate.Add(new ItemBatchingContext(clonedData.Item, capturedItemsFromReferencedItemTypes));
return clonedData;
}

/// <summary>
/// This sets the function used to determine whether an item matches an item spec.
/// </summary>
private void SetMatchItemSpec()
{
if (ItemspecContainsASingleBareItemReference(_itemSpec, _itemElement.ItemType))
{
// Perf optimization: If the Update operation references itself (e.g. <I Update="@(I)"/>)
// then all items are updated and matching is not necessary
matchItemspec = (itemSpec, item) => new MatchResult(true, null);
_matchItemSpec = (itemSpec, item) => new MatchResult(true, null);
}
else if (ItemSpecContainsItemReferences(_itemSpec)
&& QualifiedMetadataReferencesExist(_metadata, out needToExpandMetadataForEachItem)
&& QualifiedMetadataReferencesExist(_metadata, out _needToExpandMetadataForEachItem)
&& !Traits.Instance.EscapeHatches.DoNotExpandQualifiedMetadataInUpdateOperation)
{
var itemReferenceFragments = _itemSpec.Fragments.OfType<ItemSpec<P,I>.ItemExpressionFragment>().ToArray();
var nonItemReferenceFragments = _itemSpec.Fragments.Where(f => !(f is ItemSpec<P,I>.ItemExpressionFragment)).ToArray();
var itemReferenceFragments = _itemSpec.Fragments.OfType<ItemSpec<P, I>.ItemExpressionFragment>().ToArray();
var nonItemReferenceFragments = _itemSpec.Fragments.Where(f => !(f is ItemSpec<P, I>.ItemExpressionFragment)).ToArray();

matchItemspec = (itemSpec, item) =>
_matchItemSpec = (itemSpec, item) =>
{
var isMatch = nonItemReferenceFragments.Any(f => f.IsMatch(item.EvaluatedInclude));
Dictionary<string, I> capturedItemsFromReferencedItemTypes = null;
Expand All @@ -84,30 +141,8 @@ protected override void ApplyImpl(ImmutableList<ItemData>.Builder listBuilder, I
}
else
{
matchItemspec = (itemSpec, item) => new MatchResult(itemSpec.MatchesItem(item), null);
}

var itemsToUpdate = ImmutableList.CreateBuilder<ItemBatchingContext>();

for (int i = 0; i < listBuilder.Count; i++)
{
var itemData = listBuilder[i];

var matchResult = matchItemspec(_itemSpec, itemData.Item);

if (matchResult.IsMatch)
{
// items should be deep immutable, so clone and replace items before mutating them
// otherwise, with GetItems caching enabled, the mutations would leak into the cache causing
// future operations to mutate the state of past operations
var clonedItemData = listBuilder[i].Clone(_itemFactory, _itemElement);
listBuilder[i] = clonedItemData;

itemsToUpdate.Add(new ItemBatchingContext(clonedItemData.Item, matchResult.CapturedItemsFromReferencedItemTypes));
}
_matchItemSpec = (itemSpec, item) => new MatchResult(itemSpec.MatchesItem(item), null);
}

DecorateItemsWithMetadata(itemsToUpdate.ToImmutableList(), _metadata, needToExpandMetadataForEachItem);
}

private bool QualifiedMetadataReferencesExist(ImmutableList<ProjectMetadataElement> metadata, out bool? needToExpandMetadataForEachItem)
Expand Down

0 comments on commit 74eb87c

Please sign in to comment.