Skip to content

Add tak instruction-count benchmarks for CLI startup - #8237

Closed
isaacroldan wants to merge 1 commit into
mainfrom
river/tak-benchmarks
Closed

Add tak instruction-count benchmarks for CLI startup#8237
isaacroldan wants to merge 1 commit into
mainfrom
river/tak-benchmarks

Conversation

@isaacroldan

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Startup cost is the first thing a CLI user feels, and it is the thing this repo has the least visibility into over time. docs/cli/performance.md explains how to profile a single invocation, but nothing tells us that a dependency added last Tuesday made every invocation 4% slower.

The usual reason that gap stays open is that wall-clock benchmarking on a shared CI runner has roughly the same noise floor as the regressions worth catching, so any threshold either cries wolf or catches nothing. tak takes a different measurement: retired instruction counts, read out of valgrind's cachegrind, which are deterministic. The history lives in this repository as git notes under refs/notes/tak — no dashboard, no service, no account.

WHAT is this pull request doing?

  • tak.toml declares three benchmarks against the bundled CLI (pnpm nx bundle cli), which is what ships to npm: version (the startup control), help (manifest load and render), and app-dev-help (loads an actual command class). Each is commented with what it measures and why.
  • .github/actions/setup-tak installs valgrind and tak, pinned by version and sha256. The instrument is part of the experiment: changing tak can put a step in the series that looks like a change in the CLI.
  • .github/workflows/perf.yml measures the tip of every push to main and appends it to refs/notes/tak. It is the only thing that writes to the shared history.
  • .github/workflows/perf-pr.yml measures a PR, compares against the merge base, and posts a sticky comment. Measurement and reporting are split into two jobs so the write token is never in scope while PR code runs.
  • pnpm perf / pnpm perf:record so local and CI runs invoke the same command.
  • docs/cli/performance.md gains a section on running and extending it.

Two decisions worth reviewing

Every benchmark runs node --predictable, and that is load-bearing. V8 randomises its string hash seed per process, tiers functions up on a time-based budget, and marks the heap on background threads. Measured here on shopify --version:

run 1 run 2 run 3 spread
plain node 2,242,137,664 2,246,359,516 2,245,626,358 0.188%
node --predictable 2,344,654,834 2,344,659,033 2,344,651,185 0.00033%

570x tighter, which is the difference between a 1% gate that fires on noise and one that fires on a change. The tradeoff is that this is not the V8 configuration a user runs: the numbers are a relative signal, valid against themselves, and work V8 would do on a background thread is counted on the main thread. An optimization that moves work off the main thread is a real win that would read as flat here.

The PR gate is off (TAK_GATE: '0'). A gate needs a baseline series, and main has to accumulate one first; until then every comparison comes back empty, and an empty comparison is not a pass. The comment and job summary are posted from day one, so the numbers are visible immediately. Flip it to '1' once main has a few weeks of points and the series looks flat.

Hermeticity was verified, not assumed

Each benchmark was run inside a network namespace with no network at all and produced the same instruction count:

bench with network no network delta
version 1,543,545,676 1,543,559,947 0.0009%
help 1,606,628,841 1,606,569,920 0.0037%
app-dev-help 1,637,259,977 1,637,265,138 0.0003%

shopify commands was measured and rejected: 12,027,416,226 instructions with a network against 11,570,376,673 without, a 3.9% swing. It would have been the widest manifest walk available, but something on that path reaches the network.

One more thing the measurements turned up: bin/run.js calls module.enableCompileCache(), so the first invocation after a rebuild costs 2,499,297,579 instructions against 1,543,554,209 warm — a 62% difference. tak's default of three warmup runs covers it, but nothing should lower warmup in tak.toml.

Note

tak is explicitly pre-v1 and its author labels the docs as unreviewed. Its CLI, config format and storage format may change incompatibly. That is the main argument against adopting it now; the counter-argument is that everything it produces is JSON lines in a git ref this repo owns, so the data survives the tool.

How to test your changes?

Requires Linux and valgrind — instruction counting is unavailable on Apple Silicon and Windows.

sudo apt-get install -y valgrind
# install tak 0.0.5, pinned in .github/actions/setup-tak/action.yml
pnpm perf

Expect roughly:

  app-dev-help  node --predictable packages/cli/bin/run.js app dev --help
  instructions         1637126823
  help          node --predictable packages/cli/bin/run.js --help
  instructions         1606763914
  version       node --predictable packages/cli/bin/run.js --version
  instructions         1543544937

Absolute numbers will differ on your machine; the run-to-run spread should not exceed ~0.03%.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

Requested by Isaac Roldan isaac.roldan@shopify.com
Slack thread: https://shopify.slack.com/archives/C0AG0L37Q4C/p1785681072538939
AI Confidence Score: 85% — every claim in this description is a measurement taken in a sandbox with the CLI built from this commit, and pnpm perf was run end to end. The workflows themselves are unrun: they follow the patterns in tests-pr.yml and tak's own docs, and their YAML parses, but GitHub Actions is not exercised until this branch runs.

Wall-clock benchmarking on a shared CI runner has roughly the same noise
floor as the startup regressions the CLI accumulates, so a threshold
tight enough to catch one fires constantly. tak measures retired
instruction counts with cachegrind instead: on this repo they reproduce
to ~0.0003% run to run, and the history lives in git notes rather than
behind a service.

Adds tak.toml declaring three hermetic benchmarks against the bundled
CLI, a setup action that pins tak by version and sha256, a workflow
recording every main commit into refs/notes/tak, and a PR workflow that
compares against the merge base and posts the numbers. The PR gate is
off until main has a baseline series to compare against.

Every benchmark runs node with --predictable; without it V8's hash seed
randomisation, time-based tier-up and background marking move the count
by ~0.19%, which leaves too little headroom under a 1% gate.

Co-authored-by: Isaac Roldan <isaac.roldan@shopify.com>
@isaacroldan isaacroldan self-assigned this Aug 3, 2026
@github-actions github-actions Bot added cla-needed no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Instruction counts

The comparison never ran — an earlier step failed.

7c8ccbc18fd7 vs 593ee5d74f44 · measured on the runner, not pushed to the history.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action.
→ If there's no activity within a week, then a bot will automatically close this.
Thanks for helping to improve Shopify's dev tooling and experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-needed no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. no-pr-activity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants