perf(compile): read only set props and cache literal Var dispatch - #7121
Conversation
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. |
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 870787ab22
ℹ️ 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".
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
870787a to
9300edf
Compare
…d the root news fragment
Merging this PR will improve performance by 13.16%
Performance Changes
Tip Curious why performance improved? Comment Comparing Footnotes
|
…k in Component.render
masenf
left a comment
There was a problem hiding this comment.
i'm still nervous overall about not having a consistent Tag interface.
I think what we need is to move most Tag functionality into a CommonTag base class that has
def render(self, children: Sequence[Any] | None = None) -> dict[str, Any]:
if children is not None:
return dict(self.set(children=children))Have the specialty tags inherit from this. And have Tag inherit from CommonTag and overwrite the render(...) method with its more efficient implementation.
That way we don't have weird code duplication or ignored params, so CommonTag has a stable interface that is correct top to bottom, but the frequent case can still be performant.
The whole reason Tag and friends exist is to create another layer of indirection between the component and the rendered output. It's mostly an internal implementation detail, but I want to make sure that it remains consistent so we can still reason about it as a concept.
Summary
First of three stacked compile-performance PRs. This one is pure hot-path work with no change to generated output.
Component._iter_set_props:_render,_get_vars, and the prop-component scan walked every declared prop (34 to 70 per Radix component) through the field descriptor to find the few that are set. They now iterate the instance dict plus class-level defaults, in declaration order. Literal defaults,@propertyoverrides, and factory defaults still surface._literal_var_for: caches the literal Var class per exact value type instead of copying and walking the twelve-entry isinstance ladder on everyLiteralVar.create. Cleared when a literal subclass registers.insert_app_wrapsshort-circuits on identity before the field-by-field component comparison.CommonTagnow holds the fields and generic render protocol shared by every tag class, withrender(children=None)defined asdict(self.set(children=children)).Tagsubclasses it and overridesrenderwith a direct fast path that produces the same dictionary (tested).CondTag,MatchTag,IterTag, andTaglessinheritCommonTag, soComponent._renderand its overrides are annotated-> CommonTag.render_proppasses strings and dicts straight through.cached_propertykeys use a plainobject()instead ofuuid4.Measurements
Docs site (
docs/app, 511 routes),App._compile(dry_run=True)in a fresh process, warm runs:cProfile of the same compile: field-descriptor reads fell from 14.9M to 3.1M calls;
_render-6.2 s,_get_vars-3.1 s,_create_literal_var-5.1 s,Tag.add_props-2.5 s (profiled time).Benchmark pages (
tests/benchmarks/fixtures.py, full-context compile): complicated page 29.3 ms to 25.0 ms.Stack
Each later PR's diff includes the earlier ones; merge in order.
Test plan
render_proppassthrough, literal dispatch invalidation on late registration,cached_propertyidentity, pickle, and release.uv run pytest tests/units/components tests/units/reflex_base tests/units/compiler tests/units/test_event.pygreen apart from failures that reproduce on cleanmainin this environment.https://claude.ai/code/session_01PmizE1eQhtYZyVs1RK2ke3