Speed up get_render() - #751
Merged
Merged
Conversation
… request coalescing get_render() carried avoidable fixed overhead on every blocking round trip, plus a race that could hang concurrent callers: - The request message sat in the outgoing per-client buffer for up to one windowing delay (~1/60s) before hitting the wire. get_render() now flushes the buffer immediately after queueing, since batching latency buys nothing for a blocking round trip. - The response payload was decoded (and imageio lazily imported, slow on first use) inside the response callback, which runs on the server's asyncio event loop -- stalling message handling for every client for the duration of a large PNG/JPEG decode. Decoding now happens on the calling thread, and the import is started right after the request is sent so first-use import cost overlaps the render round trip instead of adding to it. - GetRenderRequestMessage had no redundancy_key override, so every request shared the class-name default key: a second concurrent get_render() on the same client evicted the first caller's still-unsent request from the buffer, hanging that caller until timeout. Requests are now keyed by their render_uuid so each in-flight request survives independently. tests/test_get_render_latency.py pins all three behaviors (each test fails against the previous implementation): per-request buffer slots, concurrent callers resolving to their own frames from crossed responses, the request window surfacing promptly with the buffer's window delay set to 10s, and decode running on the caller's thread rather than the dispatch thread.
…cal path
Follow-up latency work on get_render(), targeting the client-side fixed
costs that dominated after the server-side fixes:
- The capture state machine idled two full frames between processing a
render request and capturing ("triggered" -> "pause" -> capture). The
root cause was ordering: capture ran inside the message handler's
useFrame at priority -100000, BEFORE SceneTree's per-node pose appliers
(-1000), so extra frames were needed for poses to land. Capture now runs
in its own useFrame at priority -1 -- after the pose appliers -- and
defensively applies any still-pending "needsUpdate" poses itself. A
request that arrives with no messages in front of it captures in the
SAME frame it is processed; a request that arrives alongside scene
updates waits exactly one frame for React to commit them (sync-lane
external-store flushes complete before the next frame).
- Message handling was blocked until the async toBlob encode finished
("in_progress"). The pixels are already copied out of the renderer
before encoding starts, so the gate now lifts as soon as the copy is
done and the encode overlaps whatever comes next.
- New transport_format="raw": unencoded RGBA over the websocket, skipping
canvas.toBlob() on the client and imageio decode on the server (which
cost ~12ms combined at 320x240 and ~100ms at 1080p in benchmarks).
Returns a lossless (H, W, 4) array like PNG. Render response payloads
are also transferred (not structured-cloned) to the websocket worker.
- get_render() now flushes the BROADCAST buffer before sending its
request: scene updates ride a different buffer than the request, and a
flushed request could otherwise overtake a still-windowed scene update
and capture a stale frame.
Correctness coverage:
- tests/e2e/test_get_render_capture.py drives a real browser: captures
requested immediately after alternating color updates must never return
a stale frame (exercises the one-frame commit path and cross-buffer
ordering), and all three transport formats must agree on the scene.
- tests/test_get_render_latency.py: raw round trip (wire format, reshape,
writability, size-mismatch error) and the broadcast-flush-before-request
ordering guard.
Also ignores PLW0108 (stabilized in newer ruff; flags pre-existing
callback-style lambdas), following the PLR0917 precedent.
Benchmarks (headless Chromium + SwiftShader, sequential captures):
320x240: 127ms/call before this series, 84ms jpeg / 48ms raw now.
…t test Two additions after auditing the same-frame capture path's remaining risk window (a fresh mount's pose is marked by a passive React effect, which is not synchronously flushed with the sync-lane commit): - test_get_render_reflects_fresh_node_poses: rapid add-node-at-pose -> immediate-capture cycles with fresh node names (true first mounts), asserting via alpha-centroid side checks that no capture ever shows the box at the default pose or the previous position. Also verified out of band: 150/150 clean captures across remount, first-mount, and 4x CPU-throttled runs. - test_get_render_transport_formats_agree stabilizes shading before comparing formats: environment lighting loads asynchronously after connect, so the previous version compared a pre-lighting JPEG capture against post-lighting raw/PNG captures and flaked bimodally. Async asset loading was never awaited by any capture implementation; the test now waits until two consecutive captures agree before comparing formats.
The "raw" transport added earlier on this branch is split into two formats, and raw_rgb becomes the DEFAULT (previously "jpeg"): - raw_rgb (new default): lossless (H, W, 3) RGB rendered on an opaque white background -- the same shape and background as the old JPEG default, but without compression artifacts and substantially faster (no browser-side encode, no server-side decode). - raw_rgba: lossless (H, W, 4) RGBA on a transparent background, like PNG. Both carry RGBA bytes on the wire; they differ only in the client's clear alpha, and the server drops the (uniformly opaque) alpha channel for raw_rgb with a numpy slice. This keeps the client capture path free of per-pixel repacking. jpeg/png remain available and are recommended for slow or remote links (e.g. share URLs), where uncompressed transfer costs more than the encode/decode it saves. Because raw_rgb preserves the previous default's shape, dtype, and white background, existing callers only see pixel values improve; the docstrings carry a versionchanged note regardless. Tests: the default (raw_rgb on the wire, alpha dropped, writable array) and the raw_rgba round trip are pinned in tests/test_get_render_latency.py; the e2e format-agreement test now covers all four formats, including each format's background convention at off-scene corners and an opaque-masked rgb-vs-rgba comparison (background pixels legitimately differ between the white and transparent conventions, and the center window can catch some).
Defaulting to raw regressed the case the default must be safe for: raw is width * height * 4 bytes on the wire (~8 MB at 1080p, 10-20x jpeg), so callers capturing over slow or remote links (e.g. share URLs) would see a large silent bandwidth regression on upgrade. jpeg stays the default -- still ~1.5x faster than before this branch via the capture-path work -- and raw_rgb / raw_rgba remain documented opt-ins for local connections, where they are the fastest option. The default is pinned by a signature test alongside the existing raw round-trip coverage.
Benchmarks on real render content showed browser-native CompressionStream
("deflate-raw") is ~10x cheaper than toBlob's JPEG/PNG encoders and
compresses typical 3D renders 100-1000x (flat backgrounds, smooth
shading), while Python-side inflate is a few ms via stdlib zlib. That
removes most of the raw formats' bandwidth cost -- ~8 MB per 1080p frame
-- for a few milliseconds of CPU.
Compressing unconditionally would be wrong, though: on incompressible
content (photo textures, dense splats), deflate costs 100-200ms per frame
and saves nothing, and CompressionStream exposes no compression-level
knob to cheapen it. So the client deflates a small strided sample of the
frame first (~3ms) and only compresses the full frame when the sample
compresses beyond 1.5x; otherwise the payload ships uncompressed exactly
as before. A 1-byte payload prefix (0 = raw RGBA, 1 = deflate-raw RGBA)
tells the server which it got; browsers without CompressionStream fall
back to flag 0.
Net effect: raw_rgb / raw_rgba keep their worst-case latency, and their
typical wire cost drops from megabytes to tens of kilobytes -- making
them reasonable beyond localhost, though jpeg remains the safe default
for arbitrary content over arbitrary links.
Unit round trips now cover both payload flags plus corrupt-compressed and
size-mismatched payloads; the e2e capture suite runs against the real
client's adaptive path.
Codec benchmarks on real render content showed JPEG is only best-in-class at one thing -- guaranteed-small payloads on arbitrary content. On typical 3D scenes, losslessly deflating the raw pixels is ~10x cheaper to encode than toBlob's JPEG path (11ms vs 135ms at 720p on a busy compressible scene), a few ms to decode via stdlib zlib, and often produces a smaller payload. Those strengths are complementary, so "auto" combines them and replaces "jpeg" as the default: - Same (H, W, 3) RGB contract on an opaque white background as JPEG. - Frames that deflate at JPEG-competitive rates ship as losslessly compressed pixels: pixel-exact, cheap on both ends (payload flag 1). - Everything else falls back to an actual JPEG encode (flag 2), keeping the payload bounded on arbitrary content and connections. The decision is made before paying for a full-frame canvas readback: small frames deflate outright (bounded cost; a tiny sample would be dominated by deflate's fixed overhead and systematically underestimate compressibility -- caught by the e2e suite), larger frames estimate from 16 sampled rows read as thin getImageData strips, so the JPEG fallback's overhead vs. plain JPEG is a couple of milliseconds rather than a full readback. A final ratio guard bounds payloads when the sample misleads. Benchmarks: parity with plain JPEG on JPEG-favorable content (within environment noise both on a simple box scene and on a dense random point cloud), with lossless output and near-raw speed on compressible scenes. Covered by: unit round trips for both auto payload flags plus the unknown-flag error, a signature test pinning the default with its rationale, and an e2e check that the default returns pixel-exact frames against raw_rgb on compressible content -- proof the lossless path engages end to end.
Simplification of the previous commit: with adaptive deflate compression on the raw wire format, the "auto" format's only remaining contribution was its JPEG fallback for incompressible content -- bought with strip sampling, three decision thresholds, and a third payload flag. Not worth it: raw_rgb already compresses adaptively (and cheaply skips compression when a content sample shows it wouldn't pay), typical scenes ship tens of kilobytes, and callers with incompressible scenes on slow links can still ask for jpeg explicitly. The default becomes raw_rgb: the same (H, W, 3) RGB-on-white contract as the old jpeg default, but lossless and typically faster, with the documented worst case of uncompressed pixels (~8 MB at 1080p) on incompressible content. "auto" is removed from the protocol, client, server, and tests; the default is pinned by a signature test carrying the trade-off rationale.
"raw" stopped being accurate once these formats grew adaptive deflate compression on the wire; the name now says what actually ships. Pure rename -- transport_format values, the wire format literal, and test names -- with no behavioral change. The default is deflate_rgb.
…lity study Measured zlib ratios on captured 720p frames across scene classes: simple geometry 529x at ~13ms fullscreen shaded mesh 77x at ~18ms dense random point cloud 2.1x at 100-270ms noise/photo background 1.7x at ~145ms Two conclusions. First, deflate is slowest exactly where it saves least: in the ~2x regime the compressor's matcher grinds against high-entropy data for ~100ms+ to halve a payload whose transfer costs ~10ms on the local links the deflate formats target. The old 1.5x gate accepted that trade; the gate is now 4x, below which frames ship uncompressed (the CPU-cheap choice) -- callers with such scenes on slow links should be on jpeg anyway, per the docs. Second, a PNG-style per-row delta filter was also measured as a candidate for rescuing gradient-heavy content, and does not improve any scene class (shaded meshes already deflate at 77x without it); noted in a comment so it isn't re-attempted.
The client called canvas.toBlob(cb, format) with no quality argument, so JPEG always encoded at Chrome's 0.92 default and the request's quality field (hardcoded 80) was silently ignored -- measured at 720p, that's ~50% larger payloads (51KB vs 34KB scene-like, 807KB vs 586KB high-entropy) for no speed or fidelity intent. toBlob now receives quality/100 for JPEG (PNG's encoder has no knobs in the web API, which is also why the deflate formats exist as its fast lossless alternative). The knob is exposed as get_render(jpeg_quality=..., default 80), validated to [1, 100] and forwarded through the existing wire field. Measured sweep at 720p: dropping 80 -> 40 roughly halves the payload again and speeds encoding ~20-30%. A unit test pins that the value reaches the wire and that out-of-range values raise.
Measured across the quality sweep at 720p, JPEG's speed and size move strictly together -- lower quality means fewer surviving DCT coefficients, so less entropy-coding work for the encoder AND the decoder (q=0.95 -> 0.4: 22 -> 18ms encode+decode on scene content, 66 -> 38ms on high-entropy content). Higher quality is never faster. There's no speed/size trade-off to expose, so the parameter is gone again; the fixed 80 sits near the speed plateau on typical content while staying fidelity-safe, and is strictly faster and smaller than Chrome's 0.92 toBlob default that the client used to fall into. The client-side fix (actually passing quality to toBlob) stays, pinned by a test asserting the wire carries 80.
Reverts the "auto seems unnecessary, just deflate" simplification: the compressibility study showed exactly the content class that falsifies it. On high-entropy frames (dense colored point clouds, splats, photo textures), deflate either grinds at 100-270ms per 720p frame (~2x ratio) or -- after the 4x gate -- ships multi-MB payloads uncompressed; JPEG handles the same frames in ~40-55ms with bounded size. A speed-motivated default that is slower than JPEG on a real workload class defeats its own purpose, and no fixed choice avoids this: each format has a content class where it loses badly. "auto" is the per-frame choice and is never meaningfully slower than either by construction: row-strip sampling decides compressibility BEFORE the full-frame readback, so the JPEG fallback costs JPEG + ~2ms, while compressible frames (the typical case) take the lossless deflate path at ~13ms instead of JPEG's 30-45ms. Same (H, W, 3)-on-white contract either way. Implementation restored from 84dbb32 with the lessons kept since: deflate_rgb/deflate_rgba naming, JPEG quality honored in the fallback (quality 80, measured faster than Chrome's 0.92 toBlob default on both ends), small frames deflating outright to dodge sample-overhead bias. The default is pinned by a signature test carrying the measured rationale; auto round trips (both payload flags + unknown-flag error) and the e2e pixel-exactness check against deflate_rgb are back.
…d png Removes auto, deflate_rgb, and deflate_rgba -- the transport lineup returns to exactly the pre-branch API: jpeg (default, lossy (H, W, 3) on white) and png (lossless (H, W, 4) on transparent). The adaptive lossless transports were built and measured to a clear verdict, recorded in the default-pin test and the branch history: every fixed lossless choice has a content class where it loses badly (high-entropy frames deflate at ~2x for 100-270ms per 720p frame, or ship as multi-MB payloads), and the per-frame adaptive variant -- while never meaningfully slower by construction -- beat plain JPEG by too little to justify two extra formats, a payload-flag protocol, and sampling thresholds. The bulk of this branch's speedup was never the codec: the capture-path work (same-frame capture, request flush, decode off the event loop, encode overlap, JPEG quality honored) applies to every format and stands, taking the default from ~135ms to ~55ms per 320x240 capture in the benchmark environment.
…splat coverage The capture hook ran at priority -1 -- AFTER the Gaussian splat per-frame hook (-100). Capture calls the splat updateCamera with its virtual camera (material uniforms, camera-space group transforms, blocking sort) and only restores the sorted-index attribute itself; the old pre-branch capture at -100000 was accidentally self-healing because the splat hook re-applied interactive-camera state later in the same frame. At -1 that repair came a frame late, so every capture let the frame's visible render draw the interactive splat viewport with the capture's state -- a tight capture loop corrupted the on-screen view continuously. Verified empirically with an in-page rAF probe on the splat material's viewport uniform: the capture's size leaks across frame boundaries at -1 and can never be observed at the fix. Capture now runs at -500: still after SceneTree's pose appliers (-1000), which the same-frame capture design requires, and before the splat hook (-100), which restores the same-frame repair. The probe is promoted to an e2e regression test, which also gives splat scenes their first capture coverage. Also removes two leftovers from the transport experiments: a dead ternary branch in the request construction and a stale comment pointer in ViewerContext.ts.
Four findings from an independent review of the branch's net diff, in severity order: - The capture hook's defensive pose sweep guarded nodeRefFromName entries with === undefined, but React ref callbacks write NULL on detach (the map's type just doesn't admit it), e.g. for hidden unmountWhenInvisible nodes -- transform controls, 3D GUI containers. A pose update to such a hidden node then made the sweep throw a TypeError, failing the entire capture with a spurious "could not capture a frame" on a healthy client (intermittently: the first failure marks the pose applied). Guard is now == null; pinned by an e2e test (hide transform controls, move them, capture) verified to fail against the undefined-only guard. - infra register_handler used an unsynchronized check-then-create for the handler list; two threads issuing the first-ever concurrent get_render() calls on a connection could each create the list, the second discarding the first thread's handler -- that caller then hangs until timeout. Now dict.setdefault (atomic under the GIL). Pre-existing, but this branch explicitly supports concurrent get_render(). - test_get_render_flushes_broadcast_buffer_first claimed to pin flush-before-request ordering but only asserted that a flush happened within 2s of the call; a flush anywhere later in get_render() would have passed. The flush call is now recorded in-band into the same log as the queued request and the ordering asserted directly. - The broadcast-flush comment claimed the request "must not overtake" scene updates; the two buffers are drained by independent producer tasks, so that is best-effort, not a guarantee. Reworded to say what the flush actually buys (rare instead of routine overtaking).
…ited Captures reflect scene STATE updates made before the call (pinned by the e2e suite), but content the browser decodes or loads asynchronously -- large background/image textures, GLB assets, environment maps -- has never been awaited by any version of the capture path. Probed empirically: immediate captures picked up just-set background images at every tested size here (decode completes within the one-frame commit wait), but the window is real on fast displays where frame gaps are short relative to decode time, and GLB/environment loads can straggle by hundreds of milliseconds regardless. GPU-side texture uploads are not affected (three.js uploads pending textures synchronously inside the capture's own render). A docstring note now states the boundary.
Fixes the Python 3.8 CI failure in
test_get_render_request_skips_windowing_delay: before Python 3.10,
asyncio.Event() binds its event loop at construction time, so building
an AsyncMessageBuffer on the pytest thread attached its events to the
test thread's loop while the window generator awaited them on the
server's loop ("got Future attached to a different loop"). The buffer is
now constructed via run_coroutine_threadsafe on the server loop, exactly
as the real client path does inside the websocket handler. No product
code change.
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
1 participant
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.
Speed up
get_render()~2.5x by flushing render requests past message-buffer windowing, capturing in the same frame that scene updates are applied, and moving image encode/decode off the event-loop and frame-loop hot paths, plus fixes for concurrent-capture races and new unit/e2e coverage.