Add limit on trim analyzer CFG convergence iterations #118760
Conversation
jtschuster
added
the
area-Tools-ILLink
dotnet-policy-service
bot
added
the
linkable-framework
There was a problem hiding this comment.
Pull Request Overview
This PR adds a configurable limit on control flow graph (CFG) convergence iterations in the trim analyzer to prevent excessive memory allocation and build time issues. The change addresses a performance problem where large block counts cause the CFG convergence algorithm to perform millions of iterations, each allocating new dictionaries and consuming gigabytes of memory.
Key Changes:
- Introduces a maximum iteration limit of 10,000 for CFG convergence
- Adds early termination logic to prevent runaway iteration scenarios
- Provides a temporary fix while a more comprehensive performance solution is developed
|
Tagging subscribers to this area: @dotnet/illink |
Added an Info diagnostic, but happy to make it a warning if we think it's warranted. |
|
My vote is to make it a warning, since it's possible that it hides a real trimming issue. |
|
@jtschuster I see you have the milestone set as 10.0, but it looks like this PR missed the |
|
/backport to release/10.0-rc1 |
|
Started backporting to release/10.0-rc1: https://github.com/dotnet/runtime/actions/runs/17046124731 |
|
@jtschuster backporting to "release/10.0-rc1" failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Add limit on CFG convergence iterations
Using index info to reconstruct a base tree...
M src/tools/illink/src/ILLink.Shared/DataFlow/ForwardDataFlowAnalysis.cs
Falling back to patching base and 3-way merge...
Auto-merging src/tools/illink/src/ILLink.Shared/DataFlow/ForwardDataFlowAnalysis.cs
No changes -- Patch already applied.
Applying: Add warning for CFG convergence bailout
Using index info to reconstruct a base tree...
M src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs
M src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
M src/tools/illink/src/ILLink.Shared/DataFlow/ForwardDataFlowAnalysis.cs
M src/tools/illink/src/ILLink.Shared/DiagnosticId.cs
M src/tools/illink/src/ILLink.Shared/SharedStrings.resx
Falling back to patching base and 3-way merge...
Auto-merging src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
CONFLICT (content): Merge conflict in src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0002 Add warning for CFG convergence bailout
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
Looks like it made it in with #118812 |
Fixes #118121
With large block counts, the CFG convergence takes a long time and performs a lot of Meets, allocating a new Dictionary for each Value in each block. This adds up to gigabytes of allocations in the process.
Ideally we would fix the performance issue with some optimization, but I'm still working on a proper solution. To fix the issue for .NET 10 in the meantime, we can add a limit to the number of iterations to bail out if we reach an excessively large number of iterations. The largest number of iterations in the runtime build is 2600, so 10,000 seems fairly reasonable to me. The build time of the project in #118121 is around ~20 seconds with 10,000 iterations and ~70 seconds with 20,000.