TEST: benchmark job on the ARM runner - #2
Closed
suthat wants to merge 6 commits into
Closed
Conversation
The benchmark job measured the PR and main once each and reported the difference to three decimal places, which reads as a precise result but is not one. Across the 41 PRs that carry a bot comment, the reported delta ranges from -13.23% to +13.39%, and PRs that only touch docs, the README, the logo or CI config still move it by as much as 3.80%. Two things make the measurement worse than it needs to be. A single CoreMark run scores itself from wall-clock time over roughly fifteen seconds on a shared runner, so one sample carries several percent of noise on its own. On top of that main was always measured second: the mean delta over those 41 PRs is -1.13% and 27 of them are negative, which a sign test puts at p = 0.03 against an even split. Build both bench binaries up front, stage each one next to the wasm it loads, then run them alternately and report the median of five samples per side. Swapping which side goes first on every other sample keeps the ordering from favoring either one. The comment now carries the observed sample range and marks a delta that falls inside it as noise. This does not make scores comparable between runs and cannot: baseline scores for main span 230 to 498 depending on which CPU model the runner lands on. Co-authored-by: Cursor <cursoragent@cursor.com>
A CoreMark score mostly measures how fast the runner is. What a PR can change is how much work the interpreter does, and instructions retired measures that directly: it does not move with the CPU model the runner landed on or with whoever else is on the machine, so one run per side resolves a difference that no number of timed samples can. Hardware counters are not an option here. GitHub runners are Azure VMs whose hypervisor does not expose the PMU, so perf reports cycles, instructions and branches as unsupported, and reading a counter through ptrace is out for the same reason. Count under callgrind instead, which is pure user-space instrumentation and needs no counters at all. Counting first needs a workload that does not depend on the clock. CoreMark sizes itself by timing ten iterations, multiplying by ten until that takes at least a second, then settling on iterations * (1 + 10 / floor(seconds)). The divisor is an integer, so a calibration round of 1.9s and one of 2.1s differ by nearly 2x in the work that follows. Counting that without pinning it would be worse than timing it. COREMARK_FIXED_CLOCK feeds the module a fixed table of timestamps instead, which holds it at 110 iterations with an eleven second measured window: still a valid CoreMark run, and one that scores exactly 10.0 every time. The bench asserts that score, so a module that starts timing itself differently fails the job rather than producing two counts that describe different work. The score stays, at three samples per side rather than five. It is still worth reporting what the runner managed, but it is no longer the number a regression has to be read out of. This also increments CLOCK_CALL_COUNT, which nothing ever incremented before, since the fixed clock indexes its table by it. Co-authored-by: Cursor <cursoragent@cursor.com>
A bench binary built before COREMARK_FIXED_CLOCK existed ignores it and sizes itself from the clock, so counting it against one that pins its workload compares two different amounts of work. That is the situation for this PR's own baseline, and for any branch that predates it, so the step probes each side first: a pinned run scores exactly 10.000 and costs a fraction of a second, which is a cheaper way to find out than the callgrind run it would otherwise spoil. Skip the counts with a warning in that case rather than fail, leaving the timed comparison to stand on its own as it did before. Co-authored-by: Cursor <cursoragent@cursor.com>
Building main against this branch's harness is what makes the two instruction counts comparable, but it can fail on its own: a harness written against an interface main does not have yet will not compile there, and the first PR to change the interpreter and the bench together would fail a job it had nothing to do with. A baseline that cannot be fetched, built or pinned warns and skips the counts, leaving the timed comparison in place. A pr side that cannot pin is this branch's own doing and still fails. Also keeps the flag on an instruction difference over 1%, reworded. The count is exact, so the figure is a reading aid rather than a threshold, but a large difference is worth putting in front of a reviewer rather than leaving it to be picked out of the numbers. Co-authored-by: arthurianresolve <268402532+arthurianresolve@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Welcome, new contributor! Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly |
CoreMark Benchmark ResultsCurrent Score: 280.741 |
Code Coverage ReportCurrent Coverage: 95.29% |
Owner
Author
|
Throwaway validation PR; the runner data it produced is what matters. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Throwaway PR on my own fork, to exercise the reworked benchmark job end to end on a real runner.
Made with Cursor