Skip to content

perf: pin the recorder's inactive check and the register-operand reader inline - #175

Merged
benletchford merged 2 commits into
benletchford:masterfrom
rlanday:perf/recorder-inline-pins
Sep 5, 2026
Merged

benletchford merged 2 commits into
benletchford:masterfrom
rlanday:perf/recorder-inline-pins

Conversation

@rlanday

@rlanday rlanday commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Two inlining pins on the per-instruction fast path, so its cost no longer depends on where fat LTO's inliner happens to draw the line for a given binary size:

  1. record_executed and stop_recording run for every interpreted instruction with the "is a recording active" test inside them and no inline attribute. Whether that test is inlined is decided by the size of the recording body: one binary in our measurements inlined it, a larger one did not, and every guest instruction then paid an out-of-line call to test a flag (3 in Three +2.8%, Lemmings +7.3%, System's Twilight +1.0% host instructions at an identical trace census). Each is split into an always-inline flag check and a never-inline body.
  2. read_direct_reg, the masked register load on the decoded-op fast paths, is pinned inline for the same reason; in a binary where it had dropped out of line it showed 1.7% self time on a JIT-heavy workload.

Rebased onto current master after #173 merged. This PR contains only the two inlining changes and does not depend on #174; the outlined recorder helpers use the existing per-operation TLS accessor. The measurements below were taken against the original #174 baseline before this rebase.

Measurements

Paired hardware counters, headless replays in Systemless, both arms interleaved, identical guest instructions and ticks (and framebuffer hash except Escape Velocity, whose replay is not tick-deterministic). Baseline is the #174 head built the same way; medians of the pair ratios for host instructions retired:

workload recorder pin alone both pins
Lemmings, 1 B −1.01% (6/6) −0.95% (6/6)
SimCity 2000, 400 M −0.23% (6/6) −0.08% (5/6)
3 in Three, 200 M −0.16% (5/5) −0.15% (5/5)
System's Twilight, 100 M −0.12% (6/6) −0.12% (6/6)
Escape Velocity, 100 M 0.00% (3/6) 0.00% (3/6)

CPU time, re-measured with the machine idle (both pins vs the same baseline, 5 pairs, user seconds):

workload instructions CPU time
Lemmings, 1 B −0.9% (5/5) +2.1% (1/5; cycles +3.3%, 2/5)
SimCity 2000, 400 M −0.2% (5/5) +1.2% (1/5; cycles +1.1%, 1/5)

So in this configuration the pins lower the instruction count slightly and leave CPU time within the ±2–4% layout swing a rebuild of this crate produces at identical instructions; there is no CPU-time gain to claim here. The case for them is the other binary, where the inliner's decision cost 2.8–7.3% instructions on the same code.

On the baseline binary the inliner already had both paths inline, so these are the small direct gains; the point of the pins is the regressions above, which appeared in a binary that differed only by an unrelated, larger recorder. Cycles moved within the layout noise of a rebuild (a no-op perturbation of this crate moves cycles ±2–4% at identical instructions), so instructions are the numbers to trust.

Tests

cargo test and cargo clippy --all-targets on the branch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UqCuD9vsGeeij5DdYt3vcK

rlanday and others added 2 commits September 5, 2026 14:06
record_executed and stop_recording are called for every interpreted
instruction with the 'is a recording active' test inside them and no
inline attribute. Whether LTO inlines that test is decided by the size of
the recording body: the entry-reset binary happened to inline it, the
dispatch-tree binary (larger recorder) did not, and every guest instruction
then paid an out-of-line call to test a flag: 3 in Three +2.8%, Lemmings
+7.3%, System's Twilight +1.0% host instructions at an identical trace
census. Split each into an always-inline flag check and a never-inline body
so the fast path no longer depends on the inliner's threshold.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UqCuD9vsGeeij5DdYt3vcK
…-op path

read_direct_reg is a masked register load called from the decoded-op fast
paths. With fat LTO its inlining depends on how large the surrounding
interpreter happens to be; in a binary where it dropped out of line it
showed 1.7% self time on a JIT-heavy workload. Pin it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UqCuD9vsGeeij5DdYt3vcK
@benletchford
benletchford force-pushed the perf/recorder-inline-pins branch from 66fc897 to e997a33 Compare September 5, 2026 04:06
@benletchford

Copy link
Copy Markdown
Owner

Rebased the two inlining commits onto current master after #173 landed, dropping the dependency on #174. The only conflict was adapting the outlined recorder helpers to the existing per-operation TLS accessor; no batch-wide borrow or instruction-generation code is included.

Local ARM64 validation passes 323 library tests, 45 batch tests, and the nested-CPU callback regression with jit/serde/trace-profile. The default-feature run also passes. Fresh x86-64 CI is running on the rebased head.

@benletchford benletchford left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed the independent inline/outlined split after rebasing onto master. The active helpers retain the existing per-operation TLS access and no generation or batch-borrow changes remain. Native and portable library/batch tests and the nested-CPU regression pass locally; fresh x86-64 CI is green.

@benletchford
benletchford merged commit f62d228 into benletchford:master Sep 5, 2026
4 checks passed
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.

2 participants