perf(hw): choose write_region's delta strategy by measured link cost - #275
Merged
Conversation
write_region decided how to split a changed region into writes by comparing byte counts: chunk when the dirty 256-byte slabs totalled under 60% of the region, else push the whole thing. Bytes are the wrong currency. On socket DMA a write costs ~5.2 ms regardless of payload up to ~2.4 KB, so splitting a region into k pieces multiplies its cost by k while the byte count obediently goes down. Priced against the real link, the old rule lost every time it fired: 19 of 19 firings across three display modes on real video were slower than not chunking, one mhires clip losing 1083 ms over 400 frames concentrated into 14 frames that each stalled ~77 ms — a visible hitch on scene cuts, which is exactly when a wide sparse dirty pattern occurs. The choice is now a cost comparison against HardwareProfile.write_cost_s, max(floor, intercept + per_byte * B), measured per backend by the new link_cost_model.py. The two links sit at opposite extremes and want opposite policies, which is why this belongs on the profile: Ultimate 64 5.22 ms/write, 1.85 us/B, knee ~2.4 KB count-bound TeensyROM+ 0.29 ms/write, 1.44 us/B, knee ~54 B byte-bound The marginal slopes agree within ~30% because both are the C64 bus at about a cycle a byte; the 18x gap in fixed cost is the link protocol, and it inverts the answer. The chunked branch still earns its place on the TeensyROM for the sparse-waveform case it was written for. The separate full-push branch is gone: a span write is a subset of the same bytes and cost is monotonic in payload, so the span is never worse. Wide changes now push only the dirty range instead of the whole region, which took the same clip's mean bitmap push from 4265 to 3280 B/frame. Measured on the same clip: mean frame 18.1 -> 13.3 ms, worst frame 104.4 -> 26.0 ms, link-bound ceiling 55 -> 75 fps. full_threshold is removed from the write_region signature; no caller ever passed it, and it no longer names anything.
The mhires numbers describe the host-DMA path. On an Ultimate with the REU enabled, use_reu_staged stages hires/mhires bitmaps through the REU bank-swap and never reaches write_region — so on a default U64 what improves for those modes is screen and color RAM, not the bitmap. Char modes and the whole TeensyROM+ are always on this path. Adds the on-hardware result: at unchanged write rate and frame rate, ~25% fewer bytes on the host-DMA bitmap path (220 -> 166 KiB/s) and ~28% in char modes (41 -> 30 KiB/s), measured over a minute per mode. The write rate is deliberately reported as unchanged: the chunked branch fired on ~3.5% of frames, so its cost lands in the worst frames, not in an average.
The 'reducing write count is the only real lever' rule was asserted without its reason. The reason is now measured: payload is free below ~2.4 KB on socket DMA, so an 8-byte write and a 2 KB write cost the same ~5.2 ms and splitting one write in two doubles its price. That is what makes count the lever, and it is also what makes widening a write across a clean gap free. Records it in the three places someone would hit it from: the caveats transport section (beside the latency table it follows from), CLAUDE.md's standing note, and the extending.md guidance against hand-rolled diff layers. All three now say the same thing about scope — the trade is per-link and inverted on the TeensyROM+, which is why it lives on HardwareProfile.write_cost_s and not in any caller.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #275 +/- ##
=======================================
Coverage 82.03% 82.04%
=======================================
Files 142 142
Lines 24729 24731 +2
Branches 3628 3627 -1
=======================================
+ Hits 20287 20291 +4
+ Misses 3648 3647 -1
+ Partials 794 793 -1 ☔ View full report in Codecov by Harness. |
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.
What
write_regiondecided how to split a changed region into writes by comparing byte counts: chunk when the dirty 256-byte slabs totalled under 60% of the region, otherwise push the whole region. Bytes are the wrong currency.On socket DMA a write costs ~5.2 ms regardless of payload up to ~2.4 KB. Splitting a region into k pieces therefore multiplies its cost by k while the byte count obediently goes down — so the old rule was scored by a metric that could only ever say it was winning.
The choice is now a cost comparison against
HardwareProfile.write_cost_s,max(floor, intercept + per_byte × B), measured per backend by a new diagnostic.Why it belongs on the profile
The two links sit at opposite extremes and want opposite policies:
The marginal slopes agree within ~30% — both are the C64 bus at about a cycle a byte. What differs by 18× is the fixed per-write cost, which is the link protocol, and that single number inverts the right answer. The chunked branch still earns its place on the TeensyROM for the sparse-waveform case it was written for; it simply must not fire where the second write costs more than the bytes it saves.
Measurements
Offline, driving the real pipeline against real video (
video_render_probe.py), the old rule was slower than not chunking on 19 of 19 firings across three display modes. One mhires clip lost 1083 ms over 400 frames, concentrated into 14 frames that each stalled ~77 ms — a visible hitch on scene cuts, which is exactly when a wide sparse dirty pattern occurs.On hardware, a minute of video per mode at unchanged write rate and frame rate: ~25% fewer bytes on the host-DMA bitmap path (220 → 166 KiB/s) and ~28% in the character modes (41 → 30 KiB/s).
The write rate deliberately does not move — the chunked branch fired on only ~3.5% of frames, so its cost lands in the worst frames rather than in an average. That is why the per-frame offline probe is the tool that can see it and a 10-second heartbeat average is not.
Also in here
full_thresholdis removed from thewrite_regionsignature — no caller ever passed it, and it no longer names anything.scripts/diags/link_cost_model.py, which is where the profile constants come from. It fits the two regimes separately: a single straight line through the flat part and the sloped part reports a healthy r² while understating both terms, and the floor is the term the decision turns on.video_render_probe.pynow counts writes as well as bytes and prices frames with the profile's model, flagging any region split into more writes than pushing it whole would have cost.Scope
With the REU enabled,
[video].use_reu_staged = autostageshires/mhiresbitmaps through the REU bank-swap, which never reacheswrite_region— so on a default Ultimate the bitmap figures describe the host-DMA path, and what improves for those modes is screen and color RAM. Char modes are always on this path, as is everything on the TeensyROM+.Verification
make checkgreen: ruff,mypy --strict, pyright, 3742 tests.make site-checkgreen.