perf(FragmentsModels): skip view refresh when the view is unchanged and coalesce forced updates - #283
Open
rihokirss wants to merge 2 commits into
Open
Conversation
… unchanged Every update() dispatched REFRESH_VIEW to every model unconditionally, and worker-side setupView() always reset the cull pass — so each worker re-culled every sample of every model roughly every maxUpdateRate ms forever, even with a fully idle camera: the FINISH emitted by each pass scheduled the next update(), sustaining the loop with no user input. With one model this is barely visible; with N models on N workers it burns several cores at idle. Measured with 13 real IFC-derived models (headless Chromium): ~660 REFRESH_VIEW + ~700 tile batches per 15 s and ~2 cores continuously busy while nothing changed on screen. Changes: - ViewManager fingerprints the outgoing view (frustum planes, camera position in model space, clipping planes, viewport size, graphics quality, model placement) and skips the REFRESH_VIEW RPC when nothing changed since the last dispatch. Forced updates always dispatch because their FINISH acts as the completion fence for forceUpdateFinish. - Worker-side setupView() compares the incoming view against the current one and skips restart() when identical, so a forced update with an unchanged view (e.g. the fence issued after loading another model) no longer re-culls everything. If the previous pass already finished it emits a FINISH directly so fences still settle; an in-flight pass keeps running and its natural FINISH carries the new seq because the stamp is read at emission time. graphicThreshold is deliberately excluded from both comparisons — a budget change must not trigger a full re-cull, and the stored view still adopts the new value. - FragmentsModels.update() reschedules itself with a light timer so view-change detection stays alive without worker traffic (previously the loop was sustained by the FINISH-driven update event, which no longer fires when idle), and force now bypasses the maxUpdateRate throttle instead of being silently dropped. - FragmentsModel._isProcessing is only set when a refresh was actually dispatched, so isBusy cannot get stuck on skipped refreshes. A/B with 13 models, 15 s idle window: worker RPCs 1358 -> 0..31, CPU time of the browser process tree 31.8 s -> 11.0 s. Interaction cost is unchanged (a real view change still triggers the same full pass), and visibility/highlight/edit changes still reach the screen because those paths restart the worker pass themselves. Verified via functional tests: hide/highlight without camera movement, update(true) fence after an unrelated RPC (no hang), camera move and clipping-plane changes picked up.
… instead of bypassing it Letting update(true) skip maxUpdateRate turned every camera-controls "rest" event (the components layer forces an update on it) into a full re-cull of all models plus an unbounded tile drain on the main thread — during a programmatic orbit that is nearly every frame. Forced calls that land inside the window are now merged into a single trailing forced update; awaiting callers resolve when that one has settled, so the fence semantics are unchanged.
This was referenced Sep 4, 2026
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.
Part of #279 (the idle re-cull loop and the forced-update handling; the per-worker cache budget and the ThreadUpdater fairness are separate).
Two commits, they belong together — the second is what makes the first safe under a real app's update pattern.
1. Skip the refresh when the view has not changed.
ViewManagerfingerprints the outgoing view (frustum planes, camera position in model space, clipping planes, viewport size, graphics quality, model placement) and skips theREFRESH_VIEWRPC when nothing changed. Worker-sidesetupView()compares the incoming view against the current one and skips the full re-cull when identical, emitting a FINISH directly when its pass is already complete, soupdate(true)fences still settle; an in-flight pass keeps running and its natural FINISH carries the new seq.graphicThresholdis deliberately excluded from both comparisons so a budget change cannot trigger a re-cull. Because an idle scene then produces no worker traffic — and therefore no FINISH to drive the nextupdate()—update()reschedules itself with a light timer instead._isProcessingis only set when a refresh was actually dispatched, soisBusycannot stick on a skipped refresh.2. Coalesce forced updates instead of dropping them or letting them through. Today
update(true)insidemaxUpdateRatereturns early: the caller awaits a fence that never happened. Simply exempting forced calls from the throttle is worse in practice —@thatopen/componentsforces an update on every camera-controlsrest, which fires on nearly every frame of a programmatic orbit, and each forced refresh is a full re-cull plus an unboundeddrainAll()on the main thread (I measured −10 % orbit FPS and 3× the long-task time with that variant). Forced calls landing inside the window are now merged into a single trailing forced update, and every awaiting caller resolves when that one has settled — which covers all the RPCs they could have been waiting for. Fence semantics are unchanged.Measurements
Headless Chromium, Radeon 780M, 1920×1080, medians of 3 runs, measured against a baseline run interleaved with it (this box thermally throttles over a long chain, so a baseline from 40 minutes earlier is not comparable).
REFRESH_VIEWper 5 s idle, 1 model / 8 modelsAn older measurement of the same idle-loop fix on 13 models (headless, SwiftShader): 15 s idle went from 1 358 worker RPCs and 31.8 s of browser-process CPU to 0–31 RPCs and 11.0 s.
Screenshots of six fixed poses are pixel-identical. Visibility and highlight changes, camera moves, clipping-plane changes and
update(true)fences verified against the current build.The remaining piece of #279 — dividing the invisible-tile cache budget by the active worker count — is now #286. I first assumed it depended on this PR; re-measuring it against an interleaved baseline showed it is neutral on frame time on its own, so the two are independent and can go in either order.
Also from #279: #284 (ThreadUpdater fairness) and #286 (per-worker tile-cache budget).
🤖 Generated with Claude Code
https://claude.ai/code/session_013rtNQqhSQRM2t6E98DESNE