perf: concurrent render via a worker pool (remove process chdir)#5
Merged
Conversation
`veil render` now renders its paths through a bounded goroutine pool (--jobs, default NumCPU) instead of sequentially. Results are collected in fixed-index order so the response stays deterministic. The enabler is removing the process-global `os.Chdir(root)` the render pipeline used so hook `std`/`os` modules resolved paths against the project root. QuickJS already accepts the filesystem root per runtime (qjs.Option.CWD, mounted at "/"), so each hook now mounts the project root directly via the new hook.WithCwd option — nothing process-global is touched, which is what makes concurrent renders safe. render.Options loses the short-lived SkipChdir field; Render no longer chdir's at all. Verified: byte-identical output vs the chdir implementation, and 0 data races rendering 300 services at --jobs 16 under the race detector. Scale (300 services, 14 cores): 300 separate invocations 139s → batched --jobs 1 44s (3.2x, registry parsed once) → --jobs 14 ~9s (~15x) as the pool saturates the cores. Co-Authored-By: Claude Opus 4.8 (1M context) <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.
Summary
veil rendernow renders its resources through a bounded goroutine pool (--jobs, defaultruntime.NumCPU()) instead of one at a time, and drops the process-globalos.Chdirthat previously made concurrency unsafe.The chdir removal (the enabler)
The render pipeline used to
os.Chdir(projectRoot)before running hooks so QuickJS's built-instd/osmodules resolved paths (loadFile,readdir, …) against the project root —qjsmounts the process cwd at/when no root is given. Thatchdiris process-global, so it can't be shared safely across concurrent renders.QuickJS already accepts the filesystem root per runtime via
qjs.Option.CWD(mounted at/). So each hook runtime now mounts the project root directly through a newhook.WithCwd(root)option — the host process never changes its own working directory. The root is the directory housingveil.json/veil.yaml(a session constant), threaded fromrender.Options.Root.The pool
runRenderruns the paths through a bounded worker pool sized by--jobs(defaultNumCPU).Shared state is already concurrency-safe: the kind registry and resource catalog both cache via
sync.OnceValues, and each qjs runtime is independent.Verification
render.sh, no diff).--jobs 16undergo build -race(andgo test -race ./pkg/render ./pkg/commands).go vetgreen.Scale results (300 services, 14 cores)
veilinvocations (pre-batching)--jobs 1(registry parsed once)--jobs 4--jobs 8--jobs 14(= defaultNumCPU)Two compounding wins: batching one invocation (3.2×) and then the pool (~5× more), ~15× overall, tapering once CPU-bound on qjs hook execution past the core count.
🤖 Generated with Claude Code