perf(memo): evaluate passthrough bodies once and reuse their analysis - #7123
perf(memo): evaluate passthrough bodies once and reuse their analysis#7123FarhanAliRaza wants to merge 3 commits into
Conversation
Component render, Var collection, and the prop-component scan walked every declared prop through the field descriptor to find the few that are set. Iterate the instance dict plus class-level defaults instead. Cache the literal Var class per exact value type, short-circuit app-wrap dedupe on identity, skip the generic tag protocol for plain tags, and hoist the memoize plugin's component imports. Docs site dry compile (511 pages): 47 s to 40 s. Claude-Session: https://claude.ai/code/session_01PmizE1eQhtYZyVs1RK2ke3
EventChain.create rebuilt an identical chain for every component that bound the same handler to the same trigger, and the memoize pass then rendered each chain again to name its useCallback wrapper. Intern the chain on the handler keyed by args spec and trigger, and key the wrapper cache by chain identity so repeated call sites reuse the wrapper without rendering. Claude-Session: https://claude.ai/code/session_01PmizE1eQhtYZyVs1RK2ke3
Passthrough memo wrappers evaluated the wrapped body twice to derive the tag, and module emission rendered it a third time to collect hooks, imports, custom code, and dynamic imports. Build the passthrough definition directly from the fixed children signature, retain the rendered body and its artifacts keyed by content hash, and let emission reuse them when the styled root still matches. Claude-Session: https://claude.ai/code/session_01PmizE1eQhtYZyVs1RK2ke3
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 21 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/units/reflex_base/components/test_memo.py">
<violation number="1" location="tests/units/reflex_base/components/test_memo.py:30">
P3: `assert definition.component is definition.component` always passes because it compares an object with itself. It adds false confidence to `test_auto_memo_evaluates_body_once` while asserting nothing; remove it or replace it with the comparison it was meant to check (e.g. that the wrapper/definition retains the memoized component).</violation>
</file>
<file name="packages/reflex-base/src/reflex_base/event/__init__.py">
<violation number="1" location="packages/reflex-base/src/reflex_base/event/__init__.py:921">
P2: When a component tree is deep-copied, this handler cache is copied through each event chain, so copying one trigger also duplicates every chain cached on the handler. Keep the interning cache outside the handler instance (or explicitly exclude it from handler/component copies) so compile-time clones only copy the chain they contain.</violation>
</file>
<file name="packages/reflex-base/src/reflex_base/components/memo.py">
<violation number="1" location="packages/reflex-base/src/reflex_base/components/memo.py:1972">
P2: When the same `RegistrationContext` is reused for multiple compiles, each changed memo body leaves its analysis and referenced component objects in `_memo_body_analyses`, so repeated development/export compiles grow the context's memory footprint. Clear this map with the compile caches in the compile cleanup path, or scope the map to one compile.</violation>
</file>
<file name="packages/reflex-base/src/reflex_base/vars/base.py">
<violation number="1" location="packages/reflex-base/src/reflex_base/vars/base.py:145">
P2: When applications generate many short-lived custom value classes, this cache retains every such class after its first `LiteralVar` lookup, causing unbounded process memory growth. Use weak class keys or restrict caching to stable built-in types instead of storing every exact type strongly.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # lives on the handler, which is never pickled or copied. | ||
| bound_chains = None | ||
| if not event_chain_kwargs and isinstance(value, EventHandler): | ||
| bound_chains = value.__dict__.setdefault("_bound_chains", {}) |
There was a problem hiding this comment.
P2: When a component tree is deep-copied, this handler cache is copied through each event chain, so copying one trigger also duplicates every chain cached on the handler. Keep the interning cache outside the handler instance (or explicitly exclude it from handler/component copies) so compile-time clones only copy the chain they contain.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/event/__init__.py, line 921:
<comment>When a component tree is deep-copied, this handler cache is copied through each event chain, so copying one trigger also duplicates every chain cached on the handler. Keep the interning cache outside the handler instance (or explicitly exclude it from handler/component copies) so compile-time clones only copy the chain they contain.</comment>
<file context>
@@ -913,6 +913,16 @@ def create(
+ # lives on the handler, which is never pickled or copied.
+ bound_chains = None
+ if not event_chain_kwargs and isinstance(value, EventHandler):
+ bound_chains = value.__dict__.setdefault("_bound_chains", {})
+ bound = bound_chains.get((id(args_spec), key))
+ if bound is not None and bound[0] is args_spec:
</file context>
| rendered = component.render() | ||
| artifacts = tuple(_component_artifacts(component, recursive=False)) | ||
| digest = deterministic_hash(rendered, *artifacts) | ||
| analyses = RegistrationContext.ensure_context()._memo_body_analyses |
There was a problem hiding this comment.
P2: When the same RegistrationContext is reused for multiple compiles, each changed memo body leaves its analysis and referenced component objects in _memo_body_analyses, so repeated development/export compiles grow the context's memory footprint. Clear this map with the compile caches in the compile cleanup path, or scope the map to one compile.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/components/memo.py, line 1972:
<comment>When the same `RegistrationContext` is reused for multiple compiles, each changed memo body leaves its analysis and referenced component objects in `_memo_body_analyses`, so repeated development/export compiles grow the context's memory footprint. Clear this map with the compile caches in the compile cleanup path, or scope the map to one compile.</comment>
<file context>
@@ -1893,9 +1962,18 @@ def component_hash(component: Component, *, recursive: bool) -> str:
+ rendered = component.render()
+ artifacts = tuple(_component_artifacts(component, recursive=False))
+ digest = deterministic_hash(rendered, *artifacts)
+ analyses = RegistrationContext.ensure_context()._memo_body_analyses
+ if digest not in analyses:
+ analyses[digest] = _analyze_memo_body(component, rendered, artifacts)
</file context>
| ) | ||
| # A class object's type is its metaclass, which other classes share. | ||
| if not isinstance(value, type): | ||
| _literal_var_by_type[value_type] = literal_subclass |
There was a problem hiding this comment.
P2: When applications generate many short-lived custom value classes, this cache retains every such class after its first LiteralVar lookup, causing unbounded process memory growth. Use weak class keys or restrict caching to stable built-in types instead of storing every exact type strongly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/vars/base.py, line 145:
<comment>When applications generate many short-lived custom value classes, this cache retains every such class after its first `LiteralVar` lookup, causing unbounded process memory growth. Use weak class keys or restrict caching to stable built-in types instead of storing every exact type strongly.</comment>
<file context>
@@ -114,6 +113,37 @@ class VarSubclassEntry:
+ )
+ # A class object's type is its metaclass, which other classes share.
+ if not isinstance(value, type):
+ _literal_var_by_type[value_type] = literal_subclass
+ return literal_subclass
</file context>
| ) as evaluate: | ||
| factory, definition = memo.create_passthrough_component_memo(component) | ||
| wrapper = factory() | ||
| assert definition.component is definition.component |
There was a problem hiding this comment.
P3: assert definition.component is definition.component always passes because it compares an object with itself. It adds false confidence to test_auto_memo_evaluates_body_once while asserting nothing; remove it or replace it with the comparison it was meant to check (e.g. that the wrapper/definition retains the memoized component).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/reflex_base/components/test_memo.py, line 30:
<comment>`assert definition.component is definition.component` always passes because it compares an object with itself. It adds false confidence to `test_auto_memo_evaluates_body_once` while asserting nothing; remove it or replace it with the comparison it was meant to check (e.g. that the wrapper/definition retains the memoized component).</comment>
<file context>
@@ -0,0 +1,170 @@
+ ) as evaluate:
+ factory, definition = memo.create_passthrough_component_memo(component)
+ wrapper = factory()
+ assert definition.component is definition.component
+ assert evaluate.call_count == 1
+
</file context>
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| dynamic_imports = ( | ||
| {analysis.dynamic_import} if analysis.dynamic_import else set() | ||
| ) | ||
| render._imports_cache = analysis.imports |
There was a problem hiding this comment.
Assigning analysis.imports directly to render._imports_cache exposes the retained analysis's mutable import lists to the rest of compilation. The later shallow dictionary copy keeps those same lists, so appending the required JSX and wrapper imports mutates the cached analysis on every emission. Repeated or shared memo compilations therefore accumulate duplicate imports and make reuse increasingly expensive. Copy the parsed imports before installing them on render, or keep the retained analysis immutable.
| render._imports_cache = analysis.imports | |
| render._imports_cache = { | |
| lib: list(fields) for lib, fields in analysis.imports.items() | |
| } |
Knowledge Base Used: Frontend compilation pipeline
1 participant
Summary
Third of three stacked compile-performance PRs. Stacked on #7121 and #7122; this diff includes both. Merge those first.
create_passthrough_component_memobuilds the passthrough definition directly from the fixedchildrensignature (_PASSTHROUGH_PARAMS) and evaluates the body once. Previously it evaluated once to derive the tag and again inside_create_component_definition.component_hashretains the rendered body and the artifacts it already collected (imports, internal and added hooks, custom code, dynamic import, app wraps) in a_MemoBodyAnalysison the registration context, keyed by content hash. The component records its key in_memo_analysis_key(cleared with the other compile caches).compile_experimental_component_memoreuses that analysis when the styled root still matches (_MemoBodyAnalysis.can_reuse: same class, default__copy__, equal style and style metadata) instead of re-rendering and re-collecting. Older reflex-base releases without the analysis fall back to the previous path._repeated_stateful_pageto the benchmark fixtures.Measurements
Docs site dry compile (511 routes), warm runs:
cProfile:
create_passthrough_component_memo-7.5 s,compile_experimental_component_memo-2.0 s,_evaluate_memo_functioncalls 21.5k to 10.8k (profiled time; the full stack goes 112.5 s to 78.9 s profiled).Test plan
tests/units/reflex_base/components/test_memo.py: analysis recorded and reused, invalidated on style change, isolated per registration context fork.tests/units/reflex_base,tests/units/components,tests/units/compiler, benchmark compile tests green apart from failures that reproduce on cleanmainhere.pyi_hashes.jsonregenerated; ruff, pyright, pre-commit clean.https://claude.ai/code/session_01PmizE1eQhtYZyVs1RK2ke3