Reduce repeated targeting-pack conflict parsing allocations in ResolvePackageFileConflicts
- Dominant language
- C#
- Stars
- 3.2k
- Forks
- 1.3k
- PR merge metrics
- PR metrics pending
Description
🤖
### Describe the bug
Retargeting the OrchardCore Visual Studio performance test asset from .NET 10 to `net11.0` exposed a substantial allocation regression in `msbuild.exe`. The relevant .NET SDK implementation is unchanged on `release/11.0.1xx` and repeatedly parses identical targeting-pack conflict data for each `ResolvePackageFileConflicts` invocation.
In controlled `OrchardCoreNoCache.TestSolutionOpenClose` runs:
- Mean non-devenv allocations increased by approximately 1.62 GB (22.99%).
- A representative ETW comparison attributed approximately 1.64 GB of the increase to MSBuild.
- Both sides used SDK `11.0.100-rc.1.26411.119` and processed the same 176 projects.
- `ResolvePackageFileConflicts` ran 352 times in each trace.
- Sampled positive allocation deltas included approximately 103.6 MB under `PlatformManifestReader` and 64.4 MB under package-override parsing.
`PlatformManifest.txt` and `PackageOverrides.txt` originate primarily from installed targeting packs and are identical across many projects, but the task repeatedly splits, trims, version-parses, and materializes the same data. This issue concerns cumulative allocations and GC pressure; it does not assume the entire allocation delta is retained in the process working set.
Relevant implementation:
- `src/Tasks/Common/ConflictResolution/PlatformManifestReader.cs`
- `src/Tasks/Common/ConflictResolution/PackageOverride.cs`
- `src/Tasks/Common/ConflictResolution/PackageOverrideResolver.cs`
- `src/Tasks/Common/ConflictResolution/ResolvePackageFileConflicts.cs`
- `src/Tasks/Common/ConflictResolution/FrameworkListReader.cs`
- `src/Tasks/Microsoft.NET.Build.Tasks/ResolveTargetingPackAssets.cs`
### Steps to reproduce
1. Run the OrchardCore Visual Studio solution-open/close performance scenario with the test asset targeting .NET 10.
2. Retarget the same asset to `net11.0` without otherwise changing the project graph.
3. Collect cumulative allocation and ETW data for `msbuild.exe`.
4. Compare allocation stacks under `ResolvePackageFileConflicts`, particularly `PlatformManifestReader` and package-override parsing.
A smaller SDK-focused benchmark can reproduce the repeated work by running at least 300 `ResolvePackageFileConflicts` task instances through the same `IBuildEngine4`, using identical platform-manifest and package-override inputs.
### Expected behavior
Identical valid targeting-pack conflict inputs should be parsed at most once within an appropriate bounded MSBuild cache lifetime. Conflict winners, output ordering and metadata, diagnostics, and task success/failure must remain unchanged.
### Actual behavior
Every `ResolvePackageFileConflicts` invocation reparses each platform manifest. Each resolver also lazily reparses the same package-override metadata when conflict resolution consults it. Large multi-project builds therefore allocate repeated strings, split arrays, versions, dictionaries, and conflict records for logically identical inputs.
### Is this a regression?
The allocation increase was observed when the OrchardCore asset moved from .NET 10 to .NET 11. Task invocation and project counts remained stable; the larger .NET 11 conflict datasets amplify existing repeated parsing rather than indicating a newly introduced invocation loop.
### Are there any workarounds?
No practical product workaround is known. Avoiding the `net11.0` retarget removes the observed increase but is not viable for .NET 11 validation.
### dotnet --info output
SDK used in both compared traces: `11.0.100-rc.1.26411.119`.
### IDE version
Visual Studio insertion containing .NET 11 Preview 7.
### Other details
#### Recommended direction
Caching is worth pursuing on `release/11.0.1xx`, but an AppDomain-lifetime registered-task-object entry per distinct input should not be implemented as initially proposed. Reused Visual Studio/MSBuild nodes would retain an effectively unbounded set of keys and values, and `allowEarlyCollection` should not be treated as eviction.
Suggested phased design:
1. Start with platform-manifest caching using `RegisteredTaskObjectLifetime.Build` and verify that this lifetime captures the repeated invocations in the performance scenario.
2. Cache only fully constructed successful parses. Invalid or missing manifests must reproduce the current diagnostics for every task invocation.
3. Cache immutable parsed manifest records, or otherwise prove that shared `ConflictItem` instances cannot be mutated concurrently.
4. Preserve relative-path resolution through `TaskEnvironment` before cache lookup.
5. Honor the existing targeting-pack cache opt-out (`DOTNETSDK_ALLOW_TARGETING_PACK_CACHING=0`).
6. Consider package-override caching separately after measuring cache cardinality and retained memory. Preserve its current lazy parsing and expose immutable/read-only results.
7. If cross-build reuse proves necessary, register one bounded cache container under a fixed assembly/schema key rather than creating an unbounded AppDomain-lifetime registered object for every input identity.
#### Design risks requiring review
- `RegisteredTaskObjectLifetime.AppDomain` corresponds to reused node-process lifetime and can trade allocation pressure for retained memory.
- Length plus last-write timestamp is not a correctness-safe manifest identity and creates obsolete entries rather than replacing the entry for a path.
- `GetRegisteredTaskObject` followed by `RegisterTaskObject` is not atomic; concurrent misses require per-key synchronization or an equivalent safe publication design.
- Failed lazy initialization must not capture one task's logger and suppress diagnostics for other invocations.
- Existing package-override dictionaries and parts of `ConflictItem` are mutable and should not be shared without a stronger immutability contract.
- Package-override key construction must not defeat the existing lazy behavior for invocations that never consult overrides.
#### Acceptance criteria
- Substantially reduce allocations across 300+ identical-input task invocations.
- Demonstrate cache hits in the original OrchardCore scenario.
- Measure both allocated bytes and retained memory across repeated builds, changed manifests, multiple SDK versions, custom override sets, and multiple MSBuild nodes.
- Preserve conflict outputs, output metadata/order, warnings/errors, and task success/failure.
- Avoid unbounded process-lifetime retention.
Related work: #25250 and #53943.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.