perf(framework): run an analyzer's units concurrently - #54
Merged
Conversation
Sessions were the only axis of fan-out. `runAnalyzer` awaited each unit in turn, so one session could never have more than a single call in flight however the limits were set. Measured on a real corpus at 40-way session concurrency: per-call latency held steady around 1.5s, which should sustain ~26 calls/s across 40 lanes, but actual throughput was ~7.8/s — roughly 30% utilisation. The lanes were not waiting on the provider; many were busy with deterministic work (turn construction, tokenising, matching) and issuing no calls at all, while a session with 100 terms to judge serialised them through one lane for well over two minutes. Raising --llm-concurrency did not fix it, and could not: the ceiling was how many sessions happened to be doing LLM work at the same instant, not the semaphore. Going 10 → 40 bought only 1.7x. The degenerate case was worse still — `--session X` on its own ran at concurrency 1 regardless of any flag. Units now run through mapWithConcurrency, bounded by a new `unitConcurrency`, which the analyze command sets to the LLM concurrency so a single session can reach the gate by itself. This is layered under the existing limits rather than replacing them. The global semaphore around the LLM caller is still the only thing deciding provider load, and better-sqlite3 is synchronous, so persistence cannot interleave mid-write. `unitConcurrency` defaults to 1, so nothing changes for callers that do not opt in. Fail-fast on configuration faults is preserved with one honest difference: units already in flight finish, so a bad model spec now costs at most one concurrency width of error nodes instead of exactly one. That is still bounded, against the 113,992 it produced before fail-fast existed. Tests cover the properties that could plausibly break under concurrency: one node per unit with no duplicates, idempotent re-runs, the configured limit respected, default-sequential behaviour unchanged, and early stop on a configuration fault. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Sessions were the only axis of fan-out.
runAnalyzerawaited each unit in turn, so one session could never have more than a single call in flight, however the limits were set.Measured on a real corpus at 40-way session concurrency:
The lanes were not waiting on the provider. Many were doing deterministic work — turn construction, tokenising, matching — and issuing no calls at all, while a session with 100 terms to judge serialised them through one lane for over two minutes.
Raising
--llm-concurrencycould not fix this: the ceiling was how many sessions happened to be issuing calls at the same instant, not the semaphore. And the degenerate case was worse —--session Xalone ran at concurrency 1 regardless of any flag.Change
Units now run through the existing
mapWithConcurrency, bounded by a newunitConcurrency. The analyze command sets it to the LLM concurrency, so a single session can reach the gate by itself.Layered under the existing limits, not replacing them:
better-sqlite3is synchronous, so persistence cannot interleave mid-writeunitConcurrencydefaults to 1, so nothing changes for callers that do not opt inOne honest behaviour change
Fail-fast on configuration faults is preserved, but units already in flight now finish. A bad model spec costs at most one concurrency width of error nodes instead of exactly one — still bounded, against the 113,992 it produced before fail-fast existed. Covered by test.
Test plan
npm test— 429 passing. Newtests/component/unit-parallelism.test.tspins what could plausibly break under concurrency: one node per unit with no duplicates, idempotent re-runs, the configured limit respected, default-sequential behaviour unchanged, and early stop on a configuration fault.node --import tsx test/integration/test-commands.ts— 21 passing.npx tsc --noEmitclean.🤖 Generated with Claude Code