-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fast-path framework bookkeeping fields in state attribute access #6757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Speed up state attribute access: framework bookkeeping fields (dirty_vars, parent_state, substates, _backend_vars) resolve via the fast path, and get_skip_vars() plus get_delta's frontend computed var set are cached per class. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -357,6 +357,10 @@ def _is_user_descriptor(value: Any) -> bool: | |
|
|
||
| all_base_state_classes: dict[str, None] = {} | ||
|
|
||
| # Attribute names resolved directly via object.__getattribute__, bypassing the | ||
| # inherited-var / event-handler / proxying logic in _get_attribute. Class-level | ||
| # tracking dicts as well as per-instance framework bookkeeping fields that are | ||
| # never state vars belong here. | ||
| CLASS_VAR_NAMES = frozenset({ | ||
| "vars", | ||
| "base_vars", | ||
|
|
@@ -369,6 +373,11 @@ def _is_user_descriptor(value: Any) -> bool: | |
| "_always_dirty_computed_vars", | ||
| "_always_dirty_substates", | ||
| "_potentially_dirty_states", | ||
| "parent_state", | ||
| "substates", | ||
| "dirty_vars", | ||
| "dirty_substates", | ||
| "_backend_vars", | ||
| }) | ||
|
|
||
|
|
||
|
|
@@ -408,6 +417,13 @@ class BaseState(EvenMoreBasicBaseState): | |
| # Set of states which might need to be recomputed if vars in this state change. | ||
| _potentially_dirty_states: ClassVar[set[str]] = set() | ||
|
|
||
| # Cached result of get_skip_vars(), rebuilt lazily after inherited_vars changes. | ||
| _skip_var_names: ClassVar[frozenset[str] | None] = None | ||
|
|
||
| # Cached names of non-backend computed vars, rebuilt lazily after | ||
| # computed_vars changes. | ||
| _frontend_computed_vars: ClassVar[frozenset[str] | None] = None | ||
|
|
||
| # The parent state. | ||
| parent_state: BaseState | None = field(default=None, is_var=False) | ||
|
|
||
|
|
@@ -828,6 +844,7 @@ def computed_var_func(state: Self): | |
|
|
||
| setattr(cls, unique_var_name, computed_var_func_arg) | ||
| cls.computed_vars[unique_var_name] = computed_var_func_arg | ||
| cls._frontend_computed_vars = None | ||
| cls.vars[unique_var_name] = computed_var_func_arg | ||
| cls._update_substate_inherited_vars({unique_var_name: computed_var_func_arg}) | ||
| cls._always_dirty_computed_vars.add(unique_var_name) | ||
|
|
@@ -992,23 +1009,42 @@ def _check_overridden_computed_vars(cls) -> None: | |
| raise ComputedVarShadowsStateVarError(msg) | ||
|
|
||
| @classmethod | ||
| def get_skip_vars(cls) -> set[str]: | ||
| def get_skip_vars(cls) -> frozenset[str]: | ||
| """Get the vars to skip when serializing. | ||
|
|
||
| Returns: | ||
| The vars to skip when serializing. | ||
| """ | ||
| return ( | ||
| set(cls.inherited_vars) | ||
| | { | ||
| "parent_state", | ||
| "substates", | ||
| "dirty_vars", | ||
| "dirty_substates", | ||
| "router_data", | ||
| } | ||
| | types.RESERVED_BACKEND_VAR_NAMES | ||
| ) | ||
| # Cached per class; invalidated when inherited_vars changes. | ||
| if (skip_vars := cls.__dict__.get("_skip_var_names")) is None: | ||
| skip_vars = ( | ||
| frozenset(cls.inherited_vars) | ||
| | { | ||
| "parent_state", | ||
| "substates", | ||
| "dirty_vars", | ||
| "dirty_substates", | ||
| "router_data", | ||
| } | ||
| | types.RESERVED_BACKEND_VAR_NAMES | ||
| ) | ||
| cls._skip_var_names = skip_vars | ||
| return skip_vars | ||
|
|
||
| @classmethod | ||
| def _get_frontend_computed_var_names(cls) -> frozenset[str]: | ||
| """Get the names of computed vars that are sent to the frontend. | ||
|
|
||
| Returns: | ||
| The names of non-backend computed vars. | ||
| """ | ||
| # Cached per class; invalidated when computed_vars changes. | ||
| if (cached := cls.__dict__.get("_frontend_computed_vars")) is None: | ||
| cached = frozenset( | ||
| name for name, cv in cls.computed_vars.items() if not cv._backend | ||
| ) | ||
| cls._frontend_computed_vars = cached | ||
| return cached | ||
|
|
||
| @classmethod | ||
| @functools.lru_cache | ||
|
|
@@ -1214,6 +1250,8 @@ def add_var(cls, name: str, type_: Any, default_value: Any = None): | |
| # let substates know about the new variable | ||
| for substate_class in cls.get_substates(): | ||
| substate_class.vars.setdefault(name, var) | ||
| # inherited_vars may alias this class's vars dict. | ||
| substate_class._skip_var_names = None | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
|
|
||
| # Reinitialize dependency tracking dicts. | ||
| cls._init_var_dependency_dicts() | ||
|
|
@@ -1343,6 +1381,8 @@ def _update_substate_inherited_vars(cls, vars_to_add: dict[str, Var]): | |
| substate_class.vars.setdefault(name, var) | ||
| substate_class.inherited_vars.setdefault(name, var) | ||
| substate_class._update_substate_inherited_vars(vars_to_add) | ||
| # The skip vars cache incorporates inherited_vars. | ||
| substate_class._skip_var_names = None | ||
| # Reinitialize dependency tracking dicts. | ||
| cls._init_var_dependency_dicts() | ||
|
|
||
|
|
@@ -1401,6 +1441,7 @@ def inner_func(self: BaseState) -> list[str]: | |
|
|
||
| # Update tracking dicts. | ||
| cls.computed_vars.update(dynamic_vars) | ||
| cls._frontend_computed_vars = None | ||
| cls.vars.update(dynamic_vars) | ||
| cls._update_substate_inherited_vars(dynamic_vars) | ||
|
|
||
|
|
@@ -1474,9 +1515,9 @@ def _get_attribute(self, name: str) -> Any: | |
| if parent_state is not None: | ||
| return getattr(parent_state, name) | ||
|
|
||
| if is_mutable_type(type(value)) and ( | ||
| if ( | ||
| name in super().__getattribute__("base_vars") or name in backend_vars | ||
| ): | ||
| ) and is_mutable_type(type(value)): | ||
| # track changes in mutable containers (list, dict, set, etc) | ||
| return MutableProxy(wrapped=value, state=self, field_name=name) | ||
|
|
||
|
|
@@ -1863,9 +1904,7 @@ def get_delta(self) -> Delta: | |
| delta = {} | ||
|
|
||
| self._mark_dirty_computed_vars() | ||
| frontend_computed_vars: set[str] = { | ||
| name for name, cv in self.computed_vars.items() if not cv._backend | ||
| } | ||
| frontend_computed_vars = type(self)._get_frontend_computed_var_names() | ||
|
|
||
| # Return the dirty vars for this instance, any cached/dependent computed vars, | ||
| # and always dirty computed vars (cache=False) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
add_varruns after a grandchild has cachedget_skip_vars(), this loop clears only the direct substates while the new variable remains visible farther down through the aliasedvarsandinherited_varsdictionaries. The grandchild can then serialize the inherited variable as local state because its cached skip set was not invalidated.