perf: strip debug symbols from Node binaries (-17 MiB)#1019
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c35452e. Configure here.
| // strip removes debug symbols and unneeded sections (~17 MiB on linux-x64). | ||
| // Must run BEFORE fossilize's SEA injection — postject corrupts ELF section | ||
| // layout, making strip fail on the injected binary. | ||
| await stripCachedNodeBinaries(platforms); |
There was a problem hiding this comment.
Strip runs before cache exists
Medium Severity
stripCachedNodeBinaries runs immediately before fossilize, but fossilize fills .node-cache during that invocation. When the cache is missing (typical CI fresh checkouts), stripping is skipped and the SEA build uses an unstripped Node base, so the advertised ~17 MiB savings may not apply to release artifacts.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c35452e. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| // strip removes debug symbols and unneeded sections (~17 MiB on linux-x64). | ||
| // Must run BEFORE fossilize's SEA injection — postject corrupts ELF section | ||
| // layout, making strip fail on the injected binary. | ||
| await stripCachedNodeBinaries(platforms); |
There was a problem hiding this comment.
Bug: The stripCachedNodeBinaries function is called before fossilize populates the .node-cache/ directory, causing the stripping step to be skipped in CI and fresh builds.
Severity: MEDIUM
Suggested Fix
Modify the build script to ensure fossilize downloads the Node binaries before stripCachedNodeBinaries is called. This could involve running a pre-download step or restructuring the compileAllTargets function to execute stripping after the cache is populated. Alternatively, cache the .node-cache directory in the CI workflow.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: script/build.ts#L369
Potential issue: The function `stripCachedNodeBinaries` is invoked at
`script/build.ts:369` before the `fossilize` process runs. On a clean environment like a
CI runner, the `.node-cache/` directory, which `fossilize` is responsible for creating
and populating, does not yet exist. The `stripCachedNodeBinaries` function checks for
this directory's existence and exits early if it's not found. As a result, the Node
binaries are downloaded and used for SEA injection by `fossilize` without ever being
stripped of their symbols. This means the intended 17 MiB size reduction for release
artifacts is not achieved in any CI-produced build.
Did we get this right? 👍 / 👎 to inform future reviews.
Strip cached Node binaries in fossilize's .node-cache/ before SEA injection. Node.js ships with full symbol tables (~17 MiB on linux-x64) that aren't needed at runtime. Stripping must happen BEFORE fossilize's postject injection because postject corrupts the ELF section-to-segment layout, making GNU strip fail on the injected binary. Results (linux-x64): Raw binary: 125 MiB → 108 MiB (-14%) Compressed: 34 MiB → 30 MiB (-12%) Startup: unchanged (strip removes metadata, not code)
BYK
force-pushed
the
chore/node24-optimizations
branch
from
c35452e to
572973e
Compare
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 4283 uncovered lines. Generated by Codecov Action |
1 participant
Summary
Strip debug symbols and unneeded sections from cached Node binaries before fossilize's SEA injection. Node.js ships with full symbol tables (~17 MiB on linux-x64) that aren't needed at runtime.
Why before injection?
Stripping must happen before SEA injection because postject corrupts the ELF section-to-segment layout, making GNU
stripfail on the injected binary. The build pipeline is now:Results (linux-x64)
Comparison vs Bun (v0.34.0)
Platform handling
strip --strip-unneeded— removes symbols + debug sectionsstrip -x— removes local symbols (requires re-codesigning, handled by fossilize)