Skip to content

fix(cli): run mops bench cells with the incremental GC paused - #890

Merged
Kamirus merged 3 commits into
mainfrom
kamil-claude/bench-gc-pause
Oct 12, 2026
Merged

Kamirus merged 3 commits into
mainfrom
kamil-claude/bench-gc-pause

Conversation

@Kamirus

@Kamirus Kamirus commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Supersedes #889 by @timohanke, whose branch on an organization-owned fork doesn't accept maintainer pushes. The first commit is #889 unchanged, so the squash keeps @timohanke as co-author. Fixes #888.

Problem

mops bench instruction counts depended on the order of the cells. The bench canister was compiled with --force-gc, so the incremental GC started a new cycle at the end of every message, and on a large heap that cycle ran on into the next cell. While a cycle is in progress, the GC's write and allocation barriers are on, and the cell pays for them: identical cells differed by 20–40%. #888 has the compiler details.

Dropping --force-gc isn't enough on its own. A cell that leaves enough garbage still starts a cycle, and the next cell runs mid-cycle. In the fixture below, that cell costs 16,100,150 instructions instead of 12,500,150.

Fix

  • The incremental GC is no longer compiled with --force-gc. The legacy GCs of moc 1.x keep it, because they finish a collection within the message.
  • Before each cell, mops bench calls a new collectorInstructions() method on the bench canister until two calls in a row return the same value. rts_collector_instructions is the GC cost at the end of the previous message, not a running total: about 330 instructions while the GC is paused, and hundreds of millions per increment while a cycle runs. After 1000 messages without a pause, mops bench fails.

Before / after

cli/tests/bench-gc-barrier, moc 2.0.0: identical fill cells, each after a garbage cell, on a 1M-object live heap.

Before (3.5.0):

|    |       fill |     garbage |
| :- | ---------: | ----------: |
| a  | 17_300_150 |  75_001_653 |
| b  | 16_100_150 | 158_552_560 |
| c  | 12_500_150 |  76_051_963 |
| d  | 16_100_150 | 157_503_563 |

After:

|    |       fill |    garbage |
| :- | ---------: | ---------: |
| a  | 12_500_150 | 75_003_204 |
| b  | 12_500_150 | 75_001_630 |
| c  | 12_500_150 | 75_001_681 |
| d  | 12_500_150 | 75_001_428 |

On a real benchmark, research-ag/sort, 23 of 36 cells were inflated by 23–40% on 3.5.0. With this fix, it gives the same counts in both cell orders.

Migration

Baselines saved with 3.5.0 can show lower instruction counts for cells that used to start mid-cycle. To compare against fair numbers, re-save them with mops bench --save. Saved and published results now record forceGc: false for the incremental GC.

What is unchanged

  • The Heap table still shows exactly what a cell allocated, and the GC still never runs inside the measured window.
  • --gc copying|compacting|generational on moc 1.x still compiles with --force-gc and skips the wait.

Follow-up worth flagging

If two GC increments in a row cost exactly the same number of instructions, the wait would end mid-cycle. That's unlikely, and the Motoko runtime exposes no GC phase to check instead.

🤖 Generated with Claude Code

timohanke and others added 2 commits October 12, 2026 14:36
An incremental GC cycle can span several messages. A cell that starts
while one is in progress pays for the GC's write and allocation barriers,
so identical cells cost up to 40% more instructions depending on what ran
before them. --force-gc made this happen in about every other cell, since
it starts a new cycle after every message.

Compile the bench canister without --force-gc for the incremental GC, and
before each cell call a new collectorInstructions() method until the
collector's instruction count stops changing. The legacy GCs keep
--force-gc.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without `--force-gc`, the bench-gc-barrier fixture never started a GC cycle,
so the wait loop always returned after two calls and the test never ran it.
Its cells now alternate between identical writes and cells that leave
garbage. Without the wait, the write cell after the first garbage cell costs
16,100,150 instructions instead of 12,500,150.

`rts_collector_instructions` is the GC cost of the previous message, not a
running total, so the comments now say that.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Kamirus
Kamirus requested review from a team and automation-sa-sre as code owners October 12, 2026 13:36
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Cursor AI review

👍 APPROVE — looks safe to merge

Category Assessment Details
Summary ✅ mops bench stops compiling the incremental GC with --force-gc and waits until rts_collector_instructions is stable before each cell, so identical cells no longer pay mid-cycle write/allocation barriers.
Code Quality ✅ Reuses isLegacyGc, cliError, and the existing bench wrapper; waitForGcPause is a small local helper with no speculative API.
Consistency ✅ Changelog ## Next, docs/docs/cli/4-dev/02-mops-bench.md, and cli/declarations/bench/* all match. No new flag, so --help and sibling commands stay consistent; --force-gc remains commented out in cli/cli.ts.
Security ✅ collectorInstructions is a local PocketIC harness method, not the registry. Traced publish.ts bench("", { silent: true }) → uploadBenchmarks; forceGC is existing metadata. Frontend PackageBenchmarks.svelte does not display forceGC. No auth/integrity path changed.
Tests ✅ No snapshot hunks. New cli/tests/bench.test.ts property test plus bench-gc-barrier fixture asserts four identical fill cells and forceGc: false. Existing EOP (bench/, moc 1.16.1) and allocation (bench-allocation) tests still cover compile/run and heap isolation.
Maintainability ✅ Comments in bench.ts and bench-canister.mo state why --force-gc is skipped and what equal collector readings mean. Fail-loud after 1000 messages via cliError (throws; bench() finally still stops PocketIC).

Verdict

Decision: APPROVE
Risk: Low
Reason: Contained, tested bugfix in the local bench harness — not registry, install, auth, or release. Saved and published instruction counts will drop versus 3.5.0 mid-cycle cells, which the changelog and docs already describe.


Generated for commit 55dfced

@automation-sa-sre automation-sa-sre left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated approval: the AI review verdict for c35ee91 is APPROVE. See the "Cursor AI review" comment for details.

@Kamirus
Kamirus enabled auto-merge (squash) October 12, 2026 13:40
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@automation-sa-sre automation-sa-sre left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated approval: the AI review verdict for 55dfced is APPROVE. See the "Cursor AI review" comment for details.

@Kamirus
Kamirus merged commit 9d5814a into main Oct 12, 2026
22 checks passed
@Kamirus
Kamirus deleted the kamil-claude/bench-gc-pause branch October 12, 2026 14:12
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.

mops bench: instruction counts depend on cell order (incremental GC barriers active when a cycle spans messages)

3 participants