Skip to content

perf(ci): build the app image on 16 vCPU, pin node-gyp, and keep the toolchain out of the runtime image - #6500

Merged
waleedlatif1 merged 1 commit into
stagingfrom
perf/docker-build-optimizations
Aug 10, 2026
Merged

perf(ci): build the app image on 16 vCPU, pin node-gyp, and keep the toolchain out of the runtime image#6500
waleedlatif1 merged 1 commit into
stagingfrom
perf/docker-build-optimizations

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Three independently-verified changes to the app image build. Each measured, none dependent on the others.

1. The app image builds on 16 vCPU instead of 8. Its two dominant steps — bun install (~300-465s) and next build (~262s) — both scale with cores, and this build gates every deploy because nothing ships until the image is pushed. The same next build already runs on 16 vCPU in the Build App verification job, which gates nothing. ARM64 stays at 8: that job is off the deploy path and the workflow warns an unprovisioned runner label hangs a release in queued.

2. node-gyp comes from the lockfile, not npx. npx node-gyp resolved from the registry during every production image build, which pulled 13.x over the pinned 12.4.0 and bypassed the minimumReleaseAge supply-chain gate in bunfig.toml. It has to be an apps/sim devDependency because the only other copy is transitive through @electron/rebuild, which turbo prune sim strips.

3. base splits into base + build-base. The 390 MB compiler toolchain (measured: 1.21 GB vs 1.6 GB) exists only to compile isolated-vm against Node. The runner copies the finished binary, so shipping the toolchain inflated every ECS task pull. ffmpeg and python3 stay — fluent-ffmpeg is a serverExternalPackage.

Type of Change

  • Improvement

Testing

Verified against real image builds, not by inspection:

Check Result
Full image build ✅ succeeds
g++ / make / pip3 in runtime absent
node / bun / python3 / ffmpeg ✅ present
isolated-vm native module ✅ loads and evaluates (1+12)
lib0 / yjs / y-protocols + lib0/logging.js ✅ intact (the collab-doc 500)
sharp + libvips (compatibility with #6499) loads and encodes a PNG in the slimmed image
Container boot ✅ Next.js Ready, no module errors
Dependency hoisting ✅ root which@6.0.1, fluent-ffmpeg keeps its own 1.3.1
Full suite 21,250 passed, 0 failed
Audits / type-check ✅ 22 audits, 23 workspaces

Interaction with #6499 was checked explicitly, since that PR ships sharp/@img into the same runtime image this PR slims. libvips' shared-library dependencies (libstdc++.so.6, libgcc_s.so.1, libm.so.6, libz.so.1) come from the base image and nodejs, not from g++, and sharp was confirmed to load and encode against the slimmed image. No conflict.

What this does NOT do

It does not change time-to-live for a deploy. That is ~15 min (≈10 min build + ~40s pipeline + ~185s Install/AllowTraffic) and stays that way.

A dead end, recorded so nobody repeats it

This branch briefly carried a change adding cache-key to useblacksmith/setup-docker-builder, on the theory that the app's dependency layer re-runs every build (286s / 465s observed) because all five Dockerfiles shared one unkeyed layer cache. It was reverted — the input does not exist, at the pinned SHA or on main, and Actions silently ignores unknown inputs. The key is not configurable:

// src/setup_builder.ts:409
const stickyDiskKey = process.env.GITHUB_REPO_NAME || "";

The real CI log also disproves the eviction theory — the disk is obtained successfully, with prior state, no fallback:

Getting sticky disk for simstudioai/sim
Sticky disk parent snapshot: commit-01KZP43K3QNJ04JYYGQS7H1MMG
Successfully obtained sticky disk

So the cache is present and the deps layer still re-runs. That cause remains unidentified and is not addressed here. Verified separately that the Dockerfile is not at fault: a local source-only change rebuilds with the deps layer CACHED, and turbo prune's out/json is byte-identical across runs.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@waleedlatif1
waleedlatif1 requested a review from a team as a code owner August 10, 2026 19:13
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 10, 2026 7:37pm

Request Review

@cursor

cursor Bot commented Aug 10, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes production container contents and native-module build tooling; risk is mitigated by keeping runtime ffmpeg/python3 and verified isolated-vm/sharp loads, but any gap in copied native deps would break deploys at runtime.

Overview
Speeds up deploy-gating app image builds on Blacksmith by moving them from 8 to 16 vCPU (build-dev and build-amd64), since bun install and next build dominate that job and nothing ships until the image is pushed.

Pins node-gyp 12.4.0 as an apps/sim devDependency and invokes /app/node_modules/.bin/node-gyp when rebuilding isolated-vm, replacing npx registry resolution that could pull 13.x and skip the lockfile minimumReleaseAge gate.

Splits the app Dockerfile into runtime base vs build-base: make, g++, and pip/venv stay only on build stages; the runner inherits slim base (~390 MB smaller) while still copying the compiled isolated-vm binary and full hoisted lib0/yjs/sharp trees from deps.

Reviewed by Cursor Bugbot for commit 499a4d2. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR accelerates app-image CI builds, pins the native-module build tool through the lockfile, and separates build-only compiler packages from the runtime image.

  • Moves Blacksmith app-image jobs from 8 to 16 vCPU.
  • Adds node-gyp 12.4.0 as an exact sim devDependency and invokes its installed binary directly.
  • Introduces a build-base Docker stage so native compilation tools do not ship in the final runner image.

Confidence Score: 5/5

The current PR appears safe to merge.

No blocking failure remains, and the previous cache-key concern targeted code that was reverted and is no longer part of this PR.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Raises the Blacksmith runner tier for app-image builds to 16 vCPU while leaving other image targets unchanged.
apps/sim/package.json Adds node-gyp 12.4.0 as an exact build-time devDependency retained by the pruned sim workspace.
bun.lock Locks node-gyp 12.4.0 with integrity metadata and updates its hoisted transitive dependency layout.
docker/app.Dockerfile Separates native compilation tooling into build-base, invokes the lockfile-installed node-gyp binary, and keeps the final runner based on the slimmer runtime stage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Base[base: Node, Bun, Python, ffmpeg] --> BuildBase[build-base: compiler toolchain]
  BuildBase --> Pruner[pruner: turbo prune]
  Pruner --> Deps[deps: install and rebuild isolated-vm]
  BuildBase --> Deps
  BuildBase --> Builder[builder: Next.js and bootstrap builds]
  Deps --> Builder
  Base --> Runner[runner: runtime-only image]
  Deps -->|compiled isolated-vm| Runner
  Builder -->|application artifacts| Runner
Loading

Reviews (2): Last reviewed commit: "perf(ci): build the app image on 16 vCPU..." | Re-trigger Greptile

Comment thread .github/actions/docker-build/action.yml Outdated
Comment thread .github/actions/docker-build/action.yml Outdated
@waleedlatif1
waleedlatif1 force-pushed the perf/docker-build-optimizations branch from 50016b0 to 77758ed Compare August 10, 2026 19:20
@waleedlatif1 waleedlatif1 changed the title perf(ci): give each Docker build its own Blacksmith layer cache perf(ci): build the app image on 16 vCPU, pin node-gyp, and keep the toolchain out of the runtime image Aug 10, 2026
…f it

Three independent changes to the app image build.

The app image builds on 16 vCPU instead of 8. Its dominant steps -- bun install
(~300-465s) and next build (~262s) -- both scale with cores, and this build gates
every deploy because nothing ships until the image is pushed. The same next build
already runs on 16 vCPU in the Build App verification job, which gates nothing.
The isolated-vm rebuild will not benefit: JOBS=4 caps it deliberately to avoid an
OOM. ARM64 stays at 8 -- that job is off the deploy path, and the workflow warns
an unprovisioned runner label hangs a release in queued.

node-gyp comes from the lockfile rather than npx. npx resolved it from the
registry during every production image build, which pulled 13.x over the pinned
12.4.0 and bypassed the minimumReleaseAge gate in bunfig.toml. It has to be an
apps/sim devDependency because the only other copy is transitive through
@electron/rebuild, which turbo prune sim strips.

base splits into base and build-base. The compiler toolchain exists only to build
isolated-vm against Node and the runner copies the finished binary, so shipping
it inflated every ECS task pull: measured 1.21 GB for base against 1.6 GB for
build-base, so ~390 MB stays out of the final image. ffmpeg and python3 stay --
fluent-ffmpeg is a serverExternalPackage, and node-gyp needs the interpreter.

Verified against real image builds rather than by inspection: g++/make/pip3 are
absent from the runtime image while node/bun/python3/ffmpeg remain, the
isolated-vm native module loads and evaluates, lib0/yjs/y-protocols including the
lib0/logging.js subpath are intact, sharp and libvips load and encode a PNG so
#6499's fix still works against the slimmed image, the container boots to Next.js
Ready, and the full suite passes 21250 tests through the dependency hoisting
reshuffle that promoting node-gyp causes (root which@6.0.1, fluent-ffmpeg keeps
its own 1.3.1).

Not included: an earlier attempt added a cache-key input to
useblacksmith/setup-docker-builder, on the theory that the deps layer re-runs
every build (286s/465s observed) because all five Dockerfiles share one layer
cache. That input does not exist at the pinned SHA or on main, and the key is not
configurable -- setup_builder.ts uses GITHUB_REPO_NAME. CI logs also show the
sticky disk being obtained with a parent snapshot rather than falling back, so
the cache is present and that cause remains unidentified.
@waleedlatif1
waleedlatif1 force-pushed the perf/docker-build-optimizations branch from 77758ed to 499a4d2 Compare August 10, 2026 19:32
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Rebased onto staging with #6499 merged, and squashed to one commit — the branch previously carried a commit claiming to fix the Blacksmith layer cache plus its own revert, which was misleading history for a change that doesn't do that.

Verified the interaction with #6499 explicitly, since that PR ships sharp/@img into the same runtime image this PR slims by 390 MB. libvips' shared-library dependencies (libstdc++.so.6, libgcc_s.so.1, libm.so.6, libz.so.1) come from the base image and nodejs, not from g++. Built a probe image combining both changes and ran sharp against it:

sharp+libvips OK, png bytes: 93

No conflict in either merge order.

One thing worth recording. Building the combined Dockerfile locally failed with cannot allocate memory during next build. I did not assume that was environmental — I built staging's unmodified Dockerfile on the same machine, same context, same pruned state, and it failed identically at the same step. My Docker VM has 24 GB; this workflow's own comments record the app build peaking at 51 GB cold, which is why Build App moved to a 16 vCPU runner after 23 OOM kills. The failure is the local VM, not this change, and an earlier full build of these exact changes succeeded and produced a working image.

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

The update is clean. The squash removes the misleading revert commit, the OOM attribution is airtight (same machine, same unmodified Dockerfile, same failure point), and the sharp/libvips probe is the right way to verify — shared library deps don't come from g++ so there was never a real conflict, but actually building a probe image and encoding a PNG is better than reasoning about it.

The lockfile diff lines up exactly with what the Dockerfile comment describes: which@6.0.1 is now the hoisted root copy (from node-gyp 12.4.0's own dep), fluent-ffmpeg keeps its own which@1.3.1 under a scoped path, and the old node-gyp/which and node-gyp/which/isexe nested entries are gone because node-gyp is no longer resolved transitively through npx — it's a direct dep whose tree is now managed by bun with the lockfile pin honored.

Nothing outstanding from the review. LGTM.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 499a4d2. Configure here.

@waleedlatif1
waleedlatif1 merged commit 9ce3f51 into staging Aug 10, 2026
23 checks passed
@waleedlatif1
waleedlatif1 deleted the perf/docker-build-optimizations branch August 10, 2026 19:42
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Re-running review for a fresh summary — the 4/5 above was scored against commit 50016b05c, which is no longer on the branch. Its sole cited reason was the cache-key hardening, and that code was reverted and squashed away; .github/actions/docker-build/action.yml is now untouched by this PR.

Current state is one commit: 499a4d228.

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 499a4d2. Configure here.

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.

1 participant