Skip to content

Fast-path framework bookkeeping fields in state attribute access#6757

Open
Alek99 wants to merge 3 commits into
mainfrom
claude/reflex-perf-optimizations-01l7a3-eng-10096
Open

Fast-path framework bookkeeping fields in state attribute access#6757
Alek99 wants to merge 3 commits into
mainfrom
claude/reflex-perf-optimizations-01l7a3-eng-10096

Conversation

@Alek99

@Alek99 Alek99 commented Jul 14, 2026

Copy link
Copy Markdown
Member

Linear: ENG-10096

Description

_get_attribute was ~43% of profiled event-handling time, and much of that was the framework's own reads of dirty_vars, parent_state, substates, and _backend_vars paying the full inherited-vars/event-handler/proxying chain on every access from _mark_dirty/_clean/get_delta.

  • Add those names (plus dirty_substates) to the CLASS_VAR_NAMES fast path. They are never state vars, never inherited, and were never proxied (not in base_vars/backend_vars), so resolution via object.__getattribute__ is behavior-identical; assignments already ended in object.__setattr__ with no dirty-marking.
  • Reorder the proxying condition so base_vars/backend_vars membership is checked before is_mutable_type, sparing non-var values the type check.
  • Cache get_skip_vars() per class as a frozenset, invalidated when inherited_vars changes (_update_substate_inherited_vars, add_var).
  • Cache the frontend computed var names used by get_delta per class, invalidated when computed_vars changes (_evaluate, dynamic route vars).

Benchmarks (GitHub Actions runner, run, 2 passes each)

Case main this PR speedup
4 bookkeeping attr reads 3.66 / 3.56 us 0.61 / 0.66 us ~5.7x
setattr + get_delta + _clean cycle 84.4 / 71.4 us 54.5 / 51.4 us ~1.4x
get_skip_vars() 0.49 / 0.48 us 0.15 / 0.15 us ~3.3x

cProfile (5000 bookkeeping reads + 500 event cycles): _get_attribute tottime drops 0.120s -> 0.061s; total function calls drop 256k -> 207k.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
    • test_get_skip_vars_cached_per_class: per-class caching with inherited vars included and no cross-class leakage.
    • test_frontend_computed_var_names_cached_per_class: backend computed vars excluded; cached identity.
    • test_framework_bookkeeping_fields_not_proxied: bookkeeping containers stay raw while real state vars still get proxied.
  • Have you successfully ran tests with your changes locally?

🤖 Generated with Claude Code

https://claude.ai/code/session_01G8cXh3TUbNtbjm2ERqE62X


Generated by Claude Code

claude added 2 commits July 10, 2026 19:59
_get_attribute was 43% of profiled event-handling time; much of it was
the framework's own reads of dirty_vars, parent_state, substates, and
_backend_vars paying the full inherited-vars/event-handler/proxying
chain on every access.

- Add those names (and dirty_substates) to the CLASS_VAR_NAMES fast
  path; they are never state vars, never inherited, and never proxied.
- Check base_vars/backend_vars membership before is_mutable_type when
  deciding whether to proxy, so non-var values skip the type check.
- Cache get_skip_vars() per class (invalidated when inherited_vars
  changes) instead of rebuilding the set on every internal write.
- Cache the frontend computed var names used by get_delta per class
  (invalidated when computed_vars changes).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G8cXh3TUbNtbjm2ERqE62X
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G8cXh3TUbNtbjm2ERqE62X
@Alek99 Alek99 requested a review from a team as a code owner July 14, 2026 00:35
@linear-code

linear-code Bot commented Jul 14, 2026

Copy link
Copy Markdown

ENG-10096

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G8cXh3TUbNtbjm2ERqE62X
@codspeed-hq

codspeed-hq Bot commented Jul 14, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 24 untouched benchmarks
⏩ 8 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_var_access[non_mutable_scalar] 57.7 ms 65.9 ms -12.41%
test_process_event 7.6 ms 6.6 ms +14.32%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/reflex-perf-optimizations-01l7a3-eng-10096 (97e9e37) with main (48f9e6c)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR speeds up access to internal state bookkeeping. The main changes are:

  • Routes framework bookkeeping fields through direct attribute access.
  • Caches serialization skip names and frontend computed-variable names per class.
  • Reorders mutable-proxy checks to avoid unnecessary type inspection.
  • Adds unit coverage and a performance changelog entry.

Confidence Score: 4/5

Descendant skip-cache invalidation needs to be fixed before merging.

  • The bookkeeping fast path preserves the current internal field behavior.
  • Dynamic variable registration can leave a grandchild with stale serialization metadata.
  • Existing tests do not cover an already-cached grandchild when an ancestor gains a variable.

reflex/state.py

Important Files Changed

Filename Overview
reflex/state.py Adds bookkeeping fast paths and per-class caches; dynamic variable registration can leave a descendant skip cache stale.
tests/units/test_state.py Covers cache identity and raw bookkeeping access, but not ancestor variable registration after a grandchild cache is populated.
news/6757.performance.md Documents the state attribute and serialization-cache performance changes.

Reviews (1): Last reviewed commit: "Add changelog fragment" | Re-trigger Greptile

Comment thread reflex/state.py
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Grandchild Skip Cache Stays Stale

When add_var runs after a grandchild has cached get_skip_vars(), this loop clears only the direct substates while the new variable remains visible farther down through the aliased vars and inherited_vars dictionaries. The grandchild can then serialize the inherited variable as local state because its cached skip set was not invalidated.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 483d2d1253

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread reflex/state.py
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Recursively invalidate skip-var caches for dynamic vars

When add_var() is called on a state that already has nested substates, this clears the cache only for immediate children. Grandchildren have their inherited_vars updated indirectly because it aliases the child vars dict, but their _skip_var_names frozenset was already populated during class creation and remains stale, so Grandchild.get_skip_vars() omits the newly inherited dynamic var. Please invalidate descendants as well when propagating a dynamically added var.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants