Append reuse: skip unchanged traces in encode and on the wire (protocol v6)#186
Conversation
…ol v6) An append tick previously re-emitted every trace and shipped every byte. Now it costs O(affected trace), not O(figure): - Emit cache: unchanged traces splice their cached spec fragment and encoded chunks — no gathers, no f64->f32 re-encode, no density re-bin. The cache keys on an emit key (trace data_rev, px, drill state, and the axis ranges only for range-dependent tiers, so a neighbor's growing axis busts M4/density traces but not direct ones). The splice is positional; any column-table drift busts the build back to a fresh full emission. - Wire reuse: split columns carry a deterministic content identity (cid = trace + emit-key digest + ordinal). A cid in the client baseline (reset by every full split build) ships as addressing only, no buffer; the client resolves it from the payload it last applied and normalizes the table back to buf-per-column, keeping context restore and animation untouched. A cache miss sends one refresh request (new request kind) answered with a complete append-shaped payload. - Widget transport: the per-tick push is a small custom message again; the synced traits become debounced notebook-reopen state (complete payload, trailing sync via the running event loop, inline without one). The v5 trait-listener and its torn-pair handling are removed - traits no longer carry pushes. Protocol bumps to v6: a pre-v6 bundle would fail loudly on a cid-only column, so the first-paint handshake catches cached bundles. Measured (10x50k-trace dashboard, one trace streaming, median/tick): 3912 -> 397 KiB on the wire (-90%), kernel build 0.52 -> 0.27 ms. Single-trace figures are unchanged (the affected trace still re-ships; that is the delta-frames follow-up). Part of #163.
Merging this PR will not alter performance
Comparing Footnotes
|
Greptile SummaryThis PR makes streaming appends reuse unchanged trace data across encoding and transport. The main changes are:
Confidence Score: 5/5This looks safe to merge. The updated resolver now routes undersized split buffers through refresh recovery. No distinct blocking issue remains in the reviewed fix path.
What T-Rex did
Important Files Changed
Reviews (3): Last reviewed commit: "Pin the range-free emitter list and the ..." | Re-trigger Greptile |
A non-empty but undersized buffer resolved as a valid column span, so _applyAppend proceeded past the recovery gate: the spec and retained payload were swapped and trace resources destroyed before _columnView threw RangeError mid-rebuild — a torn client with no refresh requested. payloadResolve now checks each resolved span against the column's declared extent (byte_offset + len * bytes-per-element); a short span counts as missing, which routes through the existing refresh fallback before anything mutates. Covers both wire buffers and cached cid spans, so a stale cache entry whose length no longer matches the spec also falls back instead of tearing. Verified with a browser probe against real kernel payloads: the pre-fix bundle throws 'column extends past chart payload' from _buildTrace with the spec already swapped and no refresh sent; the fixed bundle sends exactly one refresh, mutates nothing, and applies the intact append afterwards.
FarhanAliRaza
left a comment
There was a problem hiding this comment.
Reviewed the full diff and verified the claims empirically on this branch — the headline number reproduces: 10×50k dashboard streaming one trace, measured against the merge-base with fix/append-single-send (same venv, same native core, only PYTHONPATH differs): 3,911 KiB → 396 KiB per tick (−89.9%, 9.9×), kernel 3.64 → 0.75 ms. Targeted suites (test_streaming / test_widget / test_figure, 177 tests) pass. The architecture is sound: the positional-splice guard fails toward full rebuilds, the v5 torn-trait hazard is deleted rather than defended, and payloadResolve validates before mutating. Findings below were each verified with runtime probes, not just read off the diff.
Asks before merge (both small)
1. Pin the range-free emitter list with a test. _trace_emit_key omits axis ranges for kinds asserted range-free, so the emit cache serves cached bytes across range changes for them. I probed every listed kind (direct scatter, direct line, area, hexbin, histogram, heatmap, segments, bar): all currently emit byte-identical chunks under disjoint range pairs — the list is correct today. But nothing enforces it: if any of these emitters later gains range dependence (a windowed gather, a view-clipped optimization), appends silently splice stale geometry — exactly the silent-decision class §28 exists to prevent. A parametrized test that emits each range-free kind under two disjoint range pairs and asserts byte-identical chunks (~40 lines) makes the list unable to rot. Unknown kinds already default to range-dependent, which is the safe direction.
2. One sentence in wire-protocol.md §5: cid cross-build stability is a requirement, not an accident. Verified: two consecutive full builds derive identical cids for unchanged data — which is the only reason baseline resets (reopen sync, new subscriber, refresh) don't break reuse, and the only reason a client mounted from ≤1 s-stale reopen state can resolve the next push's cid-only entries. Today that stability falls out of the digest construction; if anyone ever salts the digest or adds a build-dependent input, multi-client streams degrade to refresh storms with no test failing. Worth stating explicitly in the spec.
Follow-ups (non-blocking, all confirmed by probe)
- Reopen sync clears the emit cache every interval.
_sync_reopen_state→build_payload_split()→_append_emit_cache = {}, so at 60 ticks/s every ~60th append re-emits all traces (wire reuse survives via stable cids; the CPU win doesn't). The reset's px-mismatch rationale doesn't apply to the widget reopen path — preserving the cache when the resolved px matches would remove a periodic latency spike on exactly the hot path this PR optimizes. - Appends without a running event loop silently lose the debounce. Probe: 5 appends with no loop → 5 inline full trait syncs (with a loop: 0 immediate, trailing timer armed — correct). A user streaming from a worker thread pays the full v5 trait-sync cost per tick without knowing, and
loop.call_laterfrom a non-loop thread would be unsafe if a loop were reachable. Deserves at least a docstring note;call_soon_threadsafescheduling if threaded producers are meant to be supported. - The JS resolve path has no automated coverage. No test anywhere references
payloadResolve/_requestRefresh/_reseedCidSpans; the browser probe is manual. Python-side coverage is well chosen (buffer omission, object-identity cache hit, baseline-reset survival, tier-flip bust, range-growth busting range-dependent neighbors only). An automated headless test for resolve-miss → exactly-one-refresh → repair would protect the trickiest client logic — and the O(K) delta follow-up leans on this same machinery, so it pays for itself across the stack.
Address FarhanAliRaza's review on #186: - The always-range-free kind set moves to a module constant (RANGE_FREE_KINDS) pinned by a parametrized test that emits every listed kind under two disjoint range pairs and asserts the emission — spec fragment, column table including cids, buffer bytes — is byte-identical. A kind added without a builder fails the coverage test; an emitter that gains range dependence fails the emission test. The conditional direct tiers (scatter/line/area/error_band below their thresholds) are pinned by the same test. - Writing that test surfaced two dead names in the old inline set: no trace ever carries kind "rect" or "bar_compact". The real rect-family kinds (bar, column, box, box_whisker, box_median, violin — emitters that del the ranges at entry) now key range-free, so bars/boxes/violins stop busting on a neighbor's growing axis; the dead names are gone. - wire-protocol.md §5 states cid cross-build stability as a requirement: every baseline reset re-ships columns under the cids the next append addresses, so a salted or build-local digest input would degrade resets and multi-client streams to per-tick refresh round-trips. - FigureWidget.append documents the reopen-sync debounce's event-loop dependence: without a running loop in the calling thread every append re-syncs inline — correct, but full trait-sync cost per tick.
Stacked on #178 (base:
fix/append-single-send). Part of #163 — implements the append-path half of per-column reuse.What
An append tick previously re-emitted every trace and shipped every byte. Now it costs O(affected trace), not O(figure), in both CPU and wire bytes:
data_rev(new, bumped per append), px, drill state, and the axis ranges only for range-dependent tiers — so a neighbor's growing axis correctly busts M4/density traces while direct traces keep hitting (verified per emitter). The splice is positional; any column-table drift (e.g. the affected trace crossing the density threshold) busts the whole build back to a fresh full emission — correctness never rides the cache.cid). Split columns now carry a deterministic content identity (t<trace>.<emit-key digest>.<ordinal>). A cid in the client baseline (reset by every full split build) ships as addressing only — no buffer. The client resolves cid-only entries from the payload it last applied viapayloadResolve, then normalizes the table back tobuf: i, so_columnView, GL context restore, and the animation path see one uniform per-column array, unchanged.refreshrequest (new request kind, §2) answered with a complete append-shaped payload.Protocol bumps to v6 — a pre-v6 bundle would fail loudly on a cid-only column; the first-paint handshake catches cached bundles instead.
Measured
Same bench as #178 (median per tick, data-level wire accounting):
The single-trace rows are the affected trace itself re-shipping — that's the O(K) delta-frames follow-up, not this PR.
Spec
wire-protocol.md: §2refreshrow + description, §4 rewritten (partial buffers, emit cache, per-host delivery, reopen-state debounce), §5cidfield, §7 v6 note;rust-engine.md§5 updated. CHANGELOG under Unreleased → Changed.Verification
refreshcontract).refreshwithout applying a torn table; the refresh reply repairs and re-arms. All green.ruff/ruff format/ pre-commit /ty(25 diagnostics — pre-existing baseline, none new) /node js/build.mjs.🤖 Generated with Claude Code