perf(metrics): bound the cognitive nesting map

CHANGELOG.md

@@ -24,6 +24,26 @@ for historical reference.
 
 ## [Unreleased]
 
+### Performance
+
+- The metric walk's cognitive nesting map no longer grows to one entry
+  per AST node (#1375). Each node's slot was seeded by its parent, read
+  once by its own `compute`, read once more when its children were
+  seeded, and then kept until the walk ended; the map was also reserved
+  at the tree's node count up front. On a 28 MB generated
+  `tree-sitter` `parser.c` that reserve was 540 MB of a 1,265 MB peak.
+  The slot is now freed by its last reader, so the map holds only the
+  seeded-but-unvisited nodes — bounded by the traversal stack, not the
+  tree. Same file, release build, `-j 1`: 1,265 MB → 737 MB peak RSS
+  and 5.1 s → 3.9 s; `metrics -O json` over a 100k-file cargo registry
+  at `-j 16`: 5.1 GB → 3.6 GB; `tests/repositories/DeepSpeech` at
+  `-j 16`: 488 MB → 439 MB. No metric value moves. What remains is
+  dominated by the parse rather than by the walk: a parse-only
+  `bca dump` over the same files peaks at 25–70× the source size, which
+  the CI recipe now records as the in-flight term in its `--jobs` sizing
+  guidance — alongside the destinations that add a result-set term on
+  top of it (`metrics --output <FILE>`, structured stdout).
+
 ### Changed
 
 - The `tree-sitter` runtime is `=0.26.13`, up one upstream patch

big-code-analysis-book/src/recipes/ci.md

@@ -868,7 +868,21 @@ Applies regardless of provider:
   cgroup-/cpuset-/quota-aware on Linux, OS CPU count on
   macOS/Windows — so CI runners no longer need to thread
   `--jobs "$(nproc)"` through every recipe. `--jobs 1`
-  remains a debugging knob, not a default.
+  remains a debugging knob, not a default. It is *not*
+  memory-aware. Each worker holds one whole file at a time and a
+  parse-only run measures 25–70× the source size, so the
+  *in-flight* term is roughly `jobs × 70 × the largest source
+  file` in the tree — and `--jobs` bounds that term only.
+  Whether it is the whole story depends on the destination:
+  `--output-dir` writes each document as it is produced and
+  `bca check` reduces each file to its violations, so both stay
+  at the in-flight bound, while `metrics --output <FILE>`
+  collects every file's result and serializes it after the walk,
+  and structured stdout holds finished documents while an
+  earlier file is still being analyzed — which is worst when one
+  large file sorts early. A memory-limited container with many
+  vCPUs should pass an explicit `--jobs`, and prefer
+  `--output-dir` to `--output <FILE>` on a large tree.
 - **Always pass `--strip-prefix "$PWD/"` to `bca report markdown`**
   so the path column is identical across runners with different
   workspace paths. Without it the diff between two reports is

src/macros/mod.rs

@@ -42,10 +42,6 @@ macro_rules! implement_metric_trait {
     (Cognitive, $($code:ident),+) => (
         $(
            impl Cognitive for $code {
-               // No slot is ever written, so the walker must not
-               // pre-size a nesting map this grammar leaves empty.
-               const SEEDS_NESTING: bool = false;
-
                fn compute<'a>(
                    _node: &Node<'a>,
                    _code: &'a [u8],

src/metrics/cognitive.rs

@@ -153,16 +153,6 @@ pub(crate) trait Cognitive
 where
     Self: Checker,
 {
-    /// Whether [`compute`](Cognitive::compute) writes a `nesting_map`
-    /// slot for the node it was handed.
-    ///
-    /// Every real implementation does, on every path, which is what
-    /// lets the walker size the map to the node count up front. The
-    /// macro-generated no-op impls (`Preproc`, `Ccomment`) never write
-    /// one, so their map stays empty and must stay unallocated too —
-    /// they override this to `false`.
-    const SEEDS_NESTING: bool = true;
-
     /// Walk `node` and update `stats` with this metric for the language
     /// implementing the trait.
     ///
@@ -872,63 +862,6 @@ mod tests {
         }
     }
 
-    /// `SEEDS_NESTING` must say what `compute` actually does.
-    ///
-    /// The walker trusts the const twice over: `true` means the map is
-    /// worth sizing to the node count up front, `false` means it must be
-    /// left unallocated. Both are silent when wrong — a `false` on a
-    /// language that does seed only costs the rehashing back, and a
-    /// `true` on one that does not only wastes an allocation, so no
-    /// metric value moves either way. This pins each impl against what
-    /// it observably writes.
-    #[test]
-    fn seeds_nesting_matches_what_compute_writes() {
-        use std::path::Path;
-
-        fn writes_a_slot<T: ParserTrait>(source: &str, filename: &str) -> bool {
-            let parser = T::new(source.as_bytes().to_vec(), Path::new(filename), None);
-            let mut nesting_map = NestingMap::default();
-            T::Cognitive::compute(
-                &parser.root(),
-                parser.code(),
-                Ancestors::known(&[]),
-                &mut Stats::default(),
-                &mut nesting_map,
-            );
-            assert_eq!(
-                <T::Cognitive as Cognitive>::SEEDS_NESTING,
-                !nesting_map.is_empty(),
-                "{filename}: SEEDS_NESTING disagrees with what compute wrote"
-            );
-            !nesting_map.is_empty()
-        }
-
-        // One representative of each family that carries a real impl:
-        // the C-like macro, the JS-family macro, and the two languages
-        // with hand-written `compute` bodies.
-        assert!(writes_a_slot::<CppParser>(
-            "int main() { return 0; }",
-            "a.cpp"
-        ));
-        assert!(writes_a_slot::<JavascriptParser>(
-            "function f() { return 0; }",
-            "a.js"
-        ));
-        assert!(writes_a_slot::<PythonParser>(
-            "def f():\n    pass\n",
-            "a.py"
-        ));
-        assert!(writes_a_slot::<ElixirParser>(
-            "def f do\n  :ok\nend\n",
-            "a.ex"
-        ));
-
-        // The macro-generated no-ops: `SEEDS_NESTING` is what keeps the
-        // walker from reserving a map they never fill.
-        assert!(!writes_a_slot::<PreprocParser>("#define A 1\n", "a.h"));
-        assert!(!writes_a_slot::<CcommentParser>("/* c */ int x;", "a.c"));
-    }
-
     /// A `Stats::default()` that never sees an
     /// observation must not leak the `usize::MAX` sentinel for
     /// `structural_min`. The getter collapses the sentinel to `0.0`

src/node.rs

@@ -284,17 +284,6 @@ impl<'a> Node<'a> {
         self.0.child_count()
     }
 
-    /// Number of nodes in this node's subtree, counting the node itself.
-    ///
-    /// `O(1)`: `tree_sitter` stores the visible-descendant count on each
-    /// subtree, so this is a field read rather than a walk. It counts the
-    /// same nodes the metric walk visits — visible children, named and
-    /// anonymous alike — which is what makes it usable as an exact
-    /// capacity for a per-node map (see `spaces::compute::metrics_inner`).
-    pub(crate) fn descendant_count(&self) -> usize {
-        self.0.descendant_count()
-    }
-
     // Returns `true` if this node is a named grammar production
     // (as opposed to an anonymous token such as a punctuation or
     // keyword literal). Used to skip anonymous tokens like the
@@ -1275,42 +1264,6 @@ mod tests {
         );
     }
 
-    /// `descendant_count` must count the same nodes the metric walk
-    /// visits, because `spaces::compute::metrics_inner` uses it as the
-    /// exact capacity for a map that ends up holding one entry per
-    /// visited node.
-    ///
-    /// The risk it guards is silent: `ts_node_descendant_count` counts
-    /// *visible* descendants, so were it ever to narrow to named nodes
-    /// only, the reserve would under-size by the anonymous-token share
-    /// of the tree — roughly half — and the map would quietly go back
-    /// to rehashing, with no test failing. The source below is chosen to
-    /// carry plenty of anonymous tokens (`int`, `(`, `{`, `=`, `;`) so
-    /// the named-only reading is not accidentally equal.
-    #[test]
-    fn descendant_count_matches_the_walked_node_population() {
-        let code = b"int main() { int x = 1; foo(x); return 0; }";
-        let tree = Tree::new::<crate::langs::CppCode>(code);
-        let root = tree.get_root();
-
-        // `preorder` yields the node itself and then every descendant,
-        // enumerating children exactly as the metric walk's
-        // `push_children` does.
-        let walked = root.preorder().count();
-        assert_eq!(
-            root.descendant_count(),
-            walked,
-            "descendant_count must equal the pre-order node count"
-        );
-
-        let named = root.preorder().filter(Node::is_named).count();
-        assert!(
-            named < walked,
-            "fixture must contain anonymous tokens, else the assertion \
-             above cannot distinguish a named-only count"
-        );
-    }
-
     /// Drains `iter`, holding it to the `ExactSizeIterator` contract at
     /// every step, and returns the `(id, kind_id)` of each child yielded.
     ///

src/node/parser_cache.rs

@@ -7,11 +7,15 @@
 //! already grown. Construction is *not* what costs; the re-grown buffers
 //! are, and only modestly — see issue #1118 for the measurements.
 //!
-//! What a thread keeps between files is bounded by the largest file it
-//! has parsed (tens of KiB): the subtree and stack-node pools are
-//! capped, but the scratch arrays around them are cleared without
-//! releasing capacity, so a long-lived server holds that per worker for
-//! the process lifetime.
+//! What a thread keeps between files is the subtree and stack-node
+//! pools, which are capped at small fixed counts, plus the scratch
+//! arrays around them, which are cleared without releasing capacity —
+//! so a long-lived server holds the latter per worker for the process
+//! lifetime. Those arrays track parse-stack depth and reduce-action
+//! counts rather than input size, so this is deliberately not stated as
+//! a bound in bytes: the "tens of KiB" figure that stood here was never
+//! measured, and #1375 established only that the *input* can run to
+//! tens of MB, not that the retention follows it.
 //!
 //! Only the parser is cached, not the language bound to it. Rebinding
 //! per file costs nothing measurable — the gain survives consecutive

src/spaces/compute.rs

@@ -6,11 +6,15 @@
 //! verbatim and re-exported from the parent so the public path
 //! `crate::spaces::analyze` (and `pub(crate) metrics_inner`) is preserved.
 
-use std::hash::BuildHasherDefault;
-
 use super::*;
 use crate::diag::warn;
 
+// Walks that ended with a cognitive nesting slot still live. Freeing
+// each slot when its node is popped keeps the map bounded by the
+// traversal stack instead of the tree (#1375); reverting that changes
+// no output, only the peak, so this is what a test can observe.
+crate::observation::counter!(nesting_slots_retained);
+
 /// Derives the two metrics that read from a space's *complete* state:
 /// Halstead's `Stats` from the accumulated occurrence maps, and MI from
 /// the resulting volume plus the space's final LOC and cyclomatic.
@@ -562,11 +566,14 @@ fn propagate_nesting_to_children(
     children: &[(Node<'_>, Walk)],
     nesting_map: &mut NestingMap,
 ) {
-    // Leaves are roughly half of a real AST, so bail before hashing a key
-    // we would only read to iterate zero children.
-    if children.is_empty() {
-        return;
-    }
+    // `remove`, not `get`: this is the slot's last reader — its own
+    // `compute` has already consumed it and the children seeded below
+    // each carry a copy — so freeing it here is what keeps the map
+    // bounded by the traversal stack rather than the tree (#1375; see
+    // the declaration in `metrics_inner`). Leaves free theirs too, which
+    // is why there is no longer an empty-children bail-out ahead of the
+    // hash: it only ever skipped a read, and the slot stayed behind.
+    //
     // A miss here is a *root-only* path. Every non-root slot is created
     // by this function's `or_insert` below, before that node is ever
     // popped, so reaching a node with no slot means it had no parent to
@@ -578,9 +585,17 @@ fn propagate_nesting_to_children(
     // path: a real impl that skipped its write would still leave its
     // children seeded, and would show up as wrong nesting values, not as
     // a missing slot.
-    let Some(&inherited) = nesting_map.get(&node.id()) else {
+    let Some(inherited) = nesting_map.remove(&node.id()) else {
         return;
     };
+    // One allocation for the whole sibling set rather than a doubling
+    // chain through it. Dropping the walk's up-front reserve left this
+    // as the only place the map grows, and a generated parser table can
+    // hang half a million children off one `initializer_list` — which
+    // from an empty map is ~18 doublings and twice that many re-inserts.
+    // A `reserve` that the current capacity already covers is a compare,
+    // so the ordinary two- or three-child node pays nothing.
+    nesting_map.reserve(children.len());
     for (child, _) in children {
         nesting_map.entry(child.id()).or_insert(inherited);
     }
@@ -659,26 +674,20 @@ pub(crate) fn metrics_inner<T: ParserTrait>(
     // back to `Nesting::default()`, so a seed would change nothing for
     // grammars that compute cognitive — while for the two whose impl is
     // the macro's no-op it is the one write that would make the walk
-    // build an entry per node that nothing ever reads.
-    //
-    // Sized up front rather than grown: every real `Cognitive::compute`
-    // ends by writing its own node's slot, so the map converges on one
-    // entry per visited node and a default-capacity map rehashes its way
-    // there a doubling at a time. `descendant_count` is that final size,
-    // known in O(1) — an upper bound rather than an exact one only when
-    // `exclude_tests` prunes a subtree the walk never descends into.
+    // seed and free a slot per node that nothing ever reads.
     //
-    // Both guards exist to keep an empty map unallocated: an unselected
-    // `Cognitive` never calls `compute` at all, and the two grammars
-    // whose impl is the macro's no-op (`Preproc`, `Ccomment`) report
-    // `SEEDS_NESTING = false` because they write no slot.
-    let mut nesting_map = if selected.contains(Metric::Cognitive)
-        && <T::Cognitive as Cognitive>::SEEDS_NESTING
-    {
-        NestingMap::with_capacity_and_hasher(node.descendant_count(), BuildHasherDefault::default())
-    } else {
-        NestingMap::default()
-    };
+    // The map holds only nodes that have been seeded and not yet
+    // popped: `propagate_nesting_to_children` frees a slot as its last
+    // reader, so the live set is bounded by the traversal `stack`, never
+    // by the tree. It used to be reserved at `descendant_count` on the
+    // reasoning that it would converge on one entry per node — which it
+    // did, and on a 28 MB generated `parser.c` that reserve alone was
+    // 540 MB of a 1.27 GB peak (#1375). Grown from empty, it now peaks
+    // at the widest pending sibling set instead, and stays unallocated
+    // for good when nothing seeds the root: an unselected `Cognitive`
+    // never calls `compute`, and the two grammars whose impl is the
+    // macro's no-op (`Preproc`, `Ccomment`) write no slot.
+    let mut nesting_map = NestingMap::default();
 
     // Suppression markers are resolved inline during the walk rather
     // than queued for a post-finalize pass. When we visit a comment
@@ -757,6 +766,11 @@ pub(crate) fn metrics_inner<T: ParserTrait>(
                     .loc
                     .exclude_test_span(node.start_row(), node.end_line());
             }
+            // The pruned node was seeded by its parent and is neither
+            // computed nor propagated, so nothing else frees its slot.
+            if selected.contains(Metric::Cognitive) {
+                nesting_map.remove(&node.id());
+            }
             continue;
         }
 
@@ -816,6 +830,26 @@ pub(crate) fn metrics_inner<T: ParserTrait>(
         }
     }
 
+    // Every slot is freed by the node that last reads it, so a non-empty
+    // map here means a seeded node was popped without freeing — the
+    // O(nodes) growth #1375 removed, which no metric value can reveal.
+    //
+    // Two guards, covering different ground. The counter is what the
+    // named regression test reads, and it survives into release builds.
+    // The assertion generalises that test to every other walk the suite
+    // runs — twenty languages of corpus fixtures rather than the four
+    // the test names — which is the coverage that matters here, because
+    // the freeing rule now has two owners (this walk's prune arm and
+    // `propagate_nesting_to_children`) and a third `continue` added
+    // later would leak in silence.
+    debug_assert!(
+        nesting_map.is_empty(),
+        "every seeded nesting slot must be freed by the node that last reads it (#1375)"
+    );
+    if !nesting_map.is_empty() {
+        nesting_slots_retained::record();
+    }
+
     finalize::<T>(&mut state_stack, usize::MAX, selected);
 
     // Reserved error path: `MetricsError::EmptyRoot` is unreachable