fix(perf): correct INLINE_BACKFILL_THRESHOLD docstring and raise threshold#1492
fix(perf): correct INLINE_BACKFILL_THRESHOLD docstring and raise threshold#1492carlos-alm wants to merge 2 commits into
Conversation
…ld for required-tier grammars The docstring claimed pool cost was "amortised over enough parse work" — measurements show IPC overhead scales linearly (~55–64ms/file pool vs ~8–10ms/file inline). The real motivation is crash safety for exotic WASM grammars (#965); JS/TS/TSX (required-tier, used in all this-dispatch backfill calls) have never triggered the V8 fatal crash class and are safe to run inline. Raise threshold 16 → 32 to keep typical this-dispatch batches (≤ 18 files on the codegraph corpus) on the inline fast path. Exotic-language drops are almost always well under 32 files and also benefit from the inline path without meaningful crash risk increase. Closes #1435
|
Claude finished @carlos-alm's task —— View job PR Review
Phase 1: Root Cause AnalysisProblem Being Solved: The Why This Approach: Two orthogonal fixes in one minimal change — (1) raise the threshold past the measured batch size of 18, and (2) correct the misleading docstring to accurately explain the crash-safety rationale (not cost amortisation). The threshold of 32 gives headroom over 18 while still routing truly large batches through the pool. Risk Assessment: Low. JS/TS/TSX are required-tier grammars with no history of the V8 fatal crash class. Raising the inline ceiling to 32 from 16 for files that call Phase 2: Configuration Change DetectionNo linting rules, test timeouts, or build configuration were changed. The only constant modified is Phase 3: Test IntegrityNo tests were modified. There are no direct unit tests for Minor observation: the threshold is a magic constant with no test asserting the inline path is taken for expected batch sizes. This isn't a blocker, but a test that exercises Phase 4: Code QualityDocstring correctness (
Threshold value ( Secondary docstring ( No bugs, security issues, or architectural concerns introduced. Phase 5: Backlog Compliance
Final Assessment |
Greptile SummaryThis PR corrects the
Confidence Score: 5/5Safe to merge — the change is a docstring correction and a threshold increase from 16 to 32, with no logic changes. The routing logic in No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "fix(perf): update stale parseFilesWasmFo..." | Re-trigger Greptile |
…nce threshold constant
The secondary docstring still described the old 16-value rationale
("engine-parity drop sizes"). Replace with a pointer to
INLINE_BACKFILL_THRESHOLD where the full rationale now lives.
|
Fixed — updated the stale |
The
INLINE_BACKFILL_THRESHOLDdocstring claimed pool cost was "amortised over enough parse work" — measurements show IPC overhead scales linearly (~55–64ms/file pool vs ~8–10ms/file inline). The real motivation is grammar-crash safety for exotic parsers (#965); JS/TS/TSX (required-tier, used in all this-dispatch backfill calls) have never triggered the V8 fatal crash class and are safe to run inline.Changes
Why 32
The primary hot caller (
runPostNativeThisDispatch) measured 18 JS/TS files at 55–64ms/file through the pool vs 8–10ms/file inline — a 3–6x penalty. Raising to 32 covers that batch with headroom. Anything larger (truly exotic-language drops at scale) still routes through the pool for crash isolation.Closes #1435