chore(build): spawn watcher tools directly instead of via npx#41034
Open
Skn0tt wants to merge 1 commit into
Open
chore(build): spawn watcher tools directly instead of via npx#41034Skn0tt wants to merge 1 commit into
Skn0tt wants to merge 1 commit into
Conversation
Skn0tt
changed the title
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Member
Are you sure we care about npx wrappers? |
The watch script orchestrates 7 vite watchers + 1 tsc, each spawned via
`spawn('npx', [...], { shell: true })`. That produces an extra
`sh -c 'npx <tool>'` + `npm exec` wrapper per tool, just to launch
another Node.js process.
Resolve the bin path via `<pkg>/package.json` and spawn `node <bin>`
directly with `shell: false`.
Per `npm run watch` (measured on macOS, fresh start, steady state):
- Processes: 26 → 17 (−9)
- ps RSS sum: 181 MB → 130 MB (−51 MB)
- Per wrapper `vmmap` physical_footprint: ~36 MB (8 wrappers × 36 MB
≈ ~290 MB of attributable memory eliminated; most was sitting in the
macOS compressor for long-running watches)
Behaviorally identical: same args reach vite/tsc.
Skn0tt
force-pushed
the
reduce-watch-memory
branch
from
d441b87 to
0888d71
Compare
Skn0tt
changed the title
Member
Author
|
Looks like my Copilot only gave me a slice of the real picture ;D I think we should take 545mb when we can get it for free. I tried replacing tsc with tsgo, but it only helps speed and not memory. #41047 has some consolidation on the Vite processes - maybe we can get it down to just one. |
Contributor
Test results for "MCP"1 failed 7206 passed, 1113 skipped Merge workflow run. |
Contributor
Test results for "tests 1"2 flaky43984 passed, 864 skipped Merge workflow run. |
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
2 participants
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.
Summary
Small cleanup. The watch script spawns 8 long-running tools (7 vite watchers + 1 tsc) through
npx, which forks an extrash -c+npm execwrapper process per tool just to launch another Node.js process. Resolving the bin paths up-front lets usspawn(node, [bin, ...], { shell: false })directly.Behaviorally identical (same args reach vite/tsc); also drops the
quotePathhelper that only existed to escapetsc -pfor the shell.Impact
Removes 8 wrapper processes per watch. Their memory cost varies wildly by OS: ~5 MB ps-RSS each on macOS (heavily compressed for long-running watches; ~36 MB physical footprint per
vmmap) up to ~78 MB ps-RSS each on Linux. So somewhere between ~40 MB and ~550 MB freed per watch depending on the platform — modest relative to the watch's total footprint (dominated bytsc -wand the vite workers), but free.