diff --git a/news/6739.performance.md b/news/6739.performance.md new file mode 100644 index 00000000000..dbc358c13cb --- /dev/null +++ b/news/6739.performance.md @@ -0,0 +1 @@ +Event chaining (`yield OtherState.handler(rows)`) no longer deep-copies payload values that are not attached to any state: only state-bound `MutableProxy` subtrees are copied, making proxy-free payloads ~5x faster to chain. diff --git a/packages/reflex-base/news/6739.performance.md b/packages/reflex-base/news/6739.performance.md new file mode 100644 index 00000000000..dbc358c13cb --- /dev/null +++ b/packages/reflex-base/news/6739.performance.md @@ -0,0 +1 @@ +Event chaining (`yield OtherState.handler(rows)`) no longer deep-copies payload values that are not attached to any state: only state-bound `MutableProxy` subtrees are copied, making proxy-free payloads ~5x faster to chain. diff --git a/packages/reflex-base/src/reflex_base/event/__init__.py b/packages/reflex-base/src/reflex_base/event/__init__.py index ff94adde754..919e1f2fd1f 100644 --- a/packages/reflex-base/src/reflex_base/event/__init__.py +++ b/packages/reflex-base/src/reflex_base/event/__init__.py @@ -144,16 +144,11 @@ def from_event_type( msg = f"Unexpected event type, {type(e)}." raise ValueError(msg) name = format.format_event_handler(e.handler) - # Deepcopy mutable values to detach them from any state-bound - # proxies (e.g. ImmutableMutableProxy from a background task's - # StateProxy). + # Detach mutable values from any state-bound proxies (e.g. + # ImmutableMutableProxy from a background task's StateProxy), + # copying only subtrees that are actually proxied. payload = { - k._js_expr: ( - decoded - if isinstance(decoded := v._decode(), _IMMUTABLE_PAYLOAD_TYPES) - else copy.deepcopy(decoded) - ) - for k, v in e.args + k._js_expr: _detach_state_proxies(v._decode()) for k, v in e.args } # Create an event and append it to the list. @@ -184,6 +179,117 @@ def from_event_type( type(None), ) +# Exact-type fast path for the scalar scan below; isinstance against the +# _IMMUTABLE_PAYLOAD_TYPES tuple is ~5x slower per element. +_SCALAR_PAYLOAD_TYPES = frozenset(_IMMUTABLE_PAYLOAD_TYPES) + + +class _PayloadCycleError(Exception): + """Internal: a payload container references itself.""" + + +def _detach_state_proxies(value: Any) -> Any: + """Detach state-bound proxies from an event payload value. + + Subtrees containing no proxies are passed through by reference. Mutable + values that are not plain containers are deep-copied whole, as before: + this covers state-bound MutableProxy values (whose exact type is never a + plain container type, and whose ``__deepcopy__`` unwraps the proxy and + snapshots the wrapped data) as well as opaque objects that may reference + proxies internally. + + Args: + value: The decoded payload value to detach. + + Returns: + The value with every state-bound proxy replaced by a detached copy. + """ + try: + return _scan_detach(value, {}, set()) + except _PayloadCycleError: + # Self-referential containers: deepcopy preserves cycles via its memo. + return copy.deepcopy(value) + + +def _scan_detach(value: Any, memo: dict[int, Any], active: set[int]) -> Any: + """Recursive copy-on-write scan behind ``_detach_state_proxies``. + + Args: + value: The payload value (or subtree) to detach. + memo: Deepcopy memo shared across one payload argument, preserving + aliasing between the copied subtrees of that argument. + active: ids of the containers on the current descent path, used to + detect self-referential payloads. + + Returns: + The value with every state-bound proxy replaced by a detached copy. + + Raises: + _PayloadCycleError: If a container references itself. + """ + cls = type(value) + if cls in _SCALAR_PAYLOAD_TYPES: + return value + # A state proxy's exact type is never a plain container type. + if cls is list or cls is tuple: + value_id = id(value) + if value_id in active: + raise _PayloadCycleError + active.add(value_id) + try: + copied = None + for index, item in enumerate(value): + detached = ( + item + if type(item) in _SCALAR_PAYLOAD_TYPES + else _scan_detach(item, memo, active) + ) + if copied is None: + if detached is item: + continue + copied = list(value[:index]) + copied.append(detached) + finally: + active.discard(value_id) + if copied is None: + return value + return cls(copied) if cls is tuple else copied + if cls is dict: + value_id = id(value) + if value_id in active: + raise _PayloadCycleError + active.add(value_id) + try: + replacements = None + for key, item in value.items(): + if type(item) in _SCALAR_PAYLOAD_TYPES: + continue + detached = _scan_detach(item, memo, active) + if detached is not item: + if replacements is None: + replacements = {} + replacements[key] = detached + finally: + active.discard(value_id) + if replacements is None: + return value + copied_dict = dict(value) + copied_dict.update(replacements) + return copied_dict + if isinstance(value, _IMMUTABLE_PAYLOAD_TYPES): + # Subclasses of the immutable scalar types. + return value + if cls is set: + # Set members are hashable, so they cannot be mutable-container + # proxies; only opaque hashable objects need the copy fallback. + if all(type(item) in _SCALAR_PAYLOAD_TYPES for item in value): + return value + return copy.deepcopy(value, memo) + # State proxy or other opaque mutable object: snapshot it. A proxy's + # __deepcopy__ unwraps it, detaching the copy from the state. + return copy.deepcopy(value, memo) + + BACKGROUND_TASK_MARKER = "_reflex_background_task" EVENT_ACTIONS_MARKER = "_rx_event_actions" UPLOAD_FILES_CLIENT_HANDLER = "uploadFiles" diff --git a/tests/units/test_event.py b/tests/units/test_event.py index 72cc9a7da29..83193b9e19c 100644 --- a/tests/units/test_event.py +++ b/tests/units/test_event.py @@ -173,6 +173,110 @@ def fn_with_args(arg1, arg2): assert event.payload == {"arg1": arg1, "arg2": arg2} +class _ProxyPayloadState(BaseState): + rows: list[dict[str, int]] = [{"a": 1}] + + +def _payload_for(value: Any) -> Any: + """Build an event passing value as the single handler arg. + + Args: + value: The value to pass to the handler. + + Returns: + The processed payload value delivered to the handler. + """ + + def fn_with_arg(arg): + pass + + fn_with_arg.__qualname__ = "fn_with_arg" + event = Event.from_event_type([EventHandler(fn=fn_with_arg)(value)])[0] + return event.payload["arg"] + + +def test_from_event_type_shares_plain_payload_values(): + """Payload values without state-bound proxies pass by reference.""" + rows = [{"a": 1}, {"b": 2}] + assert _payload_for(rows) is rows + mapping = {"x": [1, 2], "y": (3, 4)} + assert _payload_for(mapping) is mapping + + +def test_from_event_type_detaches_proxied_payload_values(): + """A MutableProxy payload value is detached from the state by copy.""" + from reflex.istate.proxy import MutableProxy + + state = _ProxyPayloadState() + proxied = state.rows + assert isinstance(proxied, MutableProxy) + + detached = _payload_for(proxied) + assert not isinstance(detached, MutableProxy) + assert detached == [{"a": 1}] + # Mutating the payload must not touch (or dirty) the state. + detached[0]["a"] = 42 + assert state.rows == [{"a": 1}] + assert "rows" not in state.dirty_vars + + +def test_from_event_type_detaches_nested_proxied_payload_values(): + """Proxies nested in plain containers are detached; clean parts shared.""" + from reflex.istate.proxy import MutableProxy + + state = _ProxyPayloadState() + plain = [{"z": 9}] + # Iterating a proxied list wraps each mutable element in a proxy. + listed_rows = list(state.rows) + assert any(isinstance(item, MutableProxy) for item in listed_rows) + + value = {"wrapped": listed_rows, "plain": plain} + detached = _payload_for(value) + assert detached is not value + assert detached["plain"] is plain + assert not any(isinstance(item, MutableProxy) for item in detached["wrapped"]) + detached["wrapped"][0]["a"] = 42 + assert state.rows == [{"a": 1}] + + +def test_from_event_type_copies_opaque_payload_objects(): + """Non-container mutable objects are still snapshotted by deepcopy.""" + import dataclasses as dc + + @dc.dataclass + class Opaque: + items: list[int] + + obj = Opaque(items=[1]) + detached = _payload_for([obj]) + assert detached[0] is not obj + assert detached[0].items == [1] + detached[0].items.append(2) + assert obj.items == [1] + + +def test_detach_state_proxies_handles_cyclic_payloads(): + """Self-referential containers fall back to deepcopy, preserving cycles.""" + # The reflex_base.event module replaces itself in sys.modules with the + # EventNamespace class, so private module names are only reachable + # through the globals of a function defined in that module. + detach = Event.from_event_type.__func__.__globals__["_detach_state_proxies"] + + cyclic_list: list = [1] + cyclic_list.append(cyclic_list) + out = detach(cyclic_list) + assert out is not cyclic_list + assert out[0] == 1 + assert out[1] is out + + cyclic_dict: dict = {"a": 1} + cyclic_dict["self"] = cyclic_dict + out = detach(cyclic_dict) + assert out is not cyclic_dict + assert out["a"] == 1 + assert out["self"] is out + + @pytest.mark.parametrize( ("input", "output"), [