perf(metrics): retire the per-node Node::parent calls in the metric bodies by dekobon · Pull Request #1101 · dekobon/big-code-analysis

dekobon · 2026-07-30T04:56:24Z

Closes #1096.

#1084 gave the metric walk an ancestor chain and
#1062 / #1088 threaded it through every
predicate that took an Ancestors::unknown(). What was left was the
same defect one layer down: node.parent() calls inside per-language
metric bodies, where no chain parameter existed. tree_sitter stores
no parent pointer, so Node::parent descends from the root and is
O(depth) — a per-node call is O(depth²) over a deeply nested file
however few steps it takes.

Metric values are unchanged for every language. The whole workspace
suite and the integration snapshots pass with no accepted snapshot.

The three groups

Halstead get_op_type. Getter::get_op_type,
get_op_type_with_code, get_operand_id, Halstead::compute, and
compute_halstead take an Ancestors, fed from compute_per_node and
ops_inner — both already maintained a chain. The six impls that read a
parent (python, rust, cpp, mozcpp, bash, irules) index it
now; Python's docstring arm wants a grandparent as well, which
Ancestors::iter hands over in two steps.

src/metrics/abc/. Not shaped the way the issue described. The
node.parent() is not at the top of compute; it is inside each
language's <lang>_inspect_container, which resolves the condition
slot's parent only to seed a boolean-context flag — and every caller had
already descended from that parent. So the parent is passed in rather
than rediscovered. Abc::compute takes an Ancestors for the arms that
genuinely ask about the node's own parent (< / > disambiguation,
the && / || chain walkers).

loc / npm / npa / cyclomatic / checker. loc's seven arms
and the shared add_multiline_string_ploc read the chain
Loc::compute already carried. Npm::compute, Npa::compute,
Cyclomatic::compute / compute_with_options, and
Checker::is_useful_comment gained the parameter. rm_comments grew a
chain of its own by the metric walk's truncate/push rule, built through
Ancestors::checked so a bookkeeping slip fails a debug build rather
than silently feeding Rust's is_useful_comment the wrong parent.

Two climbs the inventory could not see

rg '\.parent\(\)' src/ is the authority for direct calls, but two
O(depth) climbs hide behind helpers whose call sites it does not
match. This is #1088's lesson — a chain-fed predicate is only as linear
as the primitives it calls — arriving from the other direction.

Node::previous_sibling. All seven ABC container walkers ask it
whether a ternary's ? / : precedes the operand, and Kotlin's
Halstead short-interpolation test asks it once per string_content
token and once per operand. ts_node__prev_sibling opens with
ts_node_parent, so it carries the identical cost; without this the
fix would not have held on a ternary shape. New
Node::previous_sibling_under scans the known parent's children
instead.
Node::parent_grandparent_match. Python's Cyclomatic calls it
for every else token to ask whether the token opens a for /
while / try else-clause rather than an if, and both links
climbed.

Measurements

Four probes and three shape controls, added to
big-code-analysis-bench/src/shapes.rs with the fix, measured on the
same idle host either side:

probe	k before	k after	depth 4000, before → after
`halstead/nested-not`	1.99	0.99	~478 ms → ~0.57 ms
`abc/nested-if`	2.00	1.14	~808 ms → ~3.3 ms
`cyclomatic/nested-ternary`	2.06	1.27	~1.13 s → ~3.4 ms
`loc/nested-quote`	2.00	1.03	~9.6 s → ~9.2 ms
`halstead/nested-paren` (control)	0.99	0.97	—
`abc/nested-block` (control)	1.18	1.18	—
`cyclomatic/nested-and` (control)	1.26	1.31	—

loc/nested-quote has no shape control of its own: Elixir's loc
catch-all arm fires for every named node, so there is no version of the
shape with the trigger removed. nom/nested-quote — the same source
under a metric that does not ask for a parent — stands in.

Tests

Both new unit tests were verified by perturbing the production line and
confirming each is the only failure:

rust_keeps_a_macro_token_comment_and_strips_an_ordinary_one fails
alone when rm_comments hands is_useful_comment an empty chain.
previous_sibling_under_agrees_with_the_authoritative_lookup fails
alone (alongside java_ternary_conditions) when the scan returns the
wrong sibling. Its comment records the one break it cannot see — a
scan that never finds the node falls back to the authoritative lookup
and is correct but slow, which is the abc/nested-if probe's job.

The existing ABC suite is not vacuous about either change: passing the
wrong node as parent fails java_if_single_conditions and
java_bool_returning_terminal_kinds_count, and stubbing the ternary
sibling seed to None fails java_ternary_conditions.

Also here

refactor(halstead): every language's Halstead::compute forwards to
compute_halstead verbatim, so the added parameter grew 23 identical
impls from three lines to eight each. One macro emits them all
(−204 lines). No impl carried a per-language comment to hoist.
.bca-baseline.toml refreshed with
make self-scan-write-baseline-headroom in the same commits, per the
baseline-refresh discipline in AGENTS.md.

Not in scope

One previous_sibling walk is deliberately left: the exclude_tests
attribute scan in Checker::should_skip_subtree. Porting
previous_sibling_under to it would trade O(attributes × depth) for
O(attributes × children), which is worse on the shallow-but-wide trees
that dominate — it wants a single forward scan instead, plus a probe
that can select exclude_tests, which Probe cannot express today.
Filed as #1100.

After this, rg '\.parent\(\)' src/ returns only the Ancestors
fallback itself, Node::get_parent (dead code), the one-off start node
of a dump, act_on_node's chain seed, and tests.

#1084 gave the metric walk an ancestor chain and #1062 / #1088 threaded it through every predicate that took an `Ancestors::unknown()`. What was left was the same defect one layer down: `node.parent()` calls inside per-language metric *bodies*, where no chain parameter existed. `tree_sitter` stores no parent pointer, so `Node::parent` descends from the root and is `O(depth)` — a per-node call is `O(depth^2)` over a deeply nested file however few steps it takes. `Getter::get_op_type` (and `get_op_type_with_code`), `Abc::compute`, `Npm::compute`, `Npa::compute`, `Cyclomatic::compute` / `compute_with_options`, and `Checker::is_useful_comment` gain an `Ancestors` parameter. All are `pub(crate)`, so the published API is unchanged, and metric values are unchanged for every language. The ABC condition walkers took a different shape. Their `<lang>_inspect_container` helpers resolved the container's parent only to seed a boolean-context flag, and every caller had already descended from that parent — so the parent is passed in rather than rediscovered. Those same helpers asked `Node::previous_sibling` whether a ternary's `?` / `:` precedes the operand, and `ts_node__prev_sibling` opens with `ts_node_parent`, so it carried the identical cost; the new `Node::previous_sibling_under` scans the known parent's children instead. That is #1088's lesson applied ahead of time: a chain-fed predicate is only as linear as the primitives it calls. `rm_comments` maintains a chain of its own by the same truncate/push rule the metric walk uses, built through `Ancestors::checked` so a bookkeeping slip fails a debug build rather than silently feeding Rust's `is_useful_comment` the wrong parent. Guarded by three new probes and two new controls. `halstead/nested-not` fits `time ~ depth^k` at 1.99 before and 0.99 after, `abc/nested-if` at 2.00 and 1.14, `loc/nested-quote` at 2.00 and 1.03. At depth 4000 the three drop from ~478 ms to ~0.57 ms, ~808 ms to ~3.3 ms, and ~9.6 s to ~9.2 ms. Their `halstead/nested-paren` and `abc/nested-block` shape controls hold at 0.97-1.18 either side. Fixes #1096

Review follow-up to the previous commit. Three `Node::previous_sibling` calls survived it, each carrying the same `O(depth)` (`ts_node__prev_sibling` opens with `ts_node_parent`): - Perl's and PHP's ABC container walkers, whose ternary boolean-context seed was written differently enough from the other five to escape the sweep. They now scan the parent they are handed, like the rest. - `kotlin_is_short_interp_name`, which the Halstead path reaches once per `string_content` token and once per operand. It now takes the predecessor rather than resolving it: `kotlin_string_has_interp` already holds it while scanning, and the two `Getter` entry points read it off the chain. `Getter::get_operand_id` gains an `Ancestors` parameter for the second of those; it has one override and one caller. Also folds PHP's `argument`-unwrap into a single `(inner, parent)` binding rather than recovering the parent from a node-id comparison.

Every language's `Halstead::compute` forwards to `compute_halstead` verbatim — the per-language classification lives in `Getter`, not here — so the 23 impls were 23 copies of one signature, and #1096's added `Ancestors` parameter grew each from three lines to eight. One macro emits them all. No impl carried a per-language comment to hoist.

`Node::parent_grandparent_match` climbed with `Node::parent` for both links, and Python's `Cyclomatic` calls it for every `else` token to ask whether the token opens a `for` / `while` / `try` else-clause rather than an `if`. The issue's `rg '\.parent\(\)' src/` inventory does not find this one: the climb is inside the helper, not at the call site. It takes an `Ancestors` now, like every other two-link predicate. Its only caller already had one in scope after the previous commits. A probe pair covers it. `cyclomatic/nested-ternary` — Python's only shape that nests at constant bytes per level, since a block would have to be indented — fits `time ~ depth^k` at 2.06 before and 1.27 after, dropping from ~1.13 s to ~3.4 ms at depth 4000. Its `cyclomatic/nested-and` control, the same nesting through an arm that counts the token from its own kind, holds at 1.26 / 1.31.

codecov · 2026-07-30T05:05:47Z

Codecov Report

❌ Patch coverage is 99.05163% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.16%. Comparing base (6a06fd0) to head (7477384).

Files with missing lines	Patch %	Lines
src/node.rs	93.10%	2 Missing ⚠️
src/getter.rs	92.30%	1 Missing ⚠️
src/getter/kotlin.rs	95.23%	1 Missing ⚠️
src/metrics/abc/cpp.rs	95.23%	1 Missing ⚠️
src/metrics/abc/js_family.rs	95.83%	1 Missing ⚠️
src/metrics/abc/perl.rs	92.30%	1 Missing ⚠️
src/metrics/abc/php.rs	92.85%	1 Missing ⚠️
src/metrics/loc/python.rs	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1101      +/-   ##
==========================================
+ Coverage   98.08%   98.16%   +0.08%     
==========================================
  Files         272      272              
  Lines       67795    68295     +500     
  Branches    67365    67865     +500     
==========================================
+ Hits        66496    67045     +549     
+ Misses        888      858      -30     
+ Partials      411      392      -19

Flag	Coverage Δ
rust	`98.15% <99.05%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/checker.rs	`95.93% <100.00%> (ø)`
src/checker/c.rs	`88.46% <100.00%> (+11.53%)`	⬆️
src/checker/ccomment.rs	`100.00% <100.00%> (ø)`
src/checker/cpp.rs	`88.88% <100.00%> (+11.11%)`	⬆️
src/checker/mozcpp.rs	`88.88% <100.00%> (+11.11%)`	⬆️
src/checker/objc.rs	`85.71% <100.00%> (+10.71%)`	⬆️
src/checker/python.rs	`100.00% <100.00%> (ø)`
src/checker/rust.rs	`92.85% <100.00%> (+2.38%)`	⬆️
src/comment_rm.rs	`95.31% <100.00%> (+1.90%)`	⬆️
src/getter/bash.rs	`100.00% <100.00%> (ø)`
... and 112 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Patch coverage on #1101 came in at 97.04%. Most of the shortfall is a reporting artifact — adding a parameter turned one-line signatures into five-line ones, and llvm-cov attributes no region to a trait method declaration or to a multi-line match-arm guard — but underneath it were six code paths that no test reached at all. The signature change is what made them visible, so they are covered here. - `Cyclomatic::compute` for Rust, the largest single block. The metric walk goes straight to `compute_with_options` so it can pass `count_cyclomatic_try` through, which left the plain trait entry point with no caller anywhere, tests included — while #409 fixes its default as "`?` counts". Now walked against both option settings, so a delegation with the flag inverted fails. - `Checker::is_useful_comment` for C, C++, Mozilla C++, and Objective-C. All five C-family impls share one Aho-Corasick automaton whose needle is bindgen's `<div rustbindgen` marker, and only `Ccomment` had a test. Each case pairs the marker comment with an ordinary one, so an impl that answered `true` unconditionally still fails. - The `add_multiline_string_ploc` call sites for Go, Kotlin, and Mozilla C++ — the three of nineteen that no test exercised. Folded into the existing cross-language parity test for #778, since that is the decision they implement. - `csharp_walk_for_statement`'s paren/unary condition slot and both halves of `groovy_walk_for_statement`'s child-index split. Every existing `for` test in those languages uses `i < n`, whose `LT` token arm counts the condition without entering the walker at all. What remains uncovered on the patch is either a declaration llvm-cov emits no region for or a defensive branch the dispatcher cannot reach; the reachability analysis is in #1102 rather than papered over with tests that build shapes production never produces.

dekobon · 2026-07-30T12:37:14Z

Coverage follow-up in 7477384.

Patch coverage was 97.04% (25 missing). Reproduced locally with the same
cargo llvm-cov report --codecov CI uploads, then split the misses into
what a test can reach and what it cannot.

Six real gaps, now covered — all pre-existing, made visible because
this PR touched their signatures:

Cyclomatic::compute for Rust (7 lines, the largest block). The walk
calls compute_with_options directly so it can pass
count_cyclomatic_try through, which left the plain trait entry point
with no caller anywhere — tests included — while feat(metrics): make cyclomatic '?' (TryExpression) counting configurable for Rust #409 fixes its
default as "? counts".
Checker::is_useful_comment for C, C++, Mozilla C++, and
Objective-C. Five C-family impls share one Aho-Corasick automaton for
bindgen's <div rustbindgen marker; only Ccomment had a test.
The add_multiline_string_ploc call sites for Go, Kotlin, and
Mozilla C++ — three of nineteen with no test, folded into the
existing metrics(loc): multi-line string interior rows counted as PLOC in Python but as blank in Perl (cross-language inconsistency) #778 cross-language parity test.
csharp_walk_for_statement's paren/unary condition slot and both
halves of groovy_walk_for_statement's child-index split. Every
existing for test in those languages uses i < n, whose LT token
arm counts the condition without entering the walker.

Each was verified by perturbation: inverting RustCode::compute's
delegation flag and stubbing C's is_useful_comment to false each
fail exactly one test and nothing else.

23 lines remain uncovered, none of them reachable by a test:

13 are llvm-cov artifacts of the signature change — trait method
declarations (Abc, Halstead, Npm), a macro repetition's )+,
a multi-line match-arm guard whose body line is covered, and two
format arguments inside a new test's failure message.
6 are defensive branches the dispatcher cannot reach: the ABC ternary
boolean-context seed in four language families that have no ternary
walker arm, Python loc's no-parent guard for a String, and
Getter::get_op_type's default body.

The reachability analysis for those is filed as #1102 rather than
papered over — the right fix there is the ternary walker arm #403 left
unfinished, not a unit test that hands the helper a parent production
never produces.

make pre-commit is green on the pushed tree.

dekobon added 4 commits July 29, 2026 14:22

dekobon merged commit b93bacc into main Jul 30, 2026
39 checks passed

dekobon deleted the perf/1096-retire-per-node-parent-calls branch July 30, 2026 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(metrics): retire the per-node Node::parent calls in the metric bodies - #1101

perf(metrics): retire the per-node Node::parent calls in the metric bodies#1101
dekobon merged 5 commits into
mainfrom
perf/1096-retire-per-node-parent-calls

dekobon commented Jul 30, 2026

Uh oh!

codecov Bot commented Jul 30, 2026 •

edited

Loading

Uh oh!

dekobon commented Jul 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

dekobon commented Jul 30, 2026

The three groups

Two climbs the inventory could not see

Measurements

Tests

Also here

Not in scope

Uh oh!

codecov Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dekobon commented Jul 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Jul 30, 2026 •

edited

Loading