Repository navigation
Reduce CA1508 analysis cost for local-function calls - #56414
tannergooding merged 4 commits into
Conversation
Trim unrelated caller state for safe direct local-function calls and skip enclosing CA1508 analysis when only nested functions have candidates. Preserve reachable aliases and fall back to full state when indirect effects cannot be ruled out. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Review effort: Lite
Findings: 1
What changed in this PR
Improves CA1508 performance by trimming local-function analysis state when safe, preserving required captures and aliases, and avoiding nested-function scans.
Changes:
- Adds local-function state trimming and result-merging semantics.
- Adds conservative capture, static-state, and alias analysis.
- Expands regression coverage for value-content, null, and dispose analysis.
| File | Description |
|---|---|
| src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposeObjectsBeforeLosingScopeTests.cs | Updated as part of this pull request. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_ValueContentAnalysis.cs | Updated as part of this pull request. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode_NullAnalysis.cs | Updated as part of this pull request. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/DataFlowOperationVisitor.cs | Updated as part of this pull request. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/FlowAnalysis/FlowAnalysis/Framework/DataFlow/AnalysisEntityDataFlowOperationVisitor.cs | Updated as part of this pull request. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/PointsToAnalysis/PointsToAnalysis.PointsToDataFlowOperationVisitor.cs | Updated as part of this pull request. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/GlobalFlowStateAnalysis/GlobalFlowStateDataFlowOperationVisitor.cs | Updated as part of this pull request. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/DisposeAnalysis/DisposeAnalysis.DisposeDataFlowOperationVisitor.cs | Updated as part of this pull request. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Utilities/FlowAnalysis/FlowAnalysis/Analysis/CopyAnalysis/CopyAnalysis.CopyDataFlowOperationVisitor.cs | Updated as part of this pull request. |
| src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.CodeQuality.Analyzers/Maintainability/AvoidDeadConditionalCode.cs | Updated as part of this pull request. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reuse complete per-CFG referenced-member summaries across static local-function graphs; preserve aliases to static object roots when trimming would be unsafe. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Evangelink
left a comment
Evangelink
left a comment
There was a problem hiding this comment.
Reviewed through 96a8741. The local-function trimming, reachable-CFG scans, shared summary caching, and deterministic regression coverage look good. Focused tests and required CI checks pass.
GitHub Actions
Failure
|
CC. @baronfel, @marcpopMSFT This and #56415 need weigh-in on whether we want to backport to .NET 11 The analyzers in question that were the worst offenders are generally off by default. However, if someone has them enabled such as via One of my projects had a 23 minute build time, where fixing 56415 took it down to 5 minutes and fixing this one on top took it down to 40 seconds. They are, however, bigger changes and impact some shared analyzer infrastructure. They won't case cause more build warnings to surface and should just be a performance improvement, but they do deserve the explicit weigh-in given we're post RC2 snap and its a few hundred lines of code churn + a couple thousand lines of tests across both. |
|
These also impact .NET 10, but I don't think they meet the bar for it. Devs can just upgrade to the 11 package |
|
I will support backporting this into 11.0 servicing (post-GA) |
|
/backport to release/11.0.2xx |
|
Started backporting to |
Note
This pull request description was drafted with AI assistance.
Related to #56412.
Local-function calls were excluded from the existing interprocedural entry-state trimming, so each call cloned and reanalyzed unrelated caller state. Across many calls, that work could dominate analysis time.
Enable trimming for directly invoked local functions when their flow state is safe to analyze. Keep state reachable through captures, parameters, receiver, static state, and tracked aliases; conservatively use the full state when safety cannot be established. Distinguish trimmed call results so updates preserve tracked-state semantics, while full-state calls retain their existing replacement behavior. Memoize successfully scanned static-local bodies to avoid rescanning shared function graphs.
Also stop CA1508's enclosing-method candidate scan at nested functions, whose bodies have separate control-flow graphs. On a related 548-call reproducer, CA1508 analyzer time fell from 41.4 seconds to roughly 5–7 seconds.