Skip to content

perf: invoke node directly from the js_binary launcher - #2972

Merged
acozzette merged 1 commit into
mainfrom
remove-extra-bash-process
Aug 14, 2026
Merged

perf: invoke node directly from the js_binary launcher#2972
acozzette merged 1 commit into
mainfrom
remove-extra-bash-process

Conversation

@acozzette

Copy link
Copy Markdown
Contributor

The launcher ran the program through our Node wrapper script, which does nothing but pass its arguments along to Node and add a --require flag for our fs patches.

Let's just inline that into our main launcher script so that we avoid spawning another bash process unnecessarily.

We still need to keep the Node wrapper around, though, so that if we ever shell out to node then we get our wrapper with its patches.


Changes are visible to end-users: no

Test plan

  • Covered by existing test cases

@acozzette
acozzette marked this pull request as ready for review August 14, 2026 02:17
@acozzette
acozzette requested a review from jbedard August 14, 2026 02:17
Comment thread js/private/js_binary.sh.tpl
@jbedard

jbedard commented Aug 14, 2026

Copy link
Copy Markdown
Member

Make sure we have tests covering this scenario 👍

The launcher ran the program through our Node wrapper script, which does
nothing but pass its arguments along to Node and add a `--require` flag for our
fs patches.

Let's just inline that into our main launcher script so that we avoid spawning
another bash process unnecessarily.

We still need to keep the Node wrapper around, though, so that if we ever shell
out to `node` then we get our wrapper with its patches.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@acozzette
acozzette force-pushed the remove-extra-bash-process branch 2 times, most recently from 14d4bc0 to a5ae0be Compare August 14, 2026 16:23
@acozzette
acozzette merged commit bd8631f into main Aug 14, 2026
368 of 371 checks passed
@acozzette
acozzette deleted the remove-extra-bash-process branch August 14, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants