Skip to content

bench: commit class_attrs_methods jitstats baselines - #952

Merged
youknowone merged 2 commits into
pyre-lang:mainfrom
fregataa:bench/class-attrs-methods-jitstats-baseline
Aug 2, 2026
Merged

youknowone merged 2 commits into
pyre-lang:mainfrom
fregataa:bench/class-attrs-methods-jitstats-baseline

Conversation

@fregataa

@fregataa fregataa commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor

What

Commit class_attrs_methods.{dynasm,cranelift}.jitstats structural baselines
(plus their pyre/.gitignore exception line, following the existing
comprehension_object_append_hot / const_arg_call_resume /
nested_list_comprehension_hot pattern).

loops_compiled=1
loops_aborted=0
guard_failures=1
bridges_compiled=0
internal_compile_panics=0
...

Why

class_attrs_methods is the max-pypy-ratio=824 fixture — the loosest perf
gate in the tree, calibrated (#853) when its hot c.add(i & 7) method call was
declined by the method-form inline gate and ran as a full CallMayForceR
residual round trip per iteration (fresh callee PyFrame, warmstate dispatch
into a separately compiled function-entry trace via execute_token,
re-boxing — ~1.7s user on an M-series dynasm build vs 0.09s CPython).

Current main inlines the call again: loops_compiled 2 → 1, dynasm 0.07s /
cranelift 0.09s user. But nothing pins that trace shape — with an 824x
allowance, the callee residualizing again (and recompiling its own func-entry
loop) would sail through a bare pyre/check.py. The committed baselines make
check.py's jit-stats regression floor catch exactly that structural
regression, timing noise aside.

Baselines were recorded with
pyre/check.py --synthetic-only --synthetic-pattern class_attrs_methods.py --snapshot
and verified green on a bare re-run (dynasm 1/1, cranelift 1/1, 4.2x/6.2x vs
pypy). No wasm baseline: no wasm toolchain on this machine — can follow up
from a runner that has one, as the other baselined benches carry a wasm file.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated repository tracking configuration to retain selected benchmark result files.

class_attrs_methods regained its method-call inline (loops_compiled
2 -> 1, dynasm 0.07s user vs the 1.7s residual round trip it gated at
max-pypy-ratio=824), but nothing pins the trace shape: the perf gate is
too loose to notice the callee residualizing again.

Record the dynasm/cranelift structural baselines (loops_compiled=1,
loops_aborted=0) so check.py's regression floor reddens a bare run the
moment the method call falls back to a residual with its own func-entry
trace. Recorded via check.py --synthetic-only --snapshot on macOS arm64;
no wasm toolchain locally, so the wasm baseline is left for a runner
that has one.

Co-Authored-By: Claude Fable 5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d4aad851-622d-49bc-b82e-6b200f57aece

📥 Commits

Reviewing files that changed from the base of the PR and between 8be024a and 41c07c2.

📒 Files selected for processing (1)
  • pyre/.gitignore

Walkthrough

The change adds a .gitignore exception for class_attrs_methods synthetic benchmark .jitstats files.

Changes

Class attributes JIT statistics

Layer / File(s) Summary
Add the JIT statistics ignore exception
pyre/.gitignore
Adds /bench/synth/class_attrs_methods.*.jitstats to the committed exceptions.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

Suggested reviewers: youknowone

Poem

A rabbit found stats beneath the tree,
Now Git keeps them safely, wild and free.
Class methods hop into the set,
Their JIT numbers stay reset.
One small rule, neat as can be.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the addition of class_attrs_methods JIT statistics baselines.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8be024a66d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

guard_failures=1
internal_compile_panics=0
loops_aborted=0
loops_compiled=1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Gate loops_compiled before relying on this baseline

In the default pyre/check.py path this value is never compared: _jit_stats_regression_floor only considers the badness fields (loops_aborted, internal_compile_panics, and descr-set counters), and the CI --snapshot-diff invocation skips synthetic benches with --no-synthetic. If this benchmark regresses back to a separate method-entry trace (1 loop -> 2 loops) without aborting, the new baseline still lets a bare check pass; add an exact/default gate for this synthetic stat (or tighten the perf gate) rather than just recording the value.

Useful? React with 👍 / 👎.

Comment thread pyre/.gitignore Outdated
# these on every run, which is the only gate wasm has — run_synthetic_bench
# passes vs_pypy=None for wasm, so no perf ratio can catch a wasm-only trace
# abort storm there.
!/bench/synth/class_attrs_methods.*.jitstats

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Commit the wasm jitstats baseline too

On Linux CI, installing wasm32-unknown-unknown makes wasm part of DEFAULT_BACKENDS, and _apply_snapshot_gate only applies the regression floor when pyre/bench/synth/<name>.wasm.jitstats exists. This new unignore pattern covers class_attrs_methods.wasm.jitstats, but only the native baseline files were committed, so wasm-only abort/panic/descr-counter regressions for this synthetic bench still have no committed floor (unlike the other exceptions). Add the 5-field wasm baseline when enabling this benchmark.

Useful? React with 👍 / 👎.

@youknowone youknowone 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.

👍

@youknowone
youknowone merged commit 1a8e5f5 into pyre-lang:main Aug 2, 2026
7 of 8 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