Gate Plotter.compute on active-consumer interest#944
Closed
SimonHeybrock wants to merge 6 commits into
Closed
Conversation
Plotter.compute() used to run unconditionally on the background ingestion thread for every layer on every Kafka delta, regardless of whether any tab was showing the layer or whether a browser was even connected. For long-running, data-heavy plots (timeseries at N=600k, 1k x 1k detector images) this is O(N) per-tick work on a thread shared with JobOrchestrator and stale-session cleanup — the hard floor identified in #940. Split Plotter.compute into a lazy entry point + a _build override point. In managed mode the heavy _build only runs while at least one consumer has acquired interest via set_active(token, True); otherwise the latest input is stashed and a dirty flag is set. The 0→1 transition rebuilds synchronously, so the same polling pass that flips a tab to active sees the fresh cached state. Interest is tracked via opaque tokens on the plotter rather than via presenters, because presenter creation already depends on has_cached_state and tying the two together would deadlock the first build. Plotters constructed outside the orchestrator (unit tests, scripts) stay eager by default. The orchestrator calls set_active(self, False) at registration time to switch production plotters into managed mode, so the gating is in effect from the moment data starts flowing — including the no-browser case. Tab-switch latency microbenchmarks: 4.4 ms for one LinePlotter @ N=100k, 9 ms for one ImagePlotter @ 1024², 43 ms for ten LinePlotters @ N=100k. Well below the perceptible-freeze threshold for realistic plot counts.
- Drop _managed dual-mode. Compute now always gates on active tokens. Tests construct plotters directly and must acquire an interest token (set_active(object(), True)) before compute() to observe a build. Removes ambient-mode coupling where production semantics depended on whether an unrelated method had been called first. - Lock _pending/_dirty writes in compute() so the threading model no longer depends on GIL ordering being load-bearing across separate statements. - Drop hasattr(plotter, 'set_active') hedges. All registered plotters now respond to the interest knob; new plotters that don't will fail fast. - ROI _build returns None (drop speculative return primary). - Log exceptions from default _build instead of silently substituting an Error text element. - Fix the broken test_overlay_plotter_compute fixture (was passing PlotParams3d to Overlay1DPlotter.from_params which expects PlotParams1d).
The wrapper duck-typed Plotter but skipped the gating, so compute()'s histogram step ran eagerly per Kafka delta even when no consumer was watching the renderer. Mirror the same lazy machinery: stash extracted data + renderer's active-token set as the gate, deferring the histogram build until set_active(token, True) flips the renderer from inactive to active. Validation (presence of primary + axis roles) stays eager so bad input still raises synchronously at the orchestrator; only the heavy histogram + renderer build is deferred. The stale plan-doc reference in the set_active docstring is gone as a result.
The inline release of the previous plotter duplicated _release_plotter_interest. Calling it directly keeps the release path unified across orphan cleanup, shutdown, and plotter replacement.
Mirroring the full lazy machinery on a non-Plotter wrapper added more code than the gain justified — the histogram step here is modest compared with the renderer's element build, which is what set_active on the wrapper already defers via the renderer. Restores the previous forwarder shape; just refreshes the docstring to no longer reference a plan doc that doesn't exist in tree.
The two static helpers on PlotGridTabs (sync/release plotter interest) operate entirely on SessionLayer.active_plotter and don't touch any tabs-widget state — they're @staticmethod precisely because they don't need self. Make them public methods on SessionLayer, the object whose state they mutate. The tabs widget calls them directly. Drops the lazy_compute_widget_test.py file that was testing private PlotGridTabs methods; equivalent coverage now lives in session_layer_test.py against the public SessionLayer API.
3 tasks
SimonHeybrock
added a commit
that referenced
this pull request
Jun 3, 2026
Alternative shape to #944: same lazy-compute semantics, but the gate lives on LayerStateMachine instead of inside Plotter. Plotter stays a pure compute artifact -- no set_active, no compute/_build split, no internal locking. Every Plotter subclass, static/correlation special-cases, and ~30 plotter test fixtures are left untouched. LayerStateMachine gains a token set, pending input stash, and gate lock. stash_pending() and set_active() return a ComputeTask when a build should run now; callers (PlotOrchestrator._run_compute and the new activate_layer entry point) invoke the task outside any lock. The 0->1 token transition flushes synchronously on the polling thread so the same poll pass sees fresh has_cached_state. Plotter replacement via job_started clears the stash so old input cannot be flushed through a new plotter. The polling loop in plot_grid_tabs now drives orchestrator.activate_layer(layer_id, session_layer, is_active) once per (grid, layer); orphan and shutdown paths release tokens. Test migration is one helper update (add_cell_with_layer activates the new layer) plus two inline activate_layer calls in tests that bypass the helper. Plotter unit tests are unchanged. 8 new gate tests cover stash-without-token, 0->1 flush, intermediate-update collapse, multi-token ref counting, plotter replacement, and pre-job_started stash behaviour.
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.
Motivation
Plotter.compute()runs on the background ingestion thread for every layer on every Kafka delta — regardless of whether the layer's grid tab is visible, and regardless of whether any browser is even connected. Onmainthis is ~5-8 ms per plot at N=100k (extract + autoscaler +hv.Curvebuild), scaling linearly in plot count, on a thread shared withJobOrchestratorand stale-session cleanup. This is the "even with no browser, the dashboard gets laggy" floor identified in the issue #940 investigation.This PR does not fix the dominant freeze symptom — that's the ~100 ms-per-call
pipe.sendon the live document, which needs downsampling (issue #940 PR #942) and/or HoloViews patch-streaming. The value here is twofold: it removes the bg-thread floor for hidden tabs / no-browser sessions, and — more importantly — it is the foundation that lets the downsample in #942 land safely. Without lazy compute, the downsample doubles per-tick cost at N=600k, and that doubled cost is paid for every hidden tab.What changed
Plotter.compute()is now a lazy submission point: it stashes the latest input and only invokes the heavy_buildstep while at least one consumer has acquired interest viaplotter.set_active(token, True). The 0→1 active transition rebuilds synchronously from the stashed input, so a tab switch sees fresh state on the same polling pass that creates its presenter.Interest is tracked via opaque tokens on the plotter (not via the presenter
WeakSet): presenter creation already depends onhas_cached_state, so tying the two together would deadlock the first build. The_poll_for_plot_updatesloop inwidgets/plot_grid_tabs.pydrivesset_activeper(grid, layer), using theSessionLayeritself as the token.Subclasses override
_buildrather thancompute, so the gating is inherited uniformly acrossLinePlotter,ImagePlotter,Overlay1DPlotter,SlicerPlotter,BaseROIRequestPlotter, andFlattenPlotter.CorrelationHistogramPlotter(a duck-typed wrapper around a renderer) forwardsset_activeto its renderer — the renderer's element build is gated, but the wrapper's own histogram step still runs per data tick. The histogram step is modest compared to the renderer's element build, so the asymmetry is acceptable.Trade-offs
Cost shifts from "continuous bg-thread work for every layer" to "one-shot synchronous rebuild on tab activation". Measured tab-switch latencies (pytest-benchmark, dev container):
All well below the ~100 ms perceptible-freeze threshold, even at 10 busy plots on a single tab.
Notes
Tests that construct plotters directly now have to acquire an interest token before
compute()to observe a build — production semantics are no longer hidden behind a test-only eager-default mode. The migration is mechanical (plotter.set_active(object(), True)in fixtures / before inlinecompute()calls) and improves the parity between tests and production behaviour.A bug in
test_overlay_plotter_compute(passingPlotParams3dtoOverlay1DPlotter.from_params, which wantsPlotParams1d) is fixed as a drive-by.Test plan
🤖 Generated with Claude Code