Commit Graph
5 Commits
Author SHA1 Message Date
Martin Vogel 959f4de50c perf: cut the allocation churn and repeated work the sanitizer found
Every item here was measured on a real corpus before and after, not
reasoned about. Go corpus, event lane, first run -> now: 245.5 M
allocations -> 134.8 M, 190 GB cumulative -> 29.3 GB, 158 GB never
written -> 9.2 GB, 132 M churn pairs -> 35.7 M, 93 GB of allocator byte
work -> 0.84 GB. Wall clock: Go 40.7 s -> 19.3 s, linux kernel 239.6 s ->
194.6 s, and all ten LSP languages faster with none slower.

Graph output is unchanged, proven rather than assumed: per-class multiset
digests over the full kernel index (8,563,471 nodes identical, 18/20 node
classes and 18/24 edge classes byte-identical, the rest differing only in
the classes a same-binary re-run also perturbs through worker merge order).

The changes, each with its own reason:

  arena      an exact arena that is appended to later restarts growth at
             8 KB instead of the 64 KB default — the cross-file pass adds a
             few resolved calls to a compacted result, so the default block
             was ~60 KB untouched per result, 0.5 GB of the Go worker peak.
             cbm_arena_init_lazy defers the first block for arenas that are
             usually never used at all.
  scratch    per-thread reuse instead of allocate-and-free per unit of
             work: result_compact (8 MB kept), the cross-file pass (4 MB),
             extract_defs' walk stack, and the sqlite index cells (a 1 MB
             arena replacing 12.1 M malloc/free pairs on Go). Walk frames
             moving into scratch is why pass_k8s now opens the keep window
             explicitly: without it every YAML file opened a 512 KB scratch
             on the main thread, +3 GB.
  sqlite     index cells are written in place — building the record and
             copying it into the cell was a second allocation per entry,
             16 M record buffers of pure churn on Go.
  registry   is_test_qn and common_prefix_len in one pass, per-bucket test
             flags computed once instead of per candidate comparison, and
             the reachability keys arena allocated lazily.
  minhash    the unique-trigram set is sized from the trigram count and
             skips repeats, and LSH queries reuse one seen-set per worker
             instead of allocating per query.
  parallel   lsp_surface rows and the k8s manifest prep now run across
             workers (graph writes stay sequential and ordered), the file
             node is hoisted out of register_and_link_def, and configlink
             prepares dependency imports once with a length prefilter.
  go_lsp     one sealed stdlib registry shared by all workers, published
             by CAS, instead of one per worker.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
2026-09-18 03:31:22 +02:00
Daniel Ochoa 40f2722d61 fix(extract): AST traversal stacks no longer outlive the file they walk
Fixes #2010. Refs #1997.

Indexing a 14k-file TypeScript repository holds 13.3 GB resident at
post_extract on Linux aarch64, and about 3.1 GB of it is AST traversal
scratch that no CBMFileResult points at.

ts_nstack_init cut its stacks from ctx->arena, which is result->arena. In
the parallel path that arena is stored into ec->result_cache[file_idx]
(pass_parallel.c:963) and freed only after parallel_resolve and the infra
passes (pipeline.c:1324), so a structure written for a one-file lifetime is
in fact held for the whole index, once per file, concurrently. The header
claimed otherwise and was wrong: "freed when the arena is destroyed at end
of file extraction".

The largest instance is channels. CHAN_STACK_CAP is 4096 and sizeof(TSNode)
is 32, so scan_string_consts_js (extract_channels.c:104) and
extract_channels_js (:376) take 128 KB each, per JavaScript or TypeScript
file. Across 12,673 such files that is 3,168 MB, and it produced 23 Channel
nodes out of 164,684.

CBMExtractCtx gains a scratch arena, created and destroyed by the
cbm_extract_file_ex call that builds the context. Nothing else changes about
where anything lives: ctx->arena still owns every string a CBMFileResult
points at, and ctx->scratch owns only traversal stacks, which nothing points
at.

Nothing a CBMFileResult points at moved. A context built without a scratch,
and a file whose scratch arena fails to allocate, both fall back to
ctx->arena, which is exactly the behaviour that shipped before this change,
so no path can be handed a NULL arena and silently lose nodes.

The signature change is what makes the mistake unrepresentable.
ts_nstack_init takes the context rather than an arena, so handing it
ctx->arena is an incompatible pointer type; push and push_children read the
arena the stack recorded, so a stale argument is an arity error. Both fail
under -Werror rather than silently retaining memory.

The 55 init sites are NOT one uniform sed. 29 pass ctx->arena and 26 pass a
local alias, and that alias is shared with result allocations: in
parse_zig_imports (extract_imports.c:1093) the same `a` feeds cbm_node_text,
strip_quotes, path_last and cbm_imports_push (extract_imports.c:1101, 1106,
1108, 1109). Rebinding it would have moved import names and module paths
into memory that dies with the call while the result kept pointing at them.
Each site was edited on its own and no alias was rebound. Re-derive the set
with:

  grep -rn 'ts_nstack_init' internal/cbm/extract_*.c
  grep -rn 'ts_nstack_push' internal/cbm/extract_*.c

parse_lisp_imports used its alias only for the stack, so the alias goes too.
push_nested_class_nodes and push_class_body_children took an arena purely to
reach the stack, so they take the context now; the one call site passes ctx.

Two measurements, from two different builds, and they are not the same
number.

A macOS census build, which instruments every cbm_arena_alloc and sums the
blocks behind every live CBMFileResult arena, reports the retained per-file
arena set dropping from 11,226 MB mapped to 5,680 MB, and blocks from 48,045
to 34,186. Those are mapped bytes, not resident: that build logs
mem.allocator.bound_populations_only, so untouched block tails never commit.

The resident figures come from Linux aarch64, gcc 13.3, static build, where
the build logs mem.allocator.owned classes=all and mapped equals resident.
Five runs per side on the full 13,999-file tree: mem.collect post_extract
rss_mb 13293 -> 6752 (-49.2%), mem.phase peak_mb 14840 -> 8565 (-42.3%).
nodes 164684, parse_partial 280 and registry entries/defines/imports
787954/804999/100027 identical on all ten runs; edges 835869-835874 against
835867-835881, overlapping ranges, so run to run noise. Wall clock means
35651 ms against 30502 ms, but the ranges overlap and the host had other
load, so the only supportable claim is no regression. ObjectScript Studio
Export on a small fixture: nodes 18, edges 25, both sides.

Scratch block size is 512 KB, chosen from measurement. arena_grow fires on
one file in 12,000 at both 512 KB and 1 MB, and constantly at 256 KB, where
the two channel stacks alone are exactly 262144 bytes. 512 KB therefore
costs half the resident block per worker for the same growth behaviour, and
stays under MI_LARGE_MAX_OBJ_SIZE in the vendored mimalloc, which is
MI_LARGE_PAGE_SIZE/8, exactly 524288 bytes (types.h:426, with
MI_ENABLE_LARGE_PAGES defaulting to 1 at :115 and not overridden by this
build), so a 512 KB block is still bin-allocated from a 4 MiB large page.
Growth is not free at that size for the same reason: arena_grow doubles to 1
MiB, above that bound and therefore a singleton OS allocation. One file in
twelve thousand pays it, which is why it is accepted. The largest single
file's scratch high water mark on this corpus is 33.5 MB, now transient
rather than retained.

Tests. Nothing in the suite asserted on result->arena size, block count or
total_alloc after cbm_extract_file, which is why this went unnoticed for the
life of extract_node_stack.h. traversal_stack_not_in_result_arena_issue2010
pins the byte budget: extracting "export const x = 1;" as TypeScript charged
365984 bytes to the result arena before and charges 87456 after, a
difference of 278528, exactly the two 4096-entry channel walks plus the
512-entry ES import walk at 32 bytes per TSNode. Of the 87456 that remain,
7680 is the defs item array at GROW_ARRAY's starting capacity of 32 times
sizeof(CBMDefinition) 240 and the other 79776 is everything else the
extraction interns, none of it traversal scratch; the 128 KB bound is
therefore above the new figure with room and a factor of four below the old
one. extract_traversal_stacks_come_from_ctx_scratch_issue2010 in test_mem.c
pins the lifetime, since a byte budget alone would also be satisfied by
shrinking CHAN_STACK_CAP: it builds the extraction context directly over two
arenas it owns, runs cbm_extract_channels, and asserts the scratch took at
least the 262144 bytes of the two walks while the result arena did not.
extract_c_macro_hidden_call_survives_preprocessed_pass_issue2010 is not a
scratch test. pp_ctx carries ctx->scratch so every context in the file is
uniform, but nothing reads it there: pp_ctx reaches only cbm_extract_unified
and cbm_run_c_lsp, and neither extract_unified.c nor anything under
internal/cbm/lsp/ includes extract_node_stack.h, so no traversal stack is
built on that path today. The test guards the macro-expansion path itself,
which had no assertion on a call that exists only after expansion.
ObjectScript composite extraction is already covered by
pipeline_objectscript_export_preserves_calls_sequential_parallel and
iris_export_xml_multi_class, so no new test is added there; a per-call arena
is correct on that path by construction, since each call owns and destroys
its own.

Local: scripts/test.sh, 7558 passed, 0 failed, 8 skipped, 139 suites.

Alternatives considered

Thread-local scratch arena, reset per file, with a depth guard for nesting.
Rejected on three counts. This codebase never uses thread-local storage for
owned bulk memory; every existing _Thread_local here is a small cache, a
guard or a log buffer, and every per-worker resource is a field in a context
struct. It would have needed cbm_arena_reset, which has no production caller
and a documented subtlety with grown block sizes (arena.c:213-217). And
extraction threads are created per cbm_parallel_for call and joined
(worker_pool.c:71,88) with no thread-exit hook in this project's own code,
so the arena would have leaked once per thread per dispatch and turned the
leak lane red.

Save and restore mark and release on the arena. Rejected because a per-call
arena already makes nesting correct by construction, and a mark type plus
two functions would have to be added to both arena headers.

Per-walk arenas, one per traversal rather than one per file. Rejected on
cost: 55 sites would each need an explicit span, for the same bound.

Right-sizing the initial capacities alone. CHAN_STACK_CAP at 256 rather than
4096 would cut those two sites from 3,168 MB to about 198 MB, which is real,
but it tunes the constant while the lifetime defect remains at all 55 sites,
and growth is not free: 256 doubling to 4096 abandons 122,880 bytes, and one
file in this corpus took 33.5 MB of stacks. The cap and the scratch block
size are coupled, which is a further reason to change one at a time. Worth
doing separately.

Routing to a scratch inside extract_node_stack.h while keeping the arena
parameters. Rejected because a parameter that is ignored lies about the API,
and the alias hazard above would stay a review obligation instead of a
compile error.

Freeing the per-file result arena earlier. Rejected: the results are read by
parallel_resolve and the infra passes. That is the retention half #1925
addresses, and this change is independent of it.

Signed-off-by: Daniel Ochoa <daniel.ochoa@closedloop.ai>
2026-09-02 09:22:37 -05:00
Martin Vogel 7a6a8cb7a5 style: apply clang-format-20 across tree
Format drift had accumulated across recent commits (the dry-run lint enforces clang-format-20, which had not run on these commits yet). No functional changes — whitespace/layout only, verified by a clean cppcheck + clang-format + NOLINT lint-ci pass.
2026-05-29 00:34:18 +02:00
Martin Vogel 01bdba2a3f perf(extract): eliminate O(n^2) AST traversal on wide-flat files
tree-sitter's ts_node_child(node, i) is O(i) — it walks the child iterator from the first child on every call — so the common 'for (i=0; i<count; i++) ts_node_child(node, i)' idiom is O(n^2) over a node's children. On a file whose top level is very wide (e.g. a 580k-line fixture parsed as hundreds of thousands of sibling comment nodes) this made extraction run for over an hour.

Add ts_nstack_push_children(), which enumerates a node's children in a single O(n) cursor pass, and use it in the whole-tree DFS walkers (calls, channels, env-accesses, imports, type-assigns). Convert the module-level variable walk to a cursor too, and add cbm_is_module_level_p() so callers that already hold the parent node skip the O(n) ts_node_parent() rescan.

Indexing microsoft/TypeScript drops from ~5100s to ~50s with identical graph output.
2026-05-28 13:55:48 +02:00
Martin Vogel 1468c636ee refactor(extract): rename ts_node_stack.h to extract_node_stack.h
Pure rename — no logic change. The header is consumed only by
internal/cbm/extract_*.c, depends on tree_sitter/api.h, and is purpose-
built for AST traversal during extraction. The "extract_" prefix makes
the scope explicit and keeps the file naming consistent with its
neighbours (extract_calls.c, extract_imports.c, ...).

- internal/cbm/ts_node_stack.h -> internal/cbm/extract_node_stack.h
- Include guard CBM_TS_NODE_STACK_H -> CBM_EXTRACT_NODE_STACK_H
- Header banner comment updated
- 9 call sites in internal/cbm/extract_*.c updated to the new path

Full suite: 2824 passed, 0 failed.
2026-05-08 23:26:32 +02:00