From 8f68c05997deabb73c8c4d7dedad87fcc87e019f Mon Sep 17 00:00:00 2001 From: philippe Date: Fri, 6 Nov 2026 10:40:15 -0500 Subject: [PATCH 1/2] Only reset descendant layout hashes when there is one to clear Every wrapper in a re-rendered callback subtree dispatched resetComponentState, and each dispatch re-ran every wrapper's selector, so updates were O(n^2) in the subtree size (#4121). Skip the dispatch when no hash is under the path, and match whole path segments so resetting children,1 no longer clears children,10. The benchmark missed this because full_children_replace was written off as slow by design with a 12 s ceiling. Tighten it, add a growth gate and a rerender_existing_subtree scenario, and drop its stale baseline entry. --- .ai/PERFORMANCE.md | 22 ++++-- CHANGELOG.md | 5 ++ benchmarks/baseline.json | 12 +-- benchmarks/run.py | 7 +- benchmarks/scenarios.py | 73 +++++++++++++++++-- dash/dash-renderer/src/actions/index.js | 17 +++++ dash/dash-renderer/src/reducers/reducer.js | 12 +-- .../dash-renderer/src/wrapper/DashWrapper.tsx | 8 +- dash/dash-renderer/src/wrapper/wrapping.ts | 6 ++ dash/dash-renderer/tests/layoutHashes.test.js | 29 ++++++++ tests/integration/renderer/test_redraw.py | 42 +++++++++++ 11 files changed, 198 insertions(+), 35 deletions(-) create mode 100644 dash/dash-renderer/tests/layoutHashes.test.js diff --git a/.ai/PERFORMANCE.md b/.ai/PERFORMANCE.md index 4b32850800..32de69d448 100644 --- a/.ai/PERFORMANCE.md +++ b/.ai/PERFORMANCE.md @@ -138,7 +138,8 @@ function dominates), not the absolute ms. | callback_fanout (1 -> 300) | ~45 ms | 1.0x | fine | | callback_chain (100 deep) | ~440 ms | 1.0x | ~100 sequential dispatches; inherent | | wildcard_all_resolve (ALL over 400) | ~140 ms | 1.0x | was ~580 ms (O(n²)); now O(n), see below | -| full_children_replace (contrast) | ~850 ms | ~18x | O(total) every click *by design* - why Patch exists | +| full_children_replace (200 -> 2000) | ~50 ms | ~3x | linear in the list it rebuilds; was ~18x (O(n^2)), see below | +| rerender_existing_subtree (400 rows) | ~50 ms | 1.0x | was ~1200 ms (O(n^2)); see below | ### 1. Wildcard (ALL / MATCH) resolution — was O(n²), now O(n) [fixed] @@ -189,11 +190,22 @@ equivalent to the curried `path`, just without the dispatch and placeholder checks. When touching these traversals, prefer direct access over curried ramda - the per-node multiplier makes it matter. -### 4. Full children replacement is the O(total) baseline +### 4. Re-rendering callback children was O(n^2) [fixed] -`full_children_replace` grows ~18x across a run and is ~15x slower than the -equivalent `Patch().extend()`. This is expected and is the reason `Patch` -exists for growing containers; it is kept as a contrast with loose thresholds. +`full_children_replace` used to grow ~18x across a run and was written off as +the expected cost of not using `Patch`. It was not: every wrapper in a +re-rendered subtree dispatched `resetComponentState` from its `_newRender` +effect, and each dispatch re-runs every mounted wrapper's selector. 400 rows (1200 +components) took 1219 store updates per click and ~1200 ms (#4121). + +**Fix:** `resetDescendantHashes` checks `layoutHashes` first and only dispatches +when there is a hash under the wrapper's path to clear. The reducer also returns +the same object when nothing matches, and matches path segments, so resetting +`children,1` no longer clears `children,10`. Now 19 updates and ~50 ms. + +`rerender_existing_subtree` covers the same-size case, which the first-render +guard from #3929 does not skip. `max_growth` on `full_children_replace` fails +the run when the per-click cost grows faster than the list (linear is ~3x). ### 5. Layouts deeper than ~250 nested components fail to serialize diff --git a/CHANGELOG.md b/CHANGELOG.md index d752b883a1..006e382da9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to `dash` will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/). +## [UNRELEASED] + +### Fixed +- [#]() Re-rendering callback-returned children no longer dispatches one store update per component, which made updates O(n^2) in the subtree size. Fixes [#4121](https://github.com/plotly/dash/issues/4121). + ## [4.5.0] - 2026-10-29 ### Added diff --git a/benchmarks/baseline.json b/benchmarks/baseline.json index 4be5242a55..371fb469d0 100644 --- a/benchmarks/baseline.json +++ b/benchmarks/baseline.json @@ -49,16 +49,6 @@ "growth": 3.34 } }, - "full_children_replace": { - "replace_ms": { - "n": 10, - "min": 153.5, - "median": 1822.9, - "p90": 4697.1, - "max": 5768.9, - "growth": 15.93 - } - }, "patch_scalar_update_large": { "update_ms": { "n": 12, @@ -115,4 +105,4 @@ "growth": 1.0 } } -} \ No newline at end of file +} diff --git a/benchmarks/run.py b/benchmarks/run.py index 5fcc2bbaaa..0c7e2e64e0 100644 --- a/benchmarks/run.py +++ b/benchmarks/run.py @@ -348,7 +348,8 @@ def gate(results, scenarios, baseline=None): A metric fails on the absolute fail_ms ceiling, or (if a baseline exists) on a >2x regression vs baseline p90 after normalizing out machine speed (see - ``machine_scale``). It warns on warn_ms, or a >1.3x normalized baseline + ``machine_scale``), or when its growth stat exceeds the scenario's + max_growth. It warns on warn_ms, or a >1.3x normalized baseline regression. The baseline-ratio check is skipped for metrics whose baseline p90 is below ``MIN_BASELINE_MS`` (sub-ms metrics are pure timer jitter; the absolute ceilings still guard them). ``scale`` is the machine factor that @@ -372,6 +373,10 @@ def gate(results, scenarios, baseline=None): elif warn_ms is not None and p90 > warn_ms: level = "warn" reasons.append(f"p90 {p90}ms > warn {warn_ms}ms") + max_growth = sc.max_growth.get(metric) + if max_growth is not None and stats["growth"] > max_growth: + level = "fail" + reasons.append(f"growth {stats['growth']}x > {max_growth}x") base_p90 = base.get(metric, {}).get("p90") if base_p90 and base_p90 >= MIN_BASELINE_MS: # On a 3x-slower runner every raw p90 is ~3x its baseline, so diff --git a/benchmarks/scenarios.py b/benchmarks/scenarios.py index 1acd41ae46..3509bad1ae 100644 --- a/benchmarks/scenarios.py +++ b/benchmarks/scenarios.py @@ -17,7 +17,9 @@ Thresholds are deliberately generous (see ``warn_ms`` / ``fail_ms``): this is a signal for "something got materially slower", not a microbenchmark. ``fail_ms`` is meant to catch an order-of-magnitude regression; ``warn_ms`` flags a smaller -drift that is worth a look and gets surfaced as a PR comment. +drift that is worth a look and gets surfaced as a PR comment. ``max_growth`` +fails a scenario whose per-click cost grows faster than its workload does, which +is how an O(N^2) shows up when the absolute times still look tolerable. """ from __future__ import annotations @@ -59,6 +61,8 @@ class Scenario: # Thresholds keyed by metric name. Missing metric => not gated. warn_ms: dict = field(default_factory=dict) fail_ms: dict = field(default_factory=dict) + # Ceiling on the "growth" stat (late-third / early-third per-op time). + max_growth: dict = field(default_factory=dict) # How many times to repeat the measured interaction (per process launch) # and how many leading runs to discard as warm-up. repeats: int = 12 @@ -241,19 +245,76 @@ def _drive_full_replace(b, params): scenario( - # Intentionally the slow path: returning the whole list is O(total) every - # click, unlike Patch. Kept as a reference contrast, so its thresholds are - # loose - we only want to catch it getting *even* slower. + # Each click returns a list `batch` longer than the last, so the cost per + # click is linear in the list size and growth lands near 3x over a run. An + # O(N^2) renderer path shows up here as growth well above that (#4121). name="full_children_replace", description="Rebuild the whole children list from a callback each click", params={"batch": 200}, - warn_ms={"replace_ms": 6000}, - fail_ms={"replace_ms": 12000}, + warn_ms={"replace_ms": 400}, + fail_ms={"replace_ms": 1500}, + max_growth={"replace_ms": 6}, repeats=10, warmup=1, )((_build_full_replace, _drive_full_replace)) +# =========================================================================== +# 4b. Re-render a subtree that is already on the page +# =========================================================================== + + +def _build_rerender(params): + app = Dash(__name__) + app.layout = html.Div( + [html.Button("go", id="btn", n_clicks=0), html.Div(id="container"), READY] + ) + + @app.callback(Output("container", "children"), Input("btn", "n_clicks")) + def render(n): + return [ + html.Div( + [html.Span(f"label {i}"), html.Span(f"value {i} @ {n}")], + className="row", + ) + for i in range(params["rows"]) + ] + + return app + + +def _rendered(click): + return ( + "document.getElementById('container').lastElementChild" + f"?.lastElementChild?.textContent.endsWith('@ {click}')" + ) + + +def _drive_rerender(b, params): + click = b.state.get("click", 0) + 1 + if click == 1: + b.timed("", _rendered(0)) + ms = b.timed("document.getElementById('btn').click()", _rendered(click)) + b.state["click"] = click + return {"rerender_ms": ms} + + +scenario( + # Same-size subtree every click, so the cost should be flat. Each wrapper + # in a re-rendered subtree used to dispatch a store update, making this + # O(N^2): ~1200 ms at 400 rows against ~50 ms now (#4121). The size never + # changes, so growth can't see that; fail_ms is what catches it. + name="rerender_existing_subtree", + description="Re-render a 400-row (1200 component) subtree already on the page", + params={"rows": 400}, + warn_ms={"rerender_ms": 400}, + fail_ms={"rerender_ms": 800}, + max_growth={"rerender_ms": 3}, + repeats=12, + warmup=2, +)((_build_rerender, _drive_rerender)) + + # =========================================================================== # 5. Patch scalar update into a large list (in-place value change) # =========================================================================== diff --git a/dash/dash-renderer/src/actions/index.js b/dash/dash-renderer/src/actions/index.js index 247c2cea59..01b1ee3ebd 100644 --- a/dash/dash-renderer/src/actions/index.js +++ b/dash/dash-renderer/src/actions/index.js @@ -13,6 +13,7 @@ import { } from './dependencies_ts'; import {computePaths, getPath} from './paths'; import {recordUiEdit} from '../persistence'; +import {isPathOrDescendant, stringifyPath} from '../wrapper/wrapping'; export const onError = createAction(getAction('ON_ERROR')); export const setAppLifecycle = createAction(getAction('SET_APP_LIFECYCLE')); @@ -31,6 +32,22 @@ export const resetComponentState = createAction( getAction('RESET_COMPONENT_STATE') ); +// Called by every wrapper in a re-rendered subtree, so only dispatch when +// there is something to clear: each dispatch re-runs every wrapper's +// selector, which made subtree updates O(N^2) (#4121). +export function resetDescendantHashes(itempath) { + return (dispatch, getState) => { + const prefix = stringifyPath(itempath); + if ( + Object.keys(getState().layoutHashes).some(key => + isPathOrDescendant(key, prefix) + ) + ) { + dispatch(resetComponentState({itempath})); + } + }; +} + export function updateProps(payload) { return (dispatch, getState) => { const component = path(payload.itempath, getState().layout); diff --git a/dash/dash-renderer/src/reducers/reducer.js b/dash/dash-renderer/src/reducers/reducer.js index 759c198989..ad2f7a79c9 100644 --- a/dash/dash-renderer/src/reducers/reducer.js +++ b/dash/dash-renderer/src/reducers/reducer.js @@ -18,7 +18,7 @@ import layout from './layout'; import paths from './paths'; import callbackJobs from './callbackJobs'; import loading from './loading'; -import {stringifyPath} from '../wrapper/wrapping'; +import {isPathOrDescendant, stringifyPath} from '../wrapper/wrapping'; export const apiRequests = [ 'dependenciesRequest', @@ -53,12 +53,12 @@ const layoutHashes = (state = {}, action) => { const {itempath} = action.payload; if (itempath) { const prefixStr = stringifyPath(itempath); - // Remove all hashes for keys starting with prefixStr - return Object.fromEntries( - Object.entries(state).filter( - ([key]) => !key.startsWith(prefixStr) - ) + const kept = Object.entries(state).filter( + ([key]) => !isPathOrDescendant(key, prefixStr) ); + if (kept.length !== Object.keys(state).length) { + return Object.fromEntries(kept); + } } } return state; diff --git a/dash/dash-renderer/src/wrapper/DashWrapper.tsx b/dash/dash-renderer/src/wrapper/DashWrapper.tsx index 540f172031..e4f6d1a7ea 100644 --- a/dash/dash-renderer/src/wrapper/DashWrapper.tsx +++ b/dash/dash-renderer/src/wrapper/DashWrapper.tsx @@ -31,7 +31,7 @@ import { notifyObservers, onError, updateProps, - resetComponentState + resetDescendantHashes } from '../actions'; import {getWatchedKeys, stringifyId} from '../actions/dependencies'; import { @@ -505,11 +505,7 @@ function DashWrapper({ // least once, so reset from the second fresh render onward. // (#3929, keeps #3330 fixed.) if (hasFreshRendered.current) { - dispatch( - resetComponentState({ - itempath: componentPath - }) - ); + dispatch(resetDescendantHashes(componentPath) as any); } hasFreshRendered.current = true; } diff --git a/dash/dash-renderer/src/wrapper/wrapping.ts b/dash/dash-renderer/src/wrapper/wrapping.ts index 6a443990b6..74babfb372 100644 --- a/dash/dash-renderer/src/wrapper/wrapping.ts +++ b/dash/dash-renderer/src/wrapper/wrapping.ts @@ -55,6 +55,12 @@ export function stringifyPath(layoutPath: DashLayoutPath) { return join(',', layoutPath); } +// `strPath` is `prefix` or one of its descendants. A bare startsWith would +// also match siblings: 'children,1' is a prefix of 'children,10'. +export function isPathOrDescendant(strPath: string, prefix: string) { + return strPath === prefix || strPath.startsWith(prefix + ','); +} + export function getComponentLayout( componentPath: DashLayoutPath, state: any diff --git a/dash/dash-renderer/tests/layoutHashes.test.js b/dash/dash-renderer/tests/layoutHashes.test.js new file mode 100644 index 0000000000..d1407c778d --- /dev/null +++ b/dash/dash-renderer/tests/layoutHashes.test.js @@ -0,0 +1,29 @@ +import {expect} from 'chai'; +import {describe, it} from 'mocha'; +import {createReducer} from '../src/reducers/reducer'; + +const hashes = { + 'a,children,1': {hash: 1}, + 'a,children,1,props,children': {hash: 1}, + 'a,children,10': {hash: 1} +}; + +const reset = itempath => ({ + type: 'RESET_COMPONENT_STATE', + payload: {itempath} +}); + +describe('RESET_COMPONENT_STATE', () => { + const reducer = createReducer(); + const state = reducer({layoutHashes: hashes}, {type: '@@INIT'}); + + it('clears the path and its descendants but not siblings', () => { + const next = reducer(state, reset(['a', 'children', 1])); + expect(Object.keys(next.layoutHashes)).to.deep.equal(['a,children,10']); + }); + + it('keeps the same object when nothing matches', () => { + const next = reducer(state, reset(['b'])); + expect(next.layoutHashes).to.equal(state.layoutHashes); + }); +}); diff --git a/tests/integration/renderer/test_redraw.py b/tests/integration/renderer/test_redraw.py index de79487e6d..9fdccb8528 100644 --- a/tests/integration/renderer/test_redraw.py +++ b/tests/integration/renderer/test_redraw.py @@ -169,3 +169,45 @@ def update(_plain, _remount): dash_duo.wait_for_text_to_equal("#counter", "1") assert dash_duo.get_logs() == [] + + +def test_rdraw005_children_update_store_dispatches(dash_duo): + # Re-rendering a callback-returned subtree used to dispatch one store + # update per component in it, each re-running every wrapper's selector: + # O(N^2) in the subtree size. Regression test for #4121. + app = Dash() + n_rows = 100 + + app.layout = html.Div( + [ + html.Button("Re-render", id="btn", n_clicks=0), + html.Div(id="content"), + ] + ) + + @app.callback(Output("content", "children"), Input("btn", "n_clicks")) + def render(n): + return [ + html.Div( + [html.Span(f"Field {i}"), html.Span(f"value {i} @ click {n}")], + className="row", + ) + for i in range(n_rows) + ] + + dash_duo.start_server(app) + last = "#content .row:last-child span:last-child" + dash_duo.wait_for_text_to_equal(last, f"value {n_rows - 1} @ click 0") + + dash_duo.driver.execute_script( + "window.__updates = 0;" "window.store.subscribe(() => window.__updates++);" + ) + dash_duo.find_element("#btn").click() + dash_duo.wait_for_text_to_equal(last, f"value {n_rows - 1} @ click 1") + # Wrapper effects run after paint, so let a couple of frames pass. + updates = dash_duo.driver.execute_async_script( + "const done = arguments[0];" + "requestAnimationFrame(() => requestAnimationFrame(" + "() => setTimeout(() => done(window.__updates), 50)));" + ) + assert updates < 40, updates From d82c243d64a07db00204bb51d165caf6a34fe132 Mon Sep 17 00:00:00 2001 From: philippe Date: Fri, 6 Nov 2026 10:40:42 -0500 Subject: [PATCH 2/2] Add PR number to CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 006e382da9..e08c41e193 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). ## [UNRELEASED] ### Fixed -- [#]() Re-rendering callback-returned children no longer dispatches one store update per component, which made updates O(n^2) in the subtree size. Fixes [#4121](https://github.com/plotly/dash/issues/4121). +- [#4124](https://github.com/plotly/dash/pull/4124) Re-rendering callback-returned children no longer dispatches one store update per component, which made updates O(n^2) in the subtree size. Fixes [#4121](https://github.com/plotly/dash/issues/4121). ## [4.5.0] - 2026-10-29