Skip to content

Fuse snapshot memo subtree walks into one traversal - #6748

Closed
Alek99 wants to merge 3 commits into
claude/reflex-compiler-perf-t8ztc9-14-format-tag-bypassfrom
claude/reflex-compiler-perf-t8ztc9-15-snapshot-memo-walks
Closed

Fuse snapshot memo subtree walks into one traversal#6748
Alek99 wants to merge 3 commits into
claude/reflex-compiler-perf-t8ztc9-14-format-tag-bypassfrom
claude/reflex-compiler-perf-t8ztc9-15-snapshot-memo-walks

Conversation

@Alek99

@Alek99 Alek99 commented Jul 11, 2026

Copy link
Copy Markdown
Member

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • Performance improvement (non-breaking change, identical compile output)

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?

What this does

Compiling a snapshot memo definition (compile_experimental_component_memo, non-passthrough branch) walked the deep-copied subtree four separate times — _get_all_hooks, _get_all_custom_code, _get_all_dynamic_imports, _get_all_imports — each independently recursing children and prop components, and re-calling per-node getters like _get_components_in_props once per artifact. Snapshot memos dominate rx.foreach-heavy pages, so these redundant traversals multiply.

Fix

New _collect_subtree_artifacts gathers all four artifacts in a single traversal. Correctness is order-sensitive (hooks/custom-code dict insertion order determines the emitted JS), so the fused walk reproduces each legacy recursion's exact per-node combination order:

  • hooks: internal → own → added → children, and never from prop subtrees (an in_prop_tree flag mirrors _get_all_hooks skipping prop edges entirely);
  • custom code: own → prop subtrees → own add_custom_code → children (the interleaving _get_all_custom_code uses);
  • dynamic imports: set semantics, order-free;
  • imports: self → children → props, via the same merge_parsed_imports.

The fused walk runs before render() so add_hooks side effects (derived props) land first, matching the legacy hooks-walk-then-render order (the passthrough branch already collects imports before render).

Scoping note

The ticket also covers the tag-hash walks (_get_component_hash). Those are entangled with the known _get_all_hooks_internal shared-cache mutation bug (ENG-10108): today's hash output can depend on cache pollution from prior walks, so a fused hash walk can't be verified byte-identical until that bug is fixed. Deferred to a follow-up after ENG-10108; this PR takes the definition-compile side, which is pollution-free.

Verification

  • New order-sensitive equivalence test: a tree exercising hooks, internal (ref) hooks, per-instance custom code, class-level add_custom_code, dynamic imports, component-typed prop slots, and nested prop-subtree children asserts the fused walk's four results equal the four legacy recursions including dict key order, and that prop-subtree hooks don't leak out.
  • CodSpeed instrumentation on this PR is the authoritative before/after CPU measurement (this sandbox has no PyPI egress for local profiling); numbers will be posted here once the run completes — test_compile_*[_stateful_page] exercises snapshot memos via rx.foreach.

Part of a compiler-performance series; tracked in Linear as ENG-10105.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Jy8uHH11KircGa2MbTrR8g


Generated by Claude Code

@Alek99
Alek99 requested a review from a team as a code owner July 11, 2026 02:48
@greptile-apps

greptile-apps Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR combines snapshot memo artifact collection into one subtree traversal. The main changes are:

  • Collect hooks, custom code, dynamic imports, and imports in one walk.
  • Preserve artifact ordering across children and component-valued props.
  • Add an order-sensitive equivalence test.
  • Document the compiler performance improvement.

Confidence Score: 4/5

No additional blocking issue was found in the updated code.

  • The fused traversal preserves the tested child and prop ordering.
  • No separate production failure remained after accounting for the existing artifact-timing issue.

Important Files Changed

Filename Overview
reflex/compiler/utils.py Adds the fused artifact collector and uses it for non-passthrough snapshot memo compilation.
tests/units/compiler/test_compiler_utils.py Adds equivalence coverage for artifact values, ordering, structural children, and component-valued props.
news/6748.performance.md Documents the snapshot memo traversal optimization.

Reviews (3): Last reviewed commit: "Appease pyright on subtree walk over Bas..." | Re-trigger Greptile

Comment thread reflex/compiler/utils.py
@codspeed-hq

codspeed-hq Bot commented Jul 11, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 55.53%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 3 improved benchmarks
✅ 24 untouched benchmarks
⏩ 8 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation test_var_access[mutable_list] 19.1 ms 9.1 ms ×2.1
Simulation test_var_access[mutable_dict] 32.5 ms 20 ms +62.13%
Simulation test_evaluate_page_with_hooks[_stateful_page] 5.2 ms 4.8 ms +10.25%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/reflex-compiler-perf-t8ztc9-15-snapshot-memo-walks (5e8f117) with claude/reflex-compiler-perf-t8ztc9-14-format-tag-bypass (e4eaa9c)2

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.

  2. No successful run was found on claude/reflex-compiler-perf-t8ztc9-14-format-tag-bypass (002a553) during the generation of this report, so 218c8e2 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@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: 8d47b987d7

ℹ️ 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/compiler/utils.py
Comment on lines +433 to +435
child_parts = [
_collect_subtree_artifacts(child, in_prop_tree=in_prop_tree)
for child in component.children

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Include VarData component artifacts in fused walk

This fused traversal only descends structural children here (and component props above), but Bare overrides the legacy _get_all_custom_code and _get_all_dynamic_imports to also visit components stored on its contents VarData. After the snapshot memo path replaces the legacy walkers with this helper, a memo body containing a component Var/Bare such as a conditional branch with a NoSSRComponent or a component that returns _get_custom_code() will omit that dynamic import or custom code from the generated memo module, even though the rendered JSX still references it. Please preserve the VarData component edge (or otherwise delegate to the override behavior) when fusing the walk.

Useful? React with 👍 / 👎.

Comment thread reflex/compiler/utils.py
return {}


def _collect_subtree_artifacts(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is what the new plugin-based compiler is already designed to do: reduce tree walks and collect the information from all components on a single descent. instead of implementing a smaller, more limited version of that, we should probably spin up a subcompiler with its own CompileContext/PageContext without the MemoizationPlugin in the stack and compile the subtree root as a "page", then collect the data from the page context. That way as the compiler develops, we don't have to maintain parallel functionality.

@Alek99
Alek99 force-pushed the claude/reflex-compiler-perf-t8ztc9-15-snapshot-memo-walks branch from 82b57dd to 86c42bd Compare July 18, 2026 01:46
@Alek99
Alek99 changed the base branch from main to claude/reflex-compiler-perf-t8ztc9-14-format-tag-bypass July 18, 2026 01:46
claude and others added 3 commits August 12, 2026 16:26
Compiling a snapshot memo definition walked the (deep-copied) subtree
four times - _get_all_hooks, _get_all_custom_code,
_get_all_dynamic_imports, _get_all_imports - each independently
recursing children and prop components and re-calling per-node getters
like _get_components_in_props.

_collect_subtree_artifacts gathers all four in a single traversal,
combining each artifact at every node in the same order as its
dedicated recursion (hooks stay structural-tree-only; prop-subtree
custom code still lands before the node's own add_custom_code). The
walk runs before render() so add_hooks side effects land first, same
as the legacy hooks-walk-then-render order.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jy8uHH11KircGa2MbTrR8g
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jy8uHH11KircGa2MbTrR8g
@Alek99
Alek99 force-pushed the claude/reflex-compiler-perf-t8ztc9-14-format-tag-bypass branch from e4eaa9c to 002a553 Compare August 12, 2026 23:32
@Alek99
Alek99 force-pushed the claude/reflex-compiler-perf-t8ztc9-15-snapshot-memo-walks branch from 86c42bd to 5e8f117 Compare August 12, 2026 23:32
@Alek99

Alek99 commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

Closing after triaging the stack against current main:

  • masenf's review points at the plugin-based compiler as the right home for this optimization — spin up a subcompiler with its own CompileContext/PageContext (without the MemoizationPlugin) and collect artifacts from one descent, rather than maintaining a parallel fused walk here.
  • Two P1 review flags were never addressed: the fused walk evaluates a parent's artifact getters before descendant add_hooks/render side effects, and it misses Bare's VarData component artifacts (custom code / dynamic imports), so memo bodies could omit required output.
  • The +55.53% CodSpeed report carries a cross-runtime-environment warning, so the magnitude is unconfirmed.

The stack has been re-stitched around this PR: #6751 now bases directly on #6798's branch. The walk-fusion goal is still worth pursuing via the subcompiler approach.

@Alek99 Alek99 closed this Aug 13, 2026
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.

3 participants