perf(equivalence): avoid LINQ allocation in ClassifyConstantOnlyContext - #95
Conversation
ClassifyConstantOnlyContext chained Ancestors().Select(...).FirstOrDefault(...) to find the first ancestor that proves a mutation only touches a compile-time constant. That chain allocates three iterator layers per call: the ancestor enumerator, the Select wrapper, and the FirstOrDefault enumeration. Since this check runs for every mutation that survives the earlier no-op, constant-fold, regex and unreachable-code checks, it accounts for a large share of the allocations made while classifying mutations of a typical analyzed file. Replace the chain with a single foreach loop over the ancestors that calls GetConstantOnlyContextReason directly and returns as soon as it yields a reason. This keeps the traversal order, stopping condition and returned reason identical for every input, while collapsing the three allocations into the one enumerator the foreach itself needs. Extend the existing constant-only-context tests with cases where several non-matching ancestors sit between the mutated node and the ancestor that actually proves triviality, so the loop is exercised across multiple iterations instead of stopping on the very first candidate.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #95 +/- ##
=======================================
Coverage 95.26% 95.26%
=======================================
Files 88 88
Lines 5850 5851 +1
Branches 1273 1273
=======================================
+ Hits 5573 5574 +1
Misses 122 122
Partials 155 155 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Summary
ClassifyConstantOnlyContextinEquivalenceClassifierchainedAncestors().Select(...).FirstOrDefault(...)to find the first ancestor that proves a mutation only touches a compile-time constant. That chain allocates three iterator layers per call (the ancestor enumerator, theSelectwrapper, and theFirstOrDefaultenumeration) instead of the single enumerator a plain loop needs. Because this check runs for every mutation that survives the earlier no-op, constant-fold, regex-quantifier and unreachable-code checks, it accounts for a meaningful share of the allocations made while classifying mutations across a typical analyzed file.Fix
Replaced the LINQ chain with a
foreach (var ancestor in mutation.Original.Ancestors())loop that callsGetConstantOnlyContextReason(ancestor)directly and returns as soon as a non-null reason is found. Traversal order, stopping condition, and the returned verdict/reason are identical to the previous implementation for every input — this is a pure allocation reduction, not a behavior change.Test plan
EquivalenceClassifierBranchTestswith two new cases where several non-matching ancestors (nested binary/parenthesized expressions) sit between the mutated node and the ancestor that actually proves triviality, for both the attribute-argument and constant-local-declaration reasons — exercising the loop across multiple iterations rather than stopping on the first candidate.dotnet test ./Frameshift.slnx— full solution, all 29376 tests pass (0 failed, 0 skipped).dotnet csharpier format .(via thecsharpierglobal tool) — working tree clean of formatting diffs afterward.