Repository navigation
perf(dev_server): emit each npm binding set once in the generated config - #263
Merged
Merged
Conversation
A large application's files share a few npm binding sets, but the generated dev-server config inlined a copy per file, which grew past what oj evaluates in 60 s. The config now emits each distinct set once, checks each (binding set, directory) pair once, and tests existence before realpath instead of throwing on missing paths. Co-Authored-By: Claude Opus 5.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
ts_dev_serverwrites a generated config that oj evaluates at startup. For each declared file, the config inlined that file's full npm binding set (package name → member). A large application's files share only a couple of distinct binding sets, but each file carried its own copy. In one application with 12,459 declared files the generated config reached 52.6 MB, and 48.9 MB of that was thenpmContextstable. oj kills a config evaluation that takes longer than 60 s, so the dev server never started (evaluation took over 200 s).The evaluation also called
realpathon paths that did not exist and relied on the thrown error. Each throw inside a multi-megabyte module costs milliseconds.What changes
ts/private/ts_dev_server.bzlemits each distinct binding set once (npmBindingSets); each file entry refers to its set by index.node_modulesexistence per directory.realpathtestsexistsSyncbefore resolving instead of catching a throw. A permission or symlink-loop error now reads as "not found" where it used to throw.Verification
//tests/dev_server/...,//tests/codegen_tree/...and//tests/vitest/...pass on remote execution (BuildBuddy invocationfa867a77).Risks
existsSyncguard changes the failure mode for unreadable paths, as noted above.🤖 Generated with Claude Code