fix(tooling): cap local lint concurrency at 4 threads - #423
Open
PeronGH wants to merge 1 commit into
Open
Conversation
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.
pnpm lintfreezes a 32-core workstation.--concurrency=autoresolves to 16 worker threads there, each of which builds its own full TypeScript program set, and the sum blows past the machine's memory before anything reports an error. This pins local lint to 4 threads.lint:ciis untouched — CI stays at--concurrency=offwith the single 6144 MB heap that CODE-468 landed.Why
autois wrong on a workstationcalculateWorkerCount(eslint/lib/eslint/eslint.js:410) derives the cap straight from the core count:getWorkerCountForisceil(files / 50)clamped to that cap. With ~1333 lintable files it'sceil(1333/50) = 27 -> 16: maximum fan-out, every run.Each worker calls
configLoader.loadConfigArrayForFileitself (worker.js:127), so each spins up its own typescript-eslint project service and its own TS programs. Nothing is shared between threads, and it all lands in one process's RSS. This repo's own CI comment (ci.yml:31) puts one such program set at over 4 GB — hence the 6144 MB heap there. Sixteen of them is not survivable on a 32 GB box.Two details make it worse than it looks:
calculateAutoWorkerCountsetscountAllMatched = !lintResultCache || cacheStrategy === "content"(eslint.js:355). Under--cache-strategy contentevery matched file counts toward the worker calculation even when its cached result is valid, so a fully warm run still starts 16 workers and 16 project services.autoscales the wrong variable. It tracks core count, which on CI runners is small and on workstations is exactly where memory pressure hurts most. The failure gets worse the better your machine is.Why 4
The worker path is either/or, not additive:
eslint.js:1048selectslintFilesWithMultithreadingorlintFilesWithoutMultithreading, so the main thread only coordinates and never holds a program of its own. Four workers means four program sets.Numeric concurrency also skips
calculateAutoWorkerCountentirely (eslint.js:424, justMath.min(4, filePaths.length)), which sidesteps thecache-strategy contentbehaviour above. And 4 clears ESLint's ownworkerCount <= 2threshold, so it doesn't emit the "just disable concurrency" advisory.Measurements (32-core / 32 GB WSL2 VM, 4 GB swap)
--max-old-space-size=5120Idle baseline is ~1.5 GB. Both cold runs exited 0 with 0 errors (375 pre-existing warnings).
Warm is the common case and 10 seconds is not a bottleneck. Cold peaks at 19.8 GB against a 32 GB ceiling — roughly 12 GB of headroom, and swap is never touched, which is what separates "slow" from "the desktop stops repainting".
No heap flag needed
Verified against the stock 4192 MB default with
NODE_OPTIONSunset: cold run completes, no OOM. Adding a heap flag would actively hurt, becauserunWorkerspasses noresourceLimits(eslint.js:470-477), so any heap setting is per worker and multiplies by 4 — 6144 would raise the worst case from ~17 GB to 24 GB, undoing the fix.Leaving the default also keeps a useful failure mode. If the repo grows past what one worker can hold, you get a clean
JavaScript heap out of memorynaming the thread instead of a frozen machine. The right answer that day is--concurrency=3, not a bigger heap.Trade-off
4 is a fixed number, so a low-core machine now oversubscribes slightly where
autowould have backed off. That's the cheap direction to be wrong in: 4 threads on a 4-core box is a scheduling inefficiency, whereas 16 threads on a 32-core box is an unrecoverable freeze. The memory ceiling is a property of the repo's TS program size, not of the host's core count, so a constant models it better thanautodoes.Verification
pnpm check:ci— format, lint, typecheck all pass.pnpm test— 2521 passed, 1 failed:packages/host/assets/tests/integration/registry-client.test.tsfails identically on unmodifiedmaster(a loopback-timeout issue on the test machine), unrelated to this change.Prior art: CODE-468 / #312 fixed the same class of OOM on the CI side and introduced the
lint:cisplit; this is the local-workstation half that was left onauto.