Optimize TaskItem cloning between Engine and Tasks

[ccastanedaucf]

Context

Currently the engine and task implementations of ITaskItem will always do a full rebuild of their backing ImmutableDictionary<string, T>. This is due to having different backing types (string vs ProjectMetadataInstance), plus CopyOnWriteDictionary<T> type living under Shared, preventing type guards from working even if it did expose the bakcing dictionary.

This affects both Tasks copying metadata within a task, as well as the engine whenever any task completes execution, as Utilities.TaskItem must always be converted back into ProjectItemInstance.TaskItem. See GatherTaskItemOutputs in a profiler to see how expensive this is, with most of the impact coming from RAR.

Changes Made

This PR is largely two parts:

• Moving types around and modifying the backing collections which use ImmutableDictionary to allow reference equality checks.
• Giving TaskITem implementations the ability to clone from other implementations when given a CopyOnWriteDictionary instance.

Also adds a simple optimization in RAR to cache any ProjectItemInstance.TaskItem instances which came from a source file into temporary Utilities.TaskItem, avoiding the expensive enumeration. These are reused across references within each RAR invocation, and the shared ImmutableDictionary types allow us to do this for free. (Could probably split that off into its own PR to wait till this is merged, but it's a couple lines and net negative perf without this so...)

Testing

Example of spending 1718ms in GatherTaskItemOutputs alone:

374 ms with this change. Note that the TaskItem copying doesn't even show up in the profile here since it's just a no-op clone, so it's effectively free and the rest is unrelated overhead.

Need to test on more repos, and other hotspots are hard to track since these get passed around everywhere - but there's a pretty clear difference just running ad-hoc builds back-to-back:

This PR:

     2371 ms  MSBuild                                  1053 calls
     8232 ms  ResolveAssemblyReference                 142 calls
...
Time Elapsed 00:00:38.69

Mainline:

     2537 ms  MSBuild                                  1053 calls
     9117 ms  ResolveAssemblyReference                 142 calls
...
Time Elapsed 00:00:40.73

Notes

Pretty confident on the overall design, but still need to UT and see if there are places consolidate code since there's a fair amount of type casting and duplication in the TaskItem CopyTo() / ImportMetadata() methods that I think can be cleaned up still. May be able split this into A. Moves / modyfing the types + B. Optimized cloning

[ccastanedaucf]

The failing tests all fail when I checked out upstream, so not sure what's going on there. Otherwise I've had great numbers building on other repos.

I just tested a few runs on Orchard Core and it's around 37% cut from RAR alone, plus seems like every task is consistently faster.

Multi-proc:
...\artifacts\bin\bootstrap\core\dotnet.exe build -tl:off -restore:false /clp:PerformanceSummary /m

Before:

      891 ms  Hash                                     1064 calls
      974 ms  Microsoft.Build.Tasks.Git.GetUntrackedFiles 420 calls
     1116 ms  AssignProjectConfiguration               436 calls
     1243 ms  ResolvePackageAssets                     436 calls
     1414 ms  ResolveTargetingPackAssets               436 calls
     3039 ms  Copy                                     7037 calls
     5140 ms  ResolvePackageFileConflicts              436 calls
    19162 ms  ResolveAssemblyReference                 436 calls
    132508 ms  CallTarget                               872 calls
    3940843 ms  MSBuild                                  2388 calls

Time Elapsed 00:00:26.55

After:

      696 ms  Hash                                     1064 calls
      776 ms  ResolvePackageAssets                     436 calls
      814 ms  Microsoft.Build.Tasks.Git.GetUntrackedFiles 420 calls
      869 ms  ProcessFrameworkReferences               436 calls
      890 ms  AssignProjectConfiguration               436 calls
     2636 ms  Copy                                     7037 calls
     4720 ms  ResolvePackageFileConflicts              436 calls
    12221 ms  ResolveAssemblyReference                 436 calls
    82668 ms  CallTarget                               872 calls
    4317227 ms  MSBuild                                  2388 calls

Time Elapsed 00:00:24.46

Single proc (still important for quickbuild):
...\artifacts\bin\bootstrap\core\dotnet.exe build -tl:off -restore:false /clp:PerformanceSummary /m:1

Before

      385 ms  Hash                                     1064 calls
      457 ms  ProcessFrameworkReferences               436 calls
      530 ms  AssignProjectConfiguration               436 calls
      531 ms  Microsoft.Build.Tasks.Git.GetUntrackedFiles 420 calls
      770 ms  ResolvePackageAssets                     436 calls
      781 ms  ResolveTargetingPackAssets               436 calls
     1165 ms  CallTarget                               872 calls
     2122 ms  Copy                                     7037 calls
     2993 ms  ResolvePackageFileConflicts              436 calls
    10429 ms  ResolveAssemblyReference                 436 calls
    321574 ms  MSBuild                                  2388 calls

Time Elapsed 00:01:12.34

After

      362 ms  Hash                                     1064 calls
      405 ms  ProcessFrameworkReferences               436 calls
      436 ms  AssignProjectConfiguration               436 calls
      504 ms  Microsoft.Build.Tasks.Git.GetUntrackedFiles 420 calls
      651 ms  ResolvePackageAssets                     436 calls
      751 ms  WriteLinesToFile                         1492 calls
     1016 ms  CallTarget                               872 calls
     1887 ms  Copy                                     7037 calls
     2874 ms  ResolvePackageFileConflicts              436 calls
     6522 ms  ResolveAssemblyReference                 436 calls
    286337 ms  MSBuild                                  2388 calls

Time Elapsed 00:01:04.97

[ccastanedaucf]

kk going to leave this drafted for reference and get a parent item up tomorrow. I have detailed perf analysis that all looks great, but a couple parts look riskier than others so I do want to split it up so it's an easy revert in the worst case. I think those risky bits are:

• _directMetadata -> _itemDefinitions ordering requirements. This was the trickiest part to get right since there are 3 different "views" you could have on the metadata in ProjectItemInstance.TaskItem depending on which methods you access. Aka enumerating metadata may shadow item definitions that would appear if you called GetMetadataValueEscaped(). And then depending on which type of item you import and which direction, you either shadow the current item definitions or offset the list. I would expect the enumerated collection to match what is returned by GetMetadataValueEscaped(), but any change to this is breaking behavior. Fun.
• AppDomain stuff. Tbh not super familiar with how important that path is for perf and CopyOnWriteDictionary<string> is serializable, but I imagine that kills no-op cloning so it would probably just be better to pass an array or the likes in those cases. I haven't tested that at all though.

Allocation wise looks much improved as well, despite creating a new ProjectMetadataInstance on every CopyOnWritePropertyDictionary lookup. Getting a diff from 17,910 MB -> 15,943 MB on a clean run of Orchard Core (-11%).