Repository navigation
perf(loc): quadratic doc test in C# accessors and PHP signatures #1612
Description
On PR #1609, the per-comment placement test for doc comments costs O(siblings) in two shapes, so the LOC walk is quadratic in the number of doc comments:
- C#: doc comments inside a property's accessor list (between
{andget); - PHP: doc comments between a function's signature and its body.
main is unaffected, since it has no such test.
| Input | main |
PR #1609 @ c9e449e |
|---|---|---|
C#, 4k /// lines in int X { … get; } |
0.01 s | 0.28 s |
| C#, 8k | 0.01 s | 1.14 s |
| C#, 16k | 0.02 s | 4.55 s |
C#, 8k plain // (control) |
0.02 s | 0.02 s |
PHP, 4k /** */ between function f() and { |
0.01 s | 0.05 s |
| PHP, 8k | 0.01 s | 0.14 s |
| PHP, 16k | 0.02 s | 0.51 s |
| PHP, 16k inside the parameter list (control) | 0.01 s | 0.01 s |
Each doubling roughly quadruples the time.
Cause.
- C#:
documents_where_it_sitsasks the comment's two nearest ancestors without the fold cache. For the property it callsis_func, which callscsharp_member_has_accessorsand thencsharp_accessor_count. That walks every child of theaccessor_list, which is exactly where the comments are. - PHP:
holds_in_body_fieldcallschild_by_field_name("body")on the function. Tree-sitter's field lookup walks past every comment child before reaching the body.
Severity. Low. Thousands of doc comments in one accessor list or one signature does not occur in hand-written code. Generated code can produce it, though, and the walk is supposed to stay linear.
Fix.
- In
csharp_member_has_accessors, stop at the firstaccessor_declaration(any()rather thancount()), and test the comment's parent kind before askingis_funcof the property. - For PHP, find the body from the comment's side: the comment precedes the body, so it is not inside it, and no field lookup is needed.
Add loc/csharp-accessor-docs and loc/php-signature-docs probes to big-code-analysis-bench, and watch each one fail with the current code.
Found during the whole-branch review of PR #1609 (#1597).
Resolution
Commit: b803085f on fix/batch-2026-10-17 (perf(loc): linear doc test in C#, PHP and Kotlin).
Premise correction — the issue's any() fix alone does not fix the shape. With any() in place and the old C# check otherwise intact, 16k /// lines in one accessor list still took 1.88 s (pre-fix 1.98 s): any() stops at the first accessor_declaration, but every /// sits before it, so each check still walks them all. The fix decides from below, the node on the comment's path, which the walk already hands over — O(1) per comment:
- C#: a property/indexer holds every doc comment in it except those in its
AccessorList.csharp_member_has_accessorsusesany()(its callers need only a bool);csharp_accessor_countis untouched (npm uses the count). - PHP: the body is the
CompoundStatementchild by kind, notchild_by_field_name("body"); an arrow function's body is its last child (by end byte, with a guard for the zero-width MISSING body tree-sitter inserts forfn() => /** t */;— found by the review pass).holds_in_body_fieldandspansare deleted (no callers left). - Kotlin (missed by the issue):
checker/kotlin.rs'schildren().findis replaced bybelow∈FunctionBody | Block, whichnode-types.jsonconfirms is the only body-like child for every kind the check covers.
Java and Groovy compare node kinds only and never had the scan.
Probes (big-code-analysis-bench/src/shapes.rs, rows in docs/development/benchmarking.md), each watched failing with the defect reinstated: loc/csharp-accessor-docs 1.96 → 0.95, loc/php-signature-docs 1.93 → 0.98, loc/kotlin-signature-docs 1.98 → 0.97 (bound 1.50). All 62 probes pass.
Corpus identity: loc JSON from the pre- and post-fix binaries over all 247 C#/PHP/Kotlin/Java/Groovy files in the checkout: 0 diffs. No snapshot moved; submodule unchanged.
One deliberate behaviour change, outside the corpus: a C# doc comment in the header or initializer of a property/indexer with accessors (int X /// d { get; }, } /// d = 2;) was dloc 1 and is now 0. A fuzz of 231 doc-comment insertions found 18 diffs, all of this kind, none in PHP or Kotlin. The reviewer checked it against Roslyn's UnprocessedDocumentationCommentFinder: a valid location is marked on entering the property and cleared at the first token's leading trivia, so both placements are CS1587 — the old 1 was an accident of is_func returning false for a property with accessors, and this is the consistent reading of the rule #1597 recorded ("Roslyn binds one only before the member"). Recorded under Changed in the CHANGELOG.
Known remaining inconsistency (not changed here): a doc comment inside the accessor list before get; still counts as dloc 1 (pinned by the new test). Roslyn has no accessor declarations in its valid-location list, so that is CS1587 too. It is pre-existing behaviour, untouched by this commit; making it 0 is a one-line widening of the C# arm. Left for a follow-up decision rather than folded into a perf commit.
Tests: three assert_doc_rows tests in src/metrics/loc/dloc.rs (18 cases); every mutation of the three hooks fails at least one. .bca-baseline.toml: checker.rs file-level loc +3 (the spelled-out any() chain).
Gate: BCA_GATE: pass (gate=pre-commit).
Activity
dekobon commentedon Oct 19, 2026
Resolution
Commit:
b803085fonfix/batch-2026-10-17(perf(loc): linear doc test in C#, PHP and Kotlin).Premise correction — the issue's
any()fix alone does not fix the shape. Withany()in place and the old C# check otherwise intact, 16k///lines in one accessor list still took 1.88 s (pre-fix 1.98 s):any()stops at the firstaccessor_declaration, but every///sits before it, so each check still walks them all. The fix decides frombelow, the node on the comment's path, which the walk already hands over — O(1) per comment:AccessorList.csharp_member_has_accessorsusesany()(its callers need only a bool);csharp_accessor_countis untouched (npm uses the count).CompoundStatementchild by kind, notchild_by_field_name("body"); an arrow function's body is its last child (by end byte, with a guard for the zero-width MISSING body tree-sitter inserts forfn() => /** t */;— found by the review pass).holds_in_body_fieldandspansare deleted (no callers left).checker/kotlin.rs'schildren().findis replaced bybelow∈FunctionBody | Block, whichnode-types.jsonconfirms is the only body-like child for every kind the check covers.Java and Groovy compare node kinds only and never had the scan.
Probes (
big-code-analysis-bench/src/shapes.rs, rows indocs/development/benchmarking.md), each watched failing with the defect reinstated:loc/csharp-accessor-docs1.96 → 0.95,loc/php-signature-docs1.93 → 0.98,loc/kotlin-signature-docs1.98 → 0.97 (bound 1.50). All 62 probes pass.Corpus identity:
locJSON from the pre- and post-fix binaries over all 247 C#/PHP/Kotlin/Java/Groovy files in the checkout: 0 diffs. No snapshot moved; submodule unchanged.One deliberate behaviour change, outside the corpus: a C# doc comment in the header or initializer of a property/indexer with accessors (
int X /// d { get; },} /// d = 2;) wasdloc1 and is now 0. A fuzz of 231 doc-comment insertions found 18 diffs, all of this kind, none in PHP or Kotlin. The reviewer checked it against Roslyn'sUnprocessedDocumentationCommentFinder: a valid location is marked on entering the property and cleared at the first token's leading trivia, so both placements are CS1587 — the old 1 was an accident ofis_funcreturning false for a property with accessors, and this is the consistent reading of the rule #1597 recorded ("Roslyn binds one only before the member"). Recorded under Changed in the CHANGELOG.Known remaining inconsistency (not changed here): a doc comment inside the accessor list before
get;still counts asdloc1 (pinned by the new test). Roslyn has no accessor declarations in its valid-location list, so that is CS1587 too. It is pre-existing behaviour, untouched by this commit; making it 0 is a one-line widening of the C# arm. Left for a follow-up decision rather than folded into aperfcommit.Tests: three
assert_doc_rowstests insrc/metrics/loc/dloc.rs(18 cases); every mutation of the three hooks fails at least one..bca-baseline.toml:checker.rsfile-level loc +3 (the spelled-outany()chain).Gate:
BCA_GATE: pass (gate=pre-commit).dekobon commentedon Oct 19, 2026
Judgement call confirmed by the maintainer: the C# header/initializer doc comment (
int X /// d { get; },} /// d = 2;) stayscloc. Two independent checks agree it is Roslyn's rule (theUnprocessedDocumentationCommentFinderreading here, and running Roslyn under .NET 9 in #1613 part b); the olddloc1 was an accident ofis_funcreturning false for a property with accessors, not a rule anyone chose. Recorded under Changed in the CHANGELOG. The accessor-list inconsistency this commit left (a///beforeget;stilldloc) was closed by #1613 part b.perf(loc): linear doc test in C#, PHP and Kotlin
fix(loc): bind a doc comment to what follows it
fix(loc): bind C#, PHP and JS doc comments
docs(changelog): record the LOC and doc-comment fixes
perf(loc): linearize C#, PHP and Kotlin doc tests
fix(loc): bind a doc comment to what follows it
fix(loc): bind C#, PHP and JS doc comments
docs(changelog): record the LOC and doc-comment fixes
perf(loc): linearize C#, PHP and Kotlin doc tests
fix(loc): bind a doc comment to what follows it
fix(loc): bind C#, PHP and JS doc comments
docs(changelog): record the LOC and doc-comment fixes
dekobon commentedon Oct 20, 2026
Pull request: #1650.