Add limit on trim analyzer CFG convergence iterations (#118760)

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. The build time of the project in issue #118121 is around ~20 seconds with 10,000 iterations and ~70 seconds with 20,000.

Author: jtschuster

src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs

@@ -63,8 +63,9 @@ protected LocalDataFlowAnalysis(
             OperationBlock = operationBlock;
         }
 
-        public void InterproceduralAnalyze()
+        public bool InterproceduralAnalyze()
         {
+            bool succeeded = true;
             ValueSetLattice<MethodBodyValue> methodGroupLattice = default;
             DictionaryLattice<LocalKey, Maybe<TValue>, MaybeLattice<TValue, TLattice>> hoistedLocalLattice = default;
             var interproceduralStateLattice = new InterproceduralStateLattice<TValue, TLattice>(
@@ -75,8 +76,8 @@ public void InterproceduralAnalyze()
 
             if (OperationBlock is IAttributeOperation attribute)
             {
-                AnalyzeAttribute(Context.OwningSymbol, attribute);
-                return;
+                succeeded &= AnalyzeAttribute(Context.OwningSymbol, attribute);
+                return succeeded;
             }
 
             Debug.Assert(Context.OwningSymbol is not IMethodSymbol methodSymbol ||
@@ -91,29 +92,31 @@ public void InterproceduralAnalyze()
                 Debug.Assert(!oldInterproceduralState.Methods.IsUnknown());
                 foreach (var method in oldInterproceduralState.Methods.GetKnownValues())
                 {
-                    AnalyzeMethod(method, ref interproceduralState);
+                    succeeded &= AnalyzeMethod(method, ref interproceduralState);
                 }
             }
+            return succeeded;
         }
 
-        private void AnalyzeAttribute(ISymbol owningSymbol, IAttributeOperation attribute)
+        private bool AnalyzeAttribute(ISymbol owningSymbol, IAttributeOperation attribute)
         {
             var cfg = Context.GetControlFlowGraph(attribute);
             var lValueFlowCaptures = LValueFlowCapturesProvider.CreateLValueFlowCaptures(cfg);
             var visitor = GetVisitor(owningSymbol, cfg, lValueFlowCaptures, default);
-            Fixpoint(new ControlFlowGraphProxy(cfg), visitor);
+            return Fixpoint(new ControlFlowGraphProxy(cfg), visitor);
         }
 
-        private void AnalyzeMethod(MethodBodyValue method, ref InterproceduralState<TValue, TLattice> interproceduralState)
+        private bool AnalyzeMethod(MethodBodyValue method, ref InterproceduralState<TValue, TLattice> interproceduralState)
         {
             var cfg = method.ControlFlowGraph;
             var lValueFlowCaptures = LValueFlowCapturesProvider.CreateLValueFlowCaptures(cfg);
             var visitor = GetVisitor(method.OwningSymbol, cfg, lValueFlowCaptures, interproceduralState);
-            Fixpoint(new ControlFlowGraphProxy(cfg), visitor);
+            bool succeeded = Fixpoint(new ControlFlowGraphProxy(cfg), visitor);
 
             // The interprocedural state struct is stored as a field of the visitor and modified
             // in-place there, but we also need those modifications to be reflected here.
             interproceduralState = visitor.InterproceduralState;
+            return succeeded;
         }
 
         protected abstract TTransfer GetVisitor(

src/tools/illink/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs

@@ -61,6 +61,7 @@ public static ImmutableArray<DiagnosticDescriptor> GetSupportedDiagnostics()
             diagDescriptorsArrayBuilder.Add(DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.ReturnValueDoesNotMatchFeatureGuards));
             diagDescriptorsArrayBuilder.Add(DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.InvalidFeatureGuard));
             diagDescriptorsArrayBuilder.Add(DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.TypeMapGroupTypeCannotBeStaticallyDetermined));
+            diagDescriptorsArrayBuilder.Add(DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.DataflowAnalysisDidNotConverge));
 
             foreach (var requiresAnalyzer in RequiresAnalyzers.Value)
             {
@@ -110,8 +111,17 @@ public override void Initialize(AnalysisContext context)
                     foreach (var operationBlock in context.OperationBlocks)
                     {
                         TrimDataFlowAnalysis trimDataFlowAnalysis = new(context, dataFlowAnalyzerContext, operationBlock);
-                        trimDataFlowAnalysis.InterproceduralAnalyze();
+                        bool success = trimDataFlowAnalysis.InterproceduralAnalyze();
                         trimDataFlowAnalysis.ReportDiagnostics(context.ReportDiagnostic);
+                        if (!success)
+                        {
+                            context.ReportDiagnostic(
+                                Diagnostic.Create(DiagnosticDescriptors.GetDiagnosticDescriptor(
+                                    DiagnosticId.DataflowAnalysisDidNotConverge,
+                                    diagnosticSeverity: DiagnosticSeverity.Warning),
+                                operationBlock.Syntax.GetLocation(),
+                                operationBlock.FindContainingSymbol(context.OwningSymbol).GetDisplayName()));
+                        }
                     }
                 });
 

src/tools/illink/src/ILLink.Shared/DataFlow/ForwardDataFlowAnalysis.cs

@@ -214,7 +214,7 @@ successor.ConditionKind is ConditionKind.WhenTrue
 
         // This just runs a dataflow algorithm until convergence. It doesn't cache any results,
         // allowing each particular kind of analysis to decide what is worth saving.
-        public void Fixpoint(TControlFlowGraph cfg, TTransfer transfer)
+        public bool Fixpoint(TControlFlowGraph cfg, TTransfer transfer)
         {
             TraceStart(cfg);
 
@@ -244,11 +244,15 @@ public void Fixpoint(TControlFlowGraph cfg, TTransfer transfer)
             };
 
             bool changed = true;
-            while (changed)
+            const int MaxIterations = 10_000;
+            int iterations = 0;
+            while (changed && iterations < MaxIterations)
             {
                 changed = false;
                 foreach (var block in cfg.Blocks)
                 {
+                    if (++iterations >= MaxIterations)
+                        break;
 
                     TraceVisitBlock(block);
 
@@ -417,6 +421,8 @@ public void Fixpoint(TControlFlowGraph cfg, TTransfer transfer)
                 }
             }
 
+            return !changed || iterations >= MaxIterations;
+
             void FlowStateThroughExitedFinallys(
                 IControlFlowGraph<TBlock, TRegion>.ControlFlowBranch predecessor,
                 ref TValue predecessorState)

src/tools/illink/src/ILLink.Shared/DiagnosticId.cs

@@ -190,6 +190,7 @@ public enum DiagnosticId
         RequiresUnreferencedCodeOnEntryPoint = 2123,
         TypeMapGroupTypeCannotBeStaticallyDetermined = 2124,
         ReferenceNotMarkedIsTrimmable = 2125,
+        DataflowAnalysisDidNotConverge = 2126,
         _EndTrimAnalysisWarningsSentinel,
 
         // Single-file diagnostic ids.

src/tools/illink/src/ILLink.Shared/SharedStrings.resx

@@ -1251,4 +1251,10 @@
   <data name="ReferenceNotMarkedIsAotCompatibleMessage" xml:space="preserve">
     <value>Referenced assembly '{0}' is not built with `<IsAotCompatible>true</IsAotCompatible>` and may not be compatible with AOT.</value>
   </data>
+  <data name="DataflowAnalysisDidNotConvergeTitle" xml:space="preserve">
+    <value>Trim dataflow analysis took too long to complete. Trim safety cannot be guaranteed.</value>
+  </data>
+  <data name="DataflowAnalysisDidNotConvergeMessage" xml:space="preserve">
+    <value>Trim dataflow analysis of member '{0}' took too long to complete. Trim safety cannot be guaranteed.</value>
+  </data>
 </root>