Skip to content

docs(plan): record the resolved speed picture - #7405

Merged
proggeramlug merged 2 commits into
mainfrom
docs/plan-speed-final
Aug 4, 2026
Merged

docs(plan): record the resolved speed picture#7405
proggeramlug merged 2 commits into
mainfrom
docs/plan-speed-final

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Completes the plan's Part 2 with what today actually established. The earlier update captured the first measurement; four corrections followed, and the corrections are the useful part.

The benchmarks were lying in two different ways

Fixed and shipped in #7403. Discarding the result let Perry eliminate the loop; and even with it consumed, fibonacci(FIB_N) is loop-invariant, so it hoists out and runs once — checksum correct, TOTAL zero.

benchmark was reported actually
bench_fibonacci ~240× faster 2.5× faster
bench_bitwise ∞ faster 20.4× SLOWER

The fast path already exists

bench_bitwise's hot loop has no runtime helper calls at all — 4 frem per iteration, 4 × bl _fmod, 1754 profile samples. But expr/binary.rs:588 already emits guarded srem with the IEEE -0 correction. The gap is the analysis, reduced to one line (#7404):

let a = 12345678;  a % 1000            // srem — fires
let a = 12345678;  a % 1000; a = a + 1 // frem — lost

Four wrong hypotheses about that subsystem are recorded so they aren't retried, along with why a careless widening is dangerous: it returns silently wrong integer arithmetic that === tests cannot see.

A measured negative result

js_array_grow's HOLE-init loop is already vectorised (6 stp in the baseline), so the slice::fill rewrite proposed for #7396 measures nothing. The contrary reading came from grepping a symbol absent from a stripped binary — grep -c returns 0 whether or not the pattern exists.

Method notes strengthened

Third instance today of a check that could not fail, and a new one worth its own line: a fix that compiles, passes every correctness test, and changes no emitted instruction is a fix to the wrong site. The guarded srem I wrote for #7404 passed the entire -0 edge-case table and emitted zero srem.

Summary by CodeRabbit

  • Documentation
    • Updated performance documentation with corrected benchmark results and validation details.
    • Documented that bench_bitwise is significantly slower than Node due to repeated modulo calculations.
    • Clarified findings around integer analysis, array growth, loop optimization, and store performance.
    • Added notes on rejected optimization hypotheses and improved measurement methodology.

Ralph Küpper added 2 commits August 4, 2026 23:37
The earlier update captured the first measurement; four corrections
followed and the corrections are the useful part.

Both bench_fibonacci and bench_bitwise measured nothing, in two different
ways: a discarded result, and a loop-invariant call that hoists out and
runs ONCE while the checksum stays correct. Fixed in #7403, which inverts
the headline -- bench_bitwise was reported as infinitely faster than Node
and is 20.4x SLOWER.

That gap is 4 frem per iteration lowering to bl _fmod, and the guarded
srem fast path ALREADY EXISTS at expr/binary.rs:588 with the IEEE -0
correction. The gap is is_integer_valued_expr, reduced to a one-line
repro: reassigning the local disqualifies it. Four wrong hypotheses about
that subsystem are recorded in #7404 so they are not retried.

Also records a measured negative result: js_array_grow's HOLE-init loop
is already vectorised (6 stp in the baseline), so the slice::fill rewrite
proposed for #7396 measures nothing. The original contrary reading came
from grepping a symbol absent from a stripped binary.
@proggeramlug
proggeramlug merged commit cd29706 into main Aug 4, 2026
@proggeramlug
proggeramlug deleted the docs/plan-speed-final branch August 4, 2026 21:37
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 08b0ca5c-9623-4d10-b52d-e273a263e714

📥 Commits

Reviewing files that changed from the base of the PR and between d5b115d and b7869b4.

📒 Files selected for processing (2)
  • changelog.d/7405-plan-speed-final.md
  • docs/engine-plan.md

📝 Walkthrough

Walkthrough

The PR documents corrected benchmark measurements and expanded validation methods. It attributes the bench_bitwise slowdown to % lowering and integer-valued analysis limits. It also records the rejected js_array_grow rewrite and additional emitted-IR checks.

Changes

Benchmark analysis

Layer / File(s) Summary
Corrected benchmark findings
changelog.d/7405-plan-speed-final.md, docs/engine-plan.md
The documentation adds checksum-based benchmark validation, corrected benchmark ratios, % lowering findings, and the rejected array-growth rewrite.
Expanded validation method
docs/engine-plan.md
The method notes add heap-limit, symbol, disassembly, debug-symbol, nm, and emitted-IR validation checks.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

  • Issue 7404 — Covers the same bench_bitwise %/fmod bottleneck and integer-analysis limitation.

Possibly related PRs

  • PerryTS/perry#6860 — Addresses related integer-value and type analysis.
  • PerryTS/perry#6902 — Covers related integer remainder lowering and proof changes.
  • PerryTS/perry#7397 — Updates overlapping benchmark methodology and performance findings in docs/engine-plan.md.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/plan-speed-final

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.

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.

1 participant