diff --git a/CLAUDE.md b/CLAUDE.md index c05a11c..a7560f8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,7 +73,7 @@ shadows it). Full detail: `docs/active/wasm-performance.md`. Key facts: - **Raw throughput** (cycles/sec & IPS) is measured **headless** (`BENCH=1` - benchmark). WASM ≈ 14× JS. + benchmark). WASM ≈ 8× JS (the older "14×" predates the #215 cycle-count fix). - ⚠️ **In-app IPS is throttle-locked** by the `Clock` to ~1MHz (both engines ~331K) — it does NOT show the WASM gain. Never compare engines by in-app IPS. - **Host CPU / headroom** (`hostMillisPerSecond`) is the in-app signal of the diff --git a/docs/active/wasm-performance.md b/docs/active/wasm-performance.md index 4eed189..db7a770 100644 --- a/docs/active/wasm-performance.md +++ b/docs/active/wasm-performance.md @@ -2,19 +2,23 @@ ## Summary -The WASM 6502 engine is **~14× faster than the JS engine** on raw throughput. Earlier +The WASM 6502 engine is **~8× faster than the JS engine** on raw throughput. Earlier observations of "160 IPS" were a **metrics defect**, not an engine problem, compounded by `yarn dev` loading an unoptimized debug WASM build. -| Build / engine | cycles/sec | effective MHz | vs JS | -| --------------------------- | ---------: | ------------: | -----: | -| JS (`CPU6502`) | ~50 M | ~50 MHz | 1× | -| WASM `--dev` (debug) | ~67 M | ~67 MHz | ~1.35× | -| WASM `--release` | ~644 M | ~644 MHz | ~12.9× | -| WASM `--release` + wasm-opt | ~708 M | ~708 MHz | ~14.3× | +| Build / engine | cycles/sec | effective MHz | vs JS | +| --------------------------- | ---------: | ------------: | ----: | +| JS (`CPU6502`) | ~56 M | ~56 MHz | 1× | +| WASM `--release` + wasm-opt | ~443 M | ~443 MHz | ~7.8× | -(Apple M-series, headless benchmark, RAM-only `LDA/STA/LDA/JMP` loop. Absolute numbers vary by -machine; the **ratio** is the durable result.) +(Apple M-series, headless benchmark, RAM-only `LDA #/STA zp/LDA zp/JMP` loop = 11 cycles per 4 +instructions. Absolute numbers vary by machine; the **ratio** is the durable result.) + +The "~14×" figure this doc used to quote was measured while the WASM engine **over-reported +cycles** (issue #215: bus helpers and opcode arms both counted, ~1.8× on this loop). The +instruction rate, which the defect never touched, gives the same answer: ~161 M vs ~18.8 M +instr/s ≈ 8.5×. The debug (`--dev`) and no-wasm-opt rows were dropped rather than re-measured; +their old values (~67 M / ~644 M) carry the same inflation. In the running app **both engines are throttled by the `Clock` to the Apple-1's ~1 MHz target**, so both show ~331 K IPS at ~100%. WASM's advantage is **headroom** — lower CPU/battery cost at 1 MHz, diff --git a/docs/lcd/triage-log.md b/docs/lcd/triage-log.md index 0bedc37..189c17b 100644 --- a/docs/lcd/triage-log.md +++ b/docs/lcd/triage-log.md @@ -4,3 +4,5 @@ 2026-08-06 · hw-accuracy · 5 signals · Deep · hard:yes · risk:yes 2026-08-06 · hw-accuracy · closeout · Deep · audit: PASS (first run) · re-routes: 0 · red-green iters: 2 (Tier 1) · interventions: 1 +2026-09-03 · wasm-cycle-double-count · 2 signals · Standard · hard:no · risk:no +2026-09-03 · wasm-cycle-double-count · closeout · Standard · audit: n/a (engine-internal, no surface) · re-routes: 0 · interventions: 0 diff --git a/docs/lcd/work/wasm-cycle-double-count/JOURNAL.md b/docs/lcd/work/wasm-cycle-double-count/JOURNAL.md new file mode 100644 index 0000000..ef61af0 --- /dev/null +++ b/docs/lcd/work/wasm-cycle-double-count/JOURNAL.md @@ -0,0 +1,48 @@ +# JOURNAL — wasm-cycle-double-count + +> **The resume anchor.** The block between the `lcd-resume:v1` markers below is the *entire* +> cold-start payload — everything a fresh session needs to continue, in <~40 lines. `/lcd:resume` +> reads MAP.md + this block + DECISIONS headers and nothing else. Keep NOW and STEPS current as +> you work (the build loop and `lcd:refine` update them as part of their normal edit/commit +> step). Everything below the `---` is history/detail, NOT read on resume. + + +## NOW + +- **Lane:** Standard +- **Goal:** WASM engine reports the same per-instruction cycle count as the JS engine (issue #215): drop the `+= 1` in the two bus helpers so only the per-arm documented totals count. +- **Next action:** done — PR #218 open (closes #215); after merge, rebase whichever of #216/#217 is still open for `src/version.ts` +- **Branch:** fix/wasm-cycle-double-count · **Updated:** 2026-09-03 23:55 + +## STEPS + +- [x] S1 — Test first: `compareEngines` in engine-parity also asserts the per-step returned cycles match; add a documented-cycle battery (NOP, LDA modes incl. `(zp),Y` page-cross, STA abs,X, JSR/RTS, branch taken/not/page-cross, INC abs). Run against real WASM via `yarn dev:vite` → RED (WASM ≈ 2×). Done: 23 failures, NOP=3 LDA#=4 LDAzp=6. +- [x] S2 — Remove `self.cycles += 1` from `read_byte_from_bus` / `write_byte_to_bus` in `wasm-cpu/src/cpu.rs`; `cargo check`; `yarn wasm:build:release`; parity suite → GREEN (27/27; all 9 engine suites 75/75 vs real WASM). +- [x] S3 — Update memory/docs that describe the double count (memory `wasm-cycle-double-count`, `docs/active/wasm-performance.md` if it mentions it); bump `src/version.ts` (fix/ → patch); `yarn test:ci`; PR closing #215. + +## DECISIONS (this work-item) + +- Model = "per-arm totals only" (issue option 2): smallest diff, matches the JS reference engine, and the JS engine's IRQ/NMI handlers also add a flat 7 with un-counted pushes — so after the fix the interrupt paths agree too. +- Cycle parity is asserted on the *returned* `performSingleStep()` value (the `ICPUEngine` contract) rather than on internal counters — it is what `Clock` budgets against. + +## OPEN QUESTIONS + +- none + +## EDIT BOUNDARY (paths this work may touch) + +- `wasm-cpu/src/cpu.rs` +- `src/core/cpu-engines/__tests__/engine-parity.vitest.test.ts` +- `src/version.ts` +- `docs/active/wasm-performance.md`, `CLAUDE.md` (throughput ratio was measured with the inflated count) + + +--- + +## LOG (append-only; not read on resume) + +- 2026-09-03 23:20 — Triage → Standard (2 signals). Cause confirmed in `cpu.rs:476-490` (helpers add 1 per access) + every arm adds the full documented count. JS engine (`src/core/cpu6502/core.ts`) counts only per-arm totals; its bus read/write helpers do not touch `cycles`. +- 2026-09-03 23:40 — S1 done. Real-WASM run (yarn dev:vite on :3000): 23/27 parity tests RED on cycles only; state assertions still pass. +- 2026-09-03 23:50 — S2 done: two `+= 1` removed from `cpu.rs` bus helpers; `cargo check` clean; WASM release rebuilt; parity 27/27, all engine suites 75/75 against real WASM; `yarn test:ci` 818 passed with the server up (parity suites ran for real). +- 2026-09-03 23:55 — S3: re-ran `BENCH=1` benchmark with honest counting → WASM 443 M vs JS 56 M cycles/s = 7.8× (instr/s 8.5×). Old "14×" was inflated by the bug. Updated `wasm-performance.md` + CLAUDE.md. Version 4.51.12. +- 2026-09-04 00:05 — PR #218 opened. Closeout appended (no surface declared → audit n/a). diff --git a/src/core/cpu-engines/__tests__/engine-parity.vitest.test.ts b/src/core/cpu-engines/__tests__/engine-parity.vitest.test.ts index c95b5d4..8824d7d 100644 --- a/src/core/cpu-engines/__tests__/engine-parity.vitest.test.ts +++ b/src/core/cpu-engines/__tests__/engine-parity.vitest.test.ts @@ -101,10 +101,14 @@ describe.skipIf(!wasmRuntimeAvailable)('CPU Engine Parity Tests', () => { jsEngine.setRegisters({ PC: 0x0000 }); wasmEngine.setRegisters({ PC: 0x0000 }); - // Execute the same number of cycles on both + // Execute the same number of instructions on both. Each step must also + // report the same cycle count: `Clock` budgets against this value, so a + // divergence here means the engines run at different emulated speeds + // even when their state agrees (#215). for (let i = 0; i < cycles; i++) { - jsEngine.performSingleStep(); - wasmEngine.performSingleStep(); + const jsCycles = jsEngine.performSingleStep(); + const wasmCycles = wasmEngine.performSingleStep(); + expect(wasmCycles, `cycles reported at step ${i}`).toBe(jsCycles); } // Get final state from both engines @@ -542,4 +546,79 @@ describe.skipIf(!wasmRuntimeAvailable)('CPU Engine Parity Tests', () => { dualEngine.cleanup(); }); }); + /** + * Reported cycle counts against the documented 6502 timings (#215). + * + * The JS engine is the reference for state, but for cycles both engines + * are held to the datasheet: a test that only compared the two would pass + * if they were wrong in the same way. Before #215 the WASM engine counted + * every bus access AND the per-instruction total, reporting ~2x. + */ + describe('Cycle count parity (#215)', () => { + function reportedCycles(program: number[], setup?: (write: (addr: number, value: number) => void) => void) { + if (!wasmEngine) { + throw new Error('WASM engine expected but failed to initialize'); + } + jsEngine.reset(); + wasmEngine.reset(); + const write = (addr: number, value: number) => { + bus.write(addr, value); + wasmEngine!.write(addr, value); + }; + program.forEach((byte, i) => write(i, byte)); + setup?.(write); + jsEngine.setRegisters({ PC: 0x0000 }); + wasmEngine.setRegisters({ PC: 0x0000 }); + return { js: () => jsEngine.performSingleStep(), wasm: () => wasmEngine!.performSingleStep() }; + } + + function expectCycles(program: number[], expected: number[], setup?: (write: (addr: number, value: number) => void) => void) { + const step = reportedCycles(program, setup); + const js = expected.map(() => step.js()); + const wasm = expected.map(() => step.wasm()); + expect(js, 'JS engine cycles').toEqual(expected); + expect(wasm, 'WASM engine cycles').toEqual(expected); + } + + it('implied and immediate: NOP=2, LDA #=2', () => { + expectCycles([0xea, 0xa9, 0x01], [2, 2]); + }); + + it('LDA zp=3, LDA abs=4', () => { + expectCycles([0xa5, 0x10, 0xad, 0x34, 0x12], [3, 4]); + }); + + it('LDA abs,X: 4 without page cross, 5 with', () => { + // LDX #$01; LDA $1234,X ; LDX #$20; LDA $12F0,X ($1310 crosses) + expectCycles([0xa2, 0x01, 0xbd, 0x34, 0x12, 0xa2, 0x20, 0xbd, 0xf0, 0x12], [2, 4, 2, 5]); + }); + + it('LDA (zp),Y: 5 without page cross, 6 with', () => { + // ($20) -> $1210. LDY #$00; LDA ($20),Y ; LDY #$F0; LDA ($20),Y ($1300 crosses) + expectCycles([0xa0, 0x00, 0xb1, 0x20, 0xa0, 0xf0, 0xb1, 0x20], [2, 5, 2, 6], (write) => { + write(0x20, 0x10); + write(0x21, 0x12); + }); + }); + + it('STA abs,X always 5; INC abs 6', () => { + expectCycles([0x9d, 0x34, 0x12, 0xee, 0x34, 0x12], [5, 6]); + }); + + it('JSR=6, RTS=6', () => { + // JSR $0010 ; at $0010: RTS + expectCycles([0x20, 0x10, 0x00], [6, 6], (write) => write(0x10, 0x60)); + }); + + it('branch: 2 not taken, 3 taken, 4 taken across a page', () => { + // LDA #$00; BNE +2 (not taken) ; LDX #$01; BNE +0 (taken, same page) + expectCycles([0xa9, 0x00, 0xd0, 0x02, 0xa2, 0x01, 0xd0, 0x00], [2, 2, 2, 3]); + // LDX #$01; BNE -$14 (from $0004 to $FFF0 crosses a page) + expectCycles([0xa2, 0x01, 0xd0, 0xec], [2, 4]); + }); + + it('stack: PHA=3, PLA=4', () => { + expectCycles([0x48, 0x68], [3, 4]); + }); + }); }); diff --git a/src/version.ts b/src/version.ts index 12abb20..c98c28e 100644 --- a/src/version.ts +++ b/src/version.ts @@ -1 +1 @@ -export const APP_VERSION = '4.51.11'; +export const APP_VERSION = '4.51.12'; diff --git a/wasm-cpu/src/cpu.rs b/wasm-cpu/src/cpu.rs index f031d9c..5616a1d 100644 --- a/wasm-cpu/src/cpu.rs +++ b/wasm-cpu/src/cpu.rs @@ -474,7 +474,8 @@ impl CPU6502 { impl CPU6502 { /// Read a byte from memory using internal Bus (for WasmSystem) pub(crate) fn read_byte_from_bus(&mut self, bus: &crate::Bus, address: u16) -> u8 { - self.cycles += 1; + // Cycles are accounted per instruction by the opcode arms (the same + // model as the JS engine); a bus access must not add to the count. let data = bus.read(address); self.last_addr = address; self.last_data = data; @@ -483,7 +484,6 @@ impl CPU6502 { /// Write a byte to memory using internal Bus (for WasmSystem) pub(crate) fn write_byte_to_bus(&mut self, bus: &mut crate::Bus, address: u16, value: u8) { - self.cycles += 1; self.last_addr = address; self.last_data = value; bus.write(address, value);