Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update and Remove should optimize on identity matches #2314

Open
2 of 3 tasks
cdmihai opened this issue Jul 18, 2017 · 2 comments · Fixed by #5350
Open
2 of 3 tasks

Update and Remove should optimize on identity matches #2314

cdmihai opened this issue Jul 18, 2017 · 2 comments · Fixed by #5350

Comments

@cdmihai
Copy link
Contributor

cdmihai commented Jul 18, 2017

Update item operations provide a way to select items and set metadata on them.

When another item is used as selection (e.g. <Foo Update="@(Bar) Metadata="Value">), MSBuild compares all items from the operation item type (Foo) with all items from the referenced item type (Bar) and updates the intersection. This results in N^2 runtime.

However, when the operation item type is the same as the referenced item type (e.g. <Foo Update="@(Foo) Metadata="Value">) MSBuild could optimize and iterate over the items only once.

Suggested by @dsplaisted who found this is slow when there are many items in a self referencing update: #2238 (comment)

Optimizations:

  • self referencing Updates can short circuit matching altogether
  • extend the optimization in Update to all item operations that use matching (for now only Remove)
  • add a lookup dictionary to ItemExpressionFragment such that operations that need to do matching hit the lookup instead of looping through all the ItemExpressionFragment items
@dsplaisted
Copy link
Member

I was thinking that a more general solution would be to create a dictionary lookup if the ReferencedItem count of an ItemExpressionFragment was high enough. That way you wouldn't get n^2 runtime in situations where you have two different lists with a lot of items, such as <A Update="@(B)" />.

cdmihai added a commit that referenced this issue Jul 20, 2017
* Self referencing Update operations skip matching

Perf optimization: If the Update operation references itself (e.g. <I Update="@(I)"/>) then all items are updated and matching is not necessary.

Implements the simpler, more constrained optimization from #2314
@cdmihai cdmihai changed the title Update should optimize on identity matches Update and Remove should optimize on identity matches Jul 20, 2017
@cdmihai cdmihai mentioned this issue Aug 10, 2017
rainersigwald added a commit to rainersigwald/msbuild that referenced this issue May 12, 2020
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.

Fixes dotnet#2314.
rainersigwald added a commit to rainersigwald/msbuild that referenced this issue May 14, 2020
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 dotnet#2314.
rainersigwald added a commit to rainersigwald/msbuild that referenced this issue May 15, 2020
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 dotnet#2314.
@dsplaisted
Copy link
Member

Re-opening per comment: #5350 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants