Make lazy evaluation even faster

[cdmihai]

There are a couple of optimizations that we can still do on the lazy item evaluator:

• cache GetAllItems results: https://github.com/Microsoft/msbuild/blob/master/src/XMakeBuildEngine/Evaluation/LazyItemEvaluator.cs#L149
• Remove and Update can shortcuit matching items if their itemspec refers to itself (e.g. <Compile Remove="@(Compile)"> or <Compile Update="@(Compile)">)

[dsplaisted]

Continuing the discussion from here: #891 (comment)

Hmm, even then, the cache would be per LazyItemList instance. For example, the lists representing [include] and [include;update] would have 2 different GetItems caches because they are 2 objects. So another item pointing to the first list would not get affected by subsequent operations, since those would be in different lazylist objects.

Using your sample, there are three LazyItemLists. We can call the first one i2_1, and it is produced by this:

<i2 Include='a;b;c'>
    <m1>m1_contents</m1>
    <m2>m2_contents</m2>
</i2>

Then we have i, which simply copies the contents of i2_1:

<i Include='@(i2)'/>

Then we have i2_2, which modifies the metadata for the items coming from i2_1:

<i2 Update='a;b;c'>
    <m1>m1_updated</m1>
    <m2>m2_updated</m2>
</i2>

Your comment seems to refer to the fact that i and i2_2 are separate objects with separate caches. That is true, but they both refer to i2_1, which is a single instance. So when i is evaluated, it will call GetItems for i2_1, which is stored in it's _previous field. This will create the items for i2_1 with the correct metadata, and if we implement caching then it will cache that list before returning it for i to use.

Then when i2_2 is evaluated, it will have a reference to the same instance of LazyItemList which represents i2_1. This time it will return the cached value, which will be the same item instances that were used by i. Then the evaluation of i2_2 will update the metadata on those item instances, which will be the same ones that are in the i item list, meaning that it will change the metadata of the unrelated item list.

Hopefully this helps make it more clear. A possible solution is to clone all the items before modifying their metadata.

[rainersigwald]

@cdmihai You did the second half of this for Update in #2326, right?

[cdmihai]

Closing as caching was done in #1192 and #2314 is tracking the Update / Remove optimizations