h-dv/code-index

Watch

Fork 0

perf: package extraction is serialized to one thread across all packages — the blocker for "all parsers are plugins" #85

New issue

Closed

opened 2026-09-01 11:22:53 +02:00 by buildagent · 1 comment

buildagent commented

2026-09-01 11:22:53 +02:00

Member

Measured during #80's pre-release review. This is the single blocker that scales the wrong way as languages move from builtin to package, and it is not on any existing list.

The defect

crates/indexer/src/packages.rs::service constructs one Supervisor on one thread and processes Jobs from an mpsc::Receiver in a while let Ok(job) = rx.recv() loop — one at a time, across all packages. Supervisor::request takes &mut self. PackageHost::extract blocks on a sync_channel(0) rendezvous reply.

Meanwhile build::dispatch_round does slice.par_iter().map(extract_one) — so the builtin path fans out across every core and every package-claimed file funnels through one serial consumer.

The existing queue-depth doc reasons correctly that the rendezvous bounds memory (depth ≤ rayon threads × file-size limit). It does not state the throughput consequence, and that is the part that matters here.

Measured cost

From mixed_load_bench on a 12-core Xeon Gold 6146:

quantity	value
warm round trip (no work)	75 µs
cold first request (incl. grammar load)	18.4 ms
grammar load alone	17.35 ms
host time per tree-sitter node	~0.585 µs (linear, 639 → 86,646 nodes)
mixed 4-package fleet	p50 22.1 ms, p99 79.5 ms
idle worker RSS	22,991 KiB Pss first, 3,343 KiB marginal (debug); 9,431 / 1,985 release

At 22 ms p50, a package claiming 20,000 files is ~7 minutes single-threaded while 11 cores idle.

Why it blocks the direction

The stated long-term architecture is that the base system builds only infrastructure and every language ships as a plugin. Under that design this serialization is not a corner case — it is the whole indexing path. The cost grows with precisely the thing the direction increases.

It is fine today because exactly one reference package exists.

Why it is a design choice, not a resource limit

The marginal idle worker costs 3,343 KiB of Pss (1,985 release), and summing VmRSS over-counts 4× at eight workers because workers share their text — so against the existing 1,024 MB fleet ceiling an idle fleet is ~275 workers in debug, ~520 in release. A worker pool is affordable; the serialization is structural, not budgetary.

Suggested shape

A pool rather than a single service thread: N workers keyed by package digest, jobs dispatched to a free worker, the rendezvous preserved per job so the memory bound its doc argues is unchanged. Supervisor already retires and respawns workers as routine operations, and workers.len() ≤ |PackageSet| is already an exact bound (ledger E11a), so the lifecycle machinery exists.

Please measure before choosing N. The settling measurement is the builtin-vs-wasm per-file A/B that does not exist anywhere in the tree today — parse the same file both ways in one bench and report the ratio. Nothing here should be sized from the round-trip figures above, which measure the trip and not the parse.

Blocks the migration direction behind #84. Adjacent: #65 (resolver fan-out), #41 (scale ceilings). Ledger rows in _prdoc/records/findings-ledger-2026-08-31.md: E11a/E11b (the worker-count and queue-depth arguments, both of which are about memory and neither about throughput), C6e (the RSS arithmetic).

Measured during #80's pre-release review. **This is the single blocker that scales the wrong way as languages move from builtin to package**, and it is not on any existing list. ## The defect `crates/indexer/src/packages.rs::service` constructs one `Supervisor` on one thread and processes `Job`s from an `mpsc::Receiver` in a `while let Ok(job) = rx.recv()` loop — **one at a time, across all packages**. `Supervisor::request` takes `&mut self`. `PackageHost::extract` blocks on a `sync_channel(0)` rendezvous reply. Meanwhile `build::dispatch_round` does `slice.par_iter().map(extract_one)` — so the **builtin path fans out across every core and every package-claimed file funnels through one serial consumer.** The existing queue-depth doc reasons correctly that the rendezvous bounds *memory* (depth ≤ rayon threads × file-size limit). It does not state the **throughput** consequence, and that is the part that matters here. ## Measured cost From `mixed_load_bench` on a 12-core Xeon Gold 6146: | quantity | value | |---|---:| | warm round trip (no work) | 75 µs | | cold first request (incl. grammar load) | 18.4 ms | | grammar load alone | 17.35 ms | | host time per tree-sitter node | ~0.585 µs (linear, 639 → 86,646 nodes) | | mixed 4-package fleet | p50 **22.1 ms**, p99 79.5 ms | | idle worker RSS | 22,991 KiB Pss first, **3,343 KiB marginal** (debug); 9,431 / 1,985 release | At 22 ms p50, **a package claiming 20,000 files is ~7 minutes single-threaded while 11 cores idle.** ## Why it blocks the direction The stated long-term architecture is that the base system builds only infrastructure and **every language ships as a plugin**. Under that design this serialization is not a corner case — it is the whole indexing path. The cost grows with precisely the thing the direction increases. It is fine today because exactly one reference package exists. ## Why it is a design choice, not a resource limit The marginal idle worker costs **3,343 KiB of Pss** (1,985 release), and summing `VmRSS` over-counts 4× at eight workers because workers share their text — so against the existing 1,024 MB fleet ceiling an idle fleet is **~275 workers in debug, ~520 in release**. A worker pool is affordable; the serialization is structural, not budgetary. ## Suggested shape A pool rather than a single service thread: N workers keyed by package digest, jobs dispatched to a free worker, the rendezvous preserved per job so the memory bound its doc argues is unchanged. `Supervisor` already retires and respawns workers as routine operations, and `workers.len() ≤ |PackageSet|` is already an exact bound (ledger E11a), so the lifecycle machinery exists. **Please measure before choosing N.** The settling measurement is the builtin-vs-wasm per-file A/B that does not exist anywhere in the tree today — parse the same file both ways in one bench and report the ratio. Nothing here should be sized from the round-trip figures above, which measure the trip and not the parse. ## Related Blocks the migration direction behind #84. Adjacent: #65 (resolver fan-out), #41 (scale ceilings). Ledger rows in `_prdoc/records/findings-ledger-2026-08-31.md`: E11a/E11b (the worker-count and queue-depth arguments, both of which are about memory and neither about throughput), C6e (the RSS arithmetic).

buildagent referenced this issue

2026-09-01 11:23:15 +02:00

plugin ABI: the qualifier-TEXT gap (gaps 1 and 2 CLOSED; gap 3 is a #77 capability decision, not an ABI gap) #86

buildagent referenced this issue

2026-09-01 11:23:38 +02:00

plugin: a package cannot replace a builtin, and a guest cannot name a node kind #87

buildagent referenced this issue

2026-09-04 09:53:42 +02:00

epic: runtime plugin architecture — dynamic grammar, extraction and resolution packages #75

buildagent commented

2026-09-04 12:20:12 +02:00

Author

Member

The premise is stale — the pool shipped in v0.24.0. The measurement did not exist, and now does.

The serialization is already gone

crates/indexer/src/packages.rs::service does not run one Supervisor on one thread. Commit 312b3cf — "feat: the package host serves on a pool of lanes (#80 P3)", 2026-09-01, shipped in v0.24.0 — replaced it with exactly the shape this issue proposed: a crossbeam_channel receiver, N lane threads each owning its own Supervisor (so PR_SET_PDEATHSIG still fires on the forking thread), one shared SharedHealth, the per-job rendezvous kept, and serving_high_water published so concurrency is measured rather than argued. PackageHost::from_set_with_lanes lets a test pin the width.

This issue was last touched 2026-09-04 and still describes the pre-312b3cf code. That is my error to own: I cited it as an open blocker twice today without checking the code first.

What genuinely did not exist: the A/B this issue named

"The settling measurement is the builtin-vs-wasm per-file A/B that does not exist anywhere in the tree today — parse the same file both ways in one bench and report the ratio."

Correct, and plugin_path_cost.rs is not it: its guest leg runs tree-sitter-json while its builtin leg runs tree-sitter-javascript, which its own text calls confound #1 and the reason its ratio is only a floor.

That confound became removable today, when tests/grammars/tree-sitter-ruby.wasm landed beside the native tree-sitter-ruby 0.23.1 workspace pin (#84 Phase 1). One grammar, two forms, same version.

New crates/plugin-host/tests/grammar_ab.rs parses identical Ruby bytes natively and through the host's Grammar::parse:

rung     bytes    nodes   native us          ratio, 6 isolated runs
  x1       418      155         123   1.52 1.50 1.51 1.49 1.49 1.50
  x4      1672      617         491   1.51 1.76 1.53 1.50 1.48 1.50
 x32     13376     4929        4053   1.25 1.47 1.49 1.46 1.47 1.47
x256    107008    39425       33991   1.45 1.50 1.55 1.40 1.40 1.39
x1024   428032   157697      145118   1.48 1.46 1.39 1.47 1.45 1.55

≈1.5x, flat across three decades. 28 of 30 readings in 1.39–1.55. ≈0.30 µs/source-byte, ≈0.93 µs/node, linear. Every run taken with zero other cargo/rustc processes, release, --test-threads=1, /proc/loadavg printed at both ends — never compared against a contended run.

Why N did not move, as a decision rather than an omission

package_pool_lanes derives lanes = clamp(1, available_parallelism, 128/packages). Neither term takes a per-file cost as an input: the parallelism ceiling is about which lanes can ever have a job (measured: a 16-lane host never exceeded 12 serving), and the clamp is about how many workers fit in memory. A 1.5x path costs 1.5x on the same cores.

What a 30x answer would have changed is whether #84's migration is worth doing at all — and 1.5x is that question answered. The measurement is now recorded in package_pool_lanes' own doc rather than left as an unwritten conclusion.

The memory argument was re-checked rather than assumed: extract still builds one rendezvous per job and still blocks, so depth is still "threads simultaneously inside extract", and widening the consumer adds no producer. Reply::send consumes the Work, so holding a file's bytes across the reply does not compile. Idle worker re-measured on this box: first 9593 KiB Pss, 8th marginal 1998 KiB (this issue said 1985).

Two method findings worth keeping

A harness bias, found and removed. The first version timed a node-count walk inside the loop. That walk is native tree_sitter::Node code on both legs, so it added the same constant to numerator and denominator: 1.41–1.47x with it in, 1.39–1.55x with only the parse. The bias sat inside the spread — but "it was small" is only sayable after taking it out.

A one-sided bound is blind to the likely failure. The ceiling passed the swapped-leg mutation (1.04 ≤ 3.0). Every way of getting this harness wrong makes the ratio smaller, so the file ships a floor as well as a ceiling, and the floor mutation — timing one parser twice — goes red at 1.00x. The ceiling is deliberately loose at 3.0: the failure it can actually catch (wasmtime ceasing to compile the grammar) is orders of magnitude, and it must survive CI hardware that is not this box. It does not claim to catch a 20% regression, and nothing in the tree does.

Registered in release_gate.rs::TIMING_GATES and wired into the weekly plugin-path-cost job, with mutations proving both the registration and the --test-threads=1 flag are graded.

Closing

The serialization this issue reports is fixed and released; the measurement it asked for is now in the tree with both bounds and a CI home. Nothing here is left to do under this number.

## The premise is stale — the pool shipped in v0.24.0. The measurement did not exist, and now does. ### The serialization is already gone `crates/indexer/src/packages.rs::service` does **not** run one `Supervisor` on one thread. Commit `312b3cf` — *"feat: the package host serves on a pool of lanes (#80 P3)"*, 2026-09-01, shipped in **v0.24.0** — replaced it with exactly the shape this issue proposed: a `crossbeam_channel` receiver, N lane threads each owning its own `Supervisor` (so `PR_SET_PDEATHSIG` still fires on the forking thread), one shared `SharedHealth`, **the per-job rendezvous kept**, and `serving_high_water` published so concurrency is measured rather than argued. `PackageHost::from_set_with_lanes` lets a test pin the width. This issue was last touched 2026-09-04 and still describes the pre-`312b3cf` code. That is my error to own: I cited it as an open blocker twice today without checking the code first. ### What genuinely did not exist: the A/B this issue named > *"The settling measurement is the builtin-vs-wasm per-file A/B that does not exist anywhere in the tree today — parse the same file both ways in one bench and report the ratio."* Correct, and `plugin_path_cost.rs` is not it: its guest leg runs `tree-sitter-json` while its builtin leg runs `tree-sitter-javascript`, which its own text calls **confound #1** and the reason its ratio is only a floor. That confound became removable **today**, when `tests/grammars/tree-sitter-ruby.wasm` landed beside the native `tree-sitter-ruby` 0.23.1 workspace pin (#84 Phase 1). One grammar, two forms, same version. New `crates/plugin-host/tests/grammar_ab.rs` parses identical Ruby bytes natively and through the host's `Grammar::parse`: ``` rung bytes nodes native us ratio, 6 isolated runs x1 418 155 123 1.52 1.50 1.51 1.49 1.49 1.50 x4 1672 617 491 1.51 1.76 1.53 1.50 1.48 1.50 x32 13376 4929 4053 1.25 1.47 1.49 1.46 1.47 1.47 x256 107008 39425 33991 1.45 1.50 1.55 1.40 1.40 1.39 x1024 428032 157697 145118 1.48 1.46 1.39 1.47 1.45 1.55 ``` **≈1.5x, flat across three decades.** 28 of 30 readings in 1.39–1.55. ≈0.30 µs/source-byte, ≈0.93 µs/node, linear. Every run taken with **zero other cargo/rustc processes**, release, `--test-threads=1`, `/proc/loadavg` printed at both ends — never compared against a contended run. ### Why N did not move, as a decision rather than an omission `package_pool_lanes` derives `lanes = clamp(1, available_parallelism, 128/packages)`. **Neither term takes a per-file cost as an input**: the parallelism ceiling is about which lanes can ever *have* a job (measured: a 16-lane host never exceeded 12 serving), and the clamp is about how many workers fit in memory. A 1.5x path costs 1.5x on the same cores. What a 30x answer *would* have changed is whether #84's migration is worth doing at all — and 1.5x is that question answered. The measurement is now recorded in `package_pool_lanes`' own doc rather than left as an unwritten conclusion. The memory argument was re-checked rather than assumed: `extract` still builds one rendezvous per job and still blocks, so depth is still "threads simultaneously inside `extract`", and widening the consumer adds no producer. `Reply::send` **consumes** the `Work`, so holding a file's bytes across the reply does not compile. Idle worker re-measured on this box: first 9593 KiB Pss, 8th marginal **1998 KiB** (this issue said 1985). ### Two method findings worth keeping **A harness bias, found and removed.** The first version timed a node-count walk inside the loop. That walk is native `tree_sitter::Node` code on *both* legs, so it added the same constant to numerator and denominator: 1.41–1.47x with it in, 1.39–1.55x with only the parse. The bias sat inside the spread — but *"it was small" is only sayable after taking it out*. **A one-sided bound is blind to the likely failure.** The ceiling **passed** the swapped-leg mutation (1.04 ≤ 3.0). Every way of getting this harness wrong makes the ratio *smaller*, so the file ships a **floor as well as a ceiling**, and the floor mutation — timing one parser twice — goes red at 1.00x. The ceiling is deliberately loose at 3.0: the failure it can actually catch (wasmtime ceasing to compile the grammar) is orders of magnitude, and it must survive CI hardware that is not this box. It does not claim to catch a 20% regression, and nothing in the tree does. Registered in `release_gate.rs::TIMING_GATES` and wired into the weekly `plugin-path-cost` job, with mutations proving both the registration and the `--test-threads=1` flag are graded. ### Closing The serialization this issue reports is fixed and released; the measurement it asked for is now in the tree with both bounds and a CI home. Nothing here is left to do under this number.

buildagent closed this issue

2026-09-04 12:20:17 +02:00

buildagent referenced this issue

2026-09-04 15:20:03 +02:00

perf/reliability: tier 1R and C# partial resolution scale quadratically and hide progress #65

buildagent referenced this issue

2026-09-04 15:21:20 +02:00

gate: plugin conformance, hostile-runtime containment and production disclosure #80