perf(ppu): specialized visible-scanline fast dot path (v2.1.8 A1, default-OFF, byte-identical +12.3%)#284
Merged
Merged
Conversation
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in (“default-OFF”) specialized visible-scanline fast path inside the PPU per-dot tick loop to reduce per-dot overhead on the dominant hot case, while preserving byte-identical output vs the exact path and providing a differential test gate to prove equivalence.
Changes:
- Add a guarded fast handler (
tick_visible_render_fast) for “clean” visible-scanline dots (1..=256) behindNes::set_fast_dotloop. - Plumb the runtime knob through
Nes → LockstepBus → Ppu, and add a differential byte-identity integration test plus A/B Criterion bench variants. - Document the design/constraints (lockstep scheduler) and record the change under Unreleased Performance notes.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/ppu-2c02.md | Documents the new fast-dot vs exact-dot split and guard conditions. |
| docs/performance.md | Adds A1 rationale, measurement notes, and scheduler constraints for why this is per-dot (not scanline-batched). |
| crates/rustynes-test-harness/tests/fast_dotloop_diff.rs | Adds a differential “byte-identical across corpus” gate for OFF vs ON runs. |
| crates/rustynes-ppu/src/ppu.rs | Implements the fast_dotloop knob, dispatch guard, and straight-line fast visible-dot handler. |
| crates/rustynes-core/src/nes.rs | Exposes Nes::set_fast_dotloop / Nes::fast_dotloop API surface. |
| crates/rustynes-core/src/bus.rs | Forwards the knob to the PPU via LockstepBus. |
| crates/rustynes-core/benches/full_frame.rs | Adds _fast Criterion variants to compare stock vs fast-dot mode. |
| CHANGELOG.md | Records the new opt-in performance knob and its equivalence/testing claims under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
doublegate
added a commit
that referenced
this pull request
Jul 12, 2026
…dress review Post-rebase onto #280 (opt-in PPU die-revision + power-on model): #280 added the Rp2c02G revision whose only per-dot effect is that an OAMADDR ($2003) write during render ARMS oam_corruption_pending. That armed/pending state is already one of the disturbances the fast-path dispatch guard tests (!oam_corruption_pending), so the fast path drops to the exact path the instant corruption is armed. This adds a Rp2c02G dimension to fast_dotloop_diff (new fast_dotloop_is_byte_identical_under_oamaddr_corruption_revision over the OAM/sprite-heavy corpus) to PROVE fast == exact through #280's corruption paths (bit-identical framebuffer + index + audio + cycles + snapshot). Green. Copilot review adoptions (PR #284): - full_frame.rs bench doc + fast_dotloop_diff module/corpus comments: correct the flowing_palette description — it is the rendering-DISABLED 64-colour backdrop-override demo (the fast path never engages; neutral guard-bail control), NOT a rendering-heavy 'full-BG-every-frame' case. The headline delta is nestest (rendering ENABLED). Aligns with docs/ppu-2c02.md + docs/performance.md and the measured result. - frame_hash: drop the two per-frame Vec<u8> allocations — add fnv1a64_stream(impl Iterator<Item=u8>) (identical FNV-1a algorithm/constants) and fold the u16 index buffer + f32 samples' little-endian bytes directly. Byte-sequence/hash unchanged; differential test still 3/3 bit-identical. Gates re-run post-rebase: fmt, clippy --workspace --all-targets -D warnings (+ scripting / scripting,hd-pack / retroachievements), rustdoc -D warnings, no_std thumbv7em, fast_dotloop_diff (both revisions), AccuracyCoin 141/141, visual_regression, nestest, pal_apu_tests 10/10, save_state — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
force-pushed
the
feat/v2.1.8-fast-dotloop
branch
from
July 12, 2026 05:04
48abaea to
fece1cb
Compare
Add an opt-in, default-OFF specialized straight-line per-dot handler for the PPU's single hottest case — visible-scanline BG-render dots (dots 1..=256, rendering stably enabled, no sub-dot disturbance). Guarded by a runtime knob (Nes::set_fast_dotloop); the shipped default takes the fully-general exact per-dot path and is byte-identical to a build without the field. Profiling (perf, representative mixed corpus) shows Ppu::tick is ~46% of frame self-time. The fast handler runs the identical helper sequence (tick_oam_corruption, tick_sprite_eval_per_dot, tick_oam_bus, reload_bg_shift_regs, ale_drive_*/fetch_* pair, inc_hori_v/inc_vert_v, emit_pixel, shift_bg) with the statically-dead event/bookkeeping branches pruned, so it is byte-identical by construction; any disturbance falls instantly back to the exact path. A whole-scanline batch is architecturally precluded by the v2.0.0 lockstep every-cycle-bus-access scheduler (run_ppu_to advances the PPU <=3 dots per CPU cycle; the CPU observes A12/NMI/sprite-0/$2002 at 3-dot granularity), so this is a per-dot specialization, not a dot-batch. Adds the fast_dotloop_diff differential test (byte-identity across nestest, flowing_palette, oam_stress, AccuracyCoin, MMC1/MMC3 Holy Mapperel, scanline demo — framebuffer + index buffer + audio + CPU cycles + full snapshot, every frame) and full_frame fast A/B bench variants. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reorder the fast-path dispatch guard so the dot-range + rendering-enabled tests short-circuit first (AND is commutative — byte-identity-neutral, and the cache-dependency order scanline==cache before cached_visible is preserved). This cuts the per-dot guard cost on rendering-DISABLED frames (e.g. the flowing_palette all-64-colour backdrop-override demo, which never enters the fast path): the interleaved A/B goes from ~-1% to neutral (~+0.3%) there, while rendering-enabled content (nestest) stays ~+12.3% faster. Docs: - docs/performance.md: new v2.1.8 A1 section — the perf profile (Ppu::tick 46.5% self-time), the design, the architectural note (whole-scanline batch precluded by the lockstep <=3-dots/cycle scheduler), the byte-identity proof, and the measured interleaved before/after (nestest +12.3%, flowing_palette neutral), with the default-OFF ship decision + promotion recommendation. - docs/ppu-2c02.md: the fast-path/exact-path split + disturbance guard. - CHANGELOG.md [Unreleased]: the Performance entry. Gates: fmt, clippy (workspace + scripting + scripting,hd-pack + retroachievements), rustdoc -D warnings, markdownlint, no_std thumbv7em, fast_dotloop_diff (byte-identical across the corpus), AccuracyCoin 141/141, visual_regression, nestest, pal_apu_tests 10/10, save_state, cpu_interrupts — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dress review Post-rebase onto #280 (opt-in PPU die-revision + power-on model): #280 added the Rp2c02G revision whose only per-dot effect is that an OAMADDR ($2003) write during render ARMS oam_corruption_pending. That armed/pending state is already one of the disturbances the fast-path dispatch guard tests (!oam_corruption_pending), so the fast path drops to the exact path the instant corruption is armed. This adds a Rp2c02G dimension to fast_dotloop_diff (new fast_dotloop_is_byte_identical_under_oamaddr_corruption_revision over the OAM/sprite-heavy corpus) to PROVE fast == exact through #280's corruption paths (bit-identical framebuffer + index + audio + cycles + snapshot). Green. Copilot review adoptions (PR #284): - full_frame.rs bench doc + fast_dotloop_diff module/corpus comments: correct the flowing_palette description — it is the rendering-DISABLED 64-colour backdrop-override demo (the fast path never engages; neutral guard-bail control), NOT a rendering-heavy 'full-BG-every-frame' case. The headline delta is nestest (rendering ENABLED). Aligns with docs/ppu-2c02.md + docs/performance.md and the measured result. - frame_hash: drop the two per-frame Vec<u8> allocations — add fnv1a64_stream(impl Iterator<Item=u8>) (identical FNV-1a algorithm/constants) and fold the u16 index buffer + f32 samples' little-endian bytes directly. Byte-sequence/hash unchanged; differential test still 3/3 bit-identical. Gates re-run post-rebase: fmt, clippy --workspace --all-targets -D warnings (+ scripting / scripting,hd-pack / retroachievements), rustdoc -D warnings, no_std thumbv7em, fast_dotloop_diff (both revisions), AccuracyCoin 141/141, visual_regression, nestest, pal_apu_tests 10/10, save_state — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
v2.1.7 (#289) cut while PR #284 was open. Rebasing onto post-v2.1.7 main, git's 3-way merge reapplied the A1 CHANGELOG entry against its old anchor context (the v2.1.6 expansion-audio text + the '## [2.1.5]' line) with no textual conflict — but that context is now INSIDE the released [2.1.6] section, so the entry landed there instead of [Unreleased]. Move it to [Unreleased] (its only content) so the shipped changelog is correct and the v2.1.7 PPU/2A03 die-revision + DMA entries are NOT duplicated (they live under [2.1.7], left exactly as main cut them). Also note in the byte-identity bullet that fast_dotloop_diff now covers #280's opt-in Rp2c02G $2003-corruption path. CHANGELOG-only; no code change (ppu.rs / test / bench identical to the prior fully-gated state). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
force-pushed
the
feat/v2.1.8-fast-dotloop
branch
from
July 12, 2026 05:33
fece1cb to
1f15179
Compare
doublegate
enabled auto-merge (squash)
July 12, 2026 05:35
doublegate
added a commit
that referenced
this pull request
Jul 12, 2026
* release: cut v2.1.8 "Fathom" (performance — "Tempo") Cuts the v2.1.8 release — the performance step of the v2.1.5 → v2.2.0 "deepen the existing project" run. Bumps the workspace version 2.1.7 → 2.1.8, renames the CHANGELOG `[Unreleased]` section to `[2.1.8]`, refreshes Cargo.lock, syncs the README badge / current-release paragraph / BibTeX, and authors `.github/release-notes/v2.1.8.md`. v2.1.8 is profile-first performance work, every change byte-identical (or default-off behind a runtime knob), on the byte-identical default core (AccuracyCoin 141/141, nestest 0-diff, visual_regression / pal_apu_tests unchanged): - #284 — specialized visible-scanline fast PPU dot path (profiled: Ppu::tick was 46.5% of frame self-time), ~12% faster on rendering-heavy frames, shipped default-OFF (Nes::set_fast_dotloop) and differential-tested bit-identical to the exact path across the corpus (incl. the v2.1.7 Rp2c02G corruption path). - #286 — vectorized software palette->RGBA blitter (scalar/wide/wasm-simd128, SIMD==scalar byte-identical; memory-bound so scalar stays default per the >3% bar) + a wasm size pass (wasm-opt -O4 -> 3.99 MiB gzip, under budget). No default-build behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(readme): clarify SIMD blitter is validated-not-a-speedup (memory-bound) Adopts Copilot's nit on the v2.1.8 release PR: 'vectorized blitter' could imply a SIMD speedup, but scalar stays the default because palette->RGBA is a memory-bound LUT gather (no path clears the >3% bar). Reworded to 'SIMD-validated blitter paths ... not a shipped speedup' so the byte-identical-but-non-default reality is clear upfront rather than only in the trailing parenthetical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Summary
Adds a specialized straight-line visible-scanline fast dot path for the PPU (roadmap item v2.1.8 "Performance" A1 — the single highest-risk item), behind a default-OFF runtime knob (
Nes::set_fast_dotloop). The shipped default is unchanged and byte-identical; the fast path is a proven, opt-in speedup.Do NOT merge — opened for review per the task.
Profile first (mandatory)
perf recordof a representative mixed workload (the PGO training corpus) attributes frame self-time as:Ppu::tickLockstepBus::cpu_clockCpu::end_cycleCpu::read1So the PPU per-dot FSM is the dominant hot function — the correct target. (This corrects a stale inference from the synthetic
ppu_tick_one_framebench, whose no-op bus + rendering-disabled default under-represent the real per-dot cost.)Design
Ppu::tickdispatches "clean" visible BG-render dots — a visible scanline, dots1..=256, rendering stably enabled, no sub-dot disturbance (no$2006copy-V / PPUMASK write-delay, no PPUDATA state machine in flight, no armed/pending OAM-corruption, warm classification cache) — toPpu::tick_visible_render_fast, which runs the identical helper sequence with the statically-dead event/bookkeeping branches pruned. Any disturbance drops instantly back to the exact per-dot path. The guard is ordered to short-circuit cheaply for non-covered dots. Compiled out underppu-state-trace.Why a per-dot specialization, not a whole-scanline batch
A Mesen2/tetanes-style whole-scanline batch is architecturally precluded by the v2.0.0 "Timebase" lockstep every-cycle-bus-access scheduler:
run_ppu_toadvances the PPU ≤3 dots per CPU cycle (called twice per cycle, split around the bus access), and the CPU observes A12→MMC3 IRQ (dot 260), the /NMI edge, sprite-0 hit / VBL ($2002), and$2004/$2007reads at that 3-dot granularity — so the PPU is never invited to run a scanline uninterrupted. Batching would require reintroducing the catch-up scheduler v2.0.0 deliberately removed and would break exact-dot event delivery. A1 optimizes the per-dot work, not the dot cadence.Byte-identity — proven, not assumed
New differential test
crates/rustynes-test-harness/tests/fast_dotloop_diff.rs: a corpus (nestest,flowing_palette,oam_stress,AccuracyCoin, Holy Mapperel MMC1/MMC3, a mid-frame raster demo) runs through both paths asserting bit-for-bit identical framebuffer + palette-index framebuffer + audio + CPU cycles + full core snapshot, every frame. Green.Sacred gates (default build, byte-identical by construction): AccuracyCoin 141/141,
nestest0-diff,visual_regression,pal_apu_tests10/10,save_state,cpu_interrupts— all green.Measured (interleaved per-frame A/B, drift-robust)
The dev host was under heavy concurrent build load, which contaminates cross-bench Criterion (later benches absorb load spikes). An interleaved harness alternating OFF/ON at per-frame granularity cancels the drift; rock-stable across 3 rounds at low load:
nestest(rendering enabled)flowing_palette(rendering disabled 64-colour backdrop demo)The +12.3% clears the standing >3% + byte-identical bar decisively; rendering-disabled content never enters the fast path (bails at
rendering_enabled()), so it is neutral. Corroborated by a tight low-load Criterionnes_run_frame_nestest_fast= 4.01 ms vs the clean stock baseline 4.26 ms.Decision
Shipped default-OFF (opt-in). It is a pure byte-identical speedup so it could be default, but as the roadmap's highest-risk item it is kept off for this cut — the shipped build stays byte-identical while the differential test + oracle prove correctness and the A/B proves the win. Recommended for promotion to default after review + a clean-host Criterion confirmation.
Gates run
fmt · clippy
--workspace --all-targets -D warnings+scripting+scripting,hd-pack+retroachievements· rustdoc-D warnings· markdownlint (v0.39.0) ·no_stdthumbv7em ·fast_dotloop_diff· AccuracyCoin 141/141 · visual_regression · nestest · pal_apu_tests · save_state · cpu_interrupts — all green.Files
crates/rustynes-ppu/src/ppu.rs—fast_dotloopfield +set_fast_dotloop/getter, the guarded dispatch,tick_visible_render_fast.crates/rustynes-core/src/{bus.rs,nes.rs}— plumbing.crates/rustynes-test-harness/tests/fast_dotloop_diff.rs— differential test.crates/rustynes-core/benches/full_frame.rs— fast A/B bench variants.docs/performance.md,docs/ppu-2c02.md,CHANGELOG.md— docs.🤖 Generated with Claude Code