Faster first page load: one-lock hydrate from the connect packet, diffed against compiled defaults - #7064
Open
FarhanAliRaza wants to merge 14 commits into
Open
Faster first page load: one-lock hydrate from the connect packet, diffed against compiled defaults#7064FarhanAliRaza wants to merge 14 commits into
FarhanAliRaza wants to merge 14 commits into
Conversation
Verify the redis lock and read its TTL once per state-tree save instead of once per substate, flip is_hydrated from the OnLoadInternalState leaf so the final step of the on_load chain no longer fetches and persists every substate, and stop opening the whole state tree on websocket connect just to stamp the session id (the first event after connecting carries it in router_data). Measured on a 20-substate app with the redis state manager: redis commands per first load 291 -> 76, backend CPU 99ms -> 62ms.
…compiled defaults Replace the three events the frontend sent on every websocket (re)connect (hydrate, update_vars_internal, on_load_internal) with a single State.hydrate_and_load event that resets and applies client storage, sends the state snapshot and queues the page's on_load handlers under one state lock. The event rides in the socket.io CONNECT packet (auth), so the backend starts loading state as soon as the namespace connects instead of after the connect acknowledgement round trip. On the first hydrate of a page the frontend still holds the compiled initialState, so it sends per-state hashes of it; for every state whose compiled defaults match the backend's, only vars that differ are sent. Reconnects and re-hydrates after storage resets still get the full state. Measured on a 20-substate app (redis state manager, 40ms RTT, 4Mbps, no websocket compression as with granian): time to hydrated 498ms -> 631ms is 716ms without the diff; hydrate payload 36KB -> 2KB on the wire; websocket frames after connect 5 -> 3; redis commands per first load 76 -> 55.
…s and tests The cached default snapshot used to diff the first hydrate against the compiled initialState is keyed by the number of registered state classes, so states defined after the first hydrate (dynamic component states, test suites) never compare against a stale snapshot; states missing from it are sent in full. The hydrate tests move to the isolated-registry processor tests so states other unit tests leave behind cannot break a full snapshot.
The frontend sets is_hydrated=false locally on navigation, but the on_load chain has always re-sent it in its first delta and tests and downstream code depend on that ordering; keep it rather than saving one 89-byte frame.
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. |
Merging this PR will not alter performance
Performance Changes
Comparing 
Footnotes
|
Contributor
 The PR appears safe to merge, with no outstanding actionable failures identified in the current changes.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
FarhanAliRaza
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d9abce959
ℹ️ 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".
- Serialize touched states first and verify the lock immediately before one pipelined write, instead of a check followed by concurrent per-substate writes; this also collapses N SET round trips into one. - Drop the sid/token link when the boot event carried in the CONNECT packet fails, since a refused connect never reaches on_disconnect. - Compare hydrate values with their compiled defaults in serialized form, so Python-equal but JSON-distinct values (1 vs 1.0, 0 vs False) are still sent. - Seed the default snapshot from compile_state, so a backend in the compiling process diffs against the exact compiled values and hot reloads refresh it. - Use CompileVars for the hydrate handler names in the processor.
…, shorten news - The hash list the frontend sends now starts with a digest of the compiled state names, so hashes are never matched positionally against a different set of states. - @event(supersedes=...) accepts a group name; hydrate_and_load and on_load_internal share the on_load group so a reconnect cancels an unfinished navigation chain the way a navigation always did. - News fragments describe the user-visible change only.
The pipelined write of a state tree now runs as a transaction that watches the lock key: if the lock expired or changed hands between the ownership check and the write, EXEC aborts and the save raises LockExpiredError instead of overwriting a newer writer's state. The mocked redis models WATCH/MULTI/EXEC so the unit tests cover the aborted-save path.
… keys The TTL warning in the redis save now reads PTTL through the pipeline that holds WATCH, so the ownership check, the warning and the write use one connection instead of borrowing a second pooled connection while the first is held. The connect-packet and hydrate payload keys are named constants on CompileVars and used by the backend and the generated frontend template.
Contributor
There was a problem hiding this comment.
1 issue found across 14 files (changes from recent commits).
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="reflex/app.py">
<violation number="1" location="reflex/app.py:2033">
P0: `constants.CompileVars.CONNECT_AUTH_EVENT` is referenced here but is not defined anywhere in the repo (only this usage matches a repo-wide search). Every websocket connect will raise `AttributeError` in `on_connect` before the event is dispatched, breaking app boot for all sessions. Define `CONNECT_AUTH_EVENT` (and the other new `CompileVars` members used by the boot flow) in `reflex/constants/compiler.py`, or fall back to the previous literal key.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…hing redis-py reports a connection error during WATCH as a WatchError, since the watch state is lost with the connection, and a lease refresh touching the lock key trips the watch as well. The fenced save now retries once on a fresh connection, which re-checks the lock before writing, and only a second WatchError discards the save as a lost lock. This surfaced in the lifespan integration test: the harness closes the redis pool on shutdown while a lifespan task's save is in flight, and the old unfenced path reconnected silently where the fenced one raised.
The redis client is created with retry_on_error=[RedisError], so when EXEC returned nil for a changed lock, redis-py reconnected and re-ran the transaction without the WATCH and the stale write landed anyway; the mock did not model that, only a real redis showed it. The save is now a single EVAL: the script compares the lock id, writes every touched state with its expiration, and returns the lock's PTTL, or nil without writing when the lock changed hands. That makes the check and write atomic on the server, safe under command retries, and one round trip instead of four (WATCH, GET, PTTL, EXEC). The mock emulates that script, and the WATCH-specific retry and tests go away with it. Also fall back to the full snapshot when a hydrate payload's hash list has a different length than the compiled states, instead of failing the event.
Older redis-py stubs declare the EVAL arguments and reply as str, while the save passes bytes keys and payloads and reads back an int or nil.
Open the socket.io transport in a microtask before React mounts, then hand the warm socket to connect() so the first hydrate_and_load round trip does not wait for the mount. Unclaimed warm sockets are discarded on a timeout, on pagehide, on a transport mismatch, or on HMR dispose. Read the Redis state tree with one MGET instead of a pipelined GET per state, and skip the read when the tree is already populated. Store the immutable per-class metadata (parent, root, name, full name) on the owning class so the 128-entry LRU cannot evict it in apps with many states, and add a hydration benchmark that covers both sides of the LRU capacity. Claude-Session: https://claude.ai/code/session_017ahSHCfgq16R8hSLBWH4p2
Contributor
There was a problem hiding this comment.
1 issue found across 11 files (changes from recent commits).
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="reflex/state.py">
<violation number="1" location="reflex/state.py:347">
P2: Apps with more than 128 generated state classes will repeatedly miss this LRU during metadata lookups. The class-local dictionary already memoizes each value, so remove the bounded LRU and use the per-class cache directly.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| """ | ||
| cache_key = fn.__name__ | ||
|
|
||
| @functools.lru_cache |
Contributor
There was a problem hiding this comment.
P2: Apps with more than 128 generated state classes will repeatedly miss this LRU during metadata lookups. The class-local dictionary already memoizes each value, so remove the bounded LRU and use the per-class cache directly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/state.py, line 347:
<comment>Apps with more than 128 generated state classes will repeatedly miss this LRU during metadata lookups. The class-local dictionary already memoizes each value, so remove the bounded LRU and use the per-class cache directly.</comment>
<file context>
@@ -327,6 +327,45 @@ def _override_base_method(fn: Callable[PARAMS, RETURN]) -> Callable[PARAMS, RETU
+ """
+ cache_key = fn.__name__
+
+ @functools.lru_cache
+ @functools.wraps(fn)
+ def wrapped(cls: type[BaseState]) -> RETURN:
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All Submissions:
Type of change
Changes To Core Features:
What changes
First load of a Reflex app used to be: HTML, JS, websocket upgrade, wait for the socket.io connect ack, then three events (
hydrate,update_vars_internal,on_load_internal), each its own state lock cycle, withhydrateserializing every var of every substate even though the bundle already carries the same defaults.Now:
hydrate_and_loadevent as socket.ioauth, so the backend starts loading state as soon as the namespace connects, one round trip earlier. The event resets and applies client storage, snapshots the state, and queues the page'son_loadhandlers under one lock.initialState; for every state whose compiled defaults match the backend's, only vars that differ are sent. Reconnects and storage resets still get the full state. States defined after the snapshot was taken (dynamic component states) invalidate the cached snapshot; unknown states and malformed hash lists are sent in full.is_hydratedis flipped from theOnLoadInternalStateleaf instead of the root (which made the redis manager fetch and persist every substate), and connecting no longer opens the whole state tree just to stamp the session id.State.hydrateis unchanged and still used by the shared-state rehydrate and by the compatibility path for events arriving on an unknown token.Measurements
Production build of a 20-substate app with an
on_loadhandler and a cookie var, redis state manager, behind a TCP proxy adding RTT and a 4 Mbps cap. Websocket compression disabled in the rig because granian (the production server) does not negotiate permessage-deflate. Medians of 7 to 9 fresh-session loads.Two further ideas were implemented, measured, and dropped: hydrating only the states a page renders (flat on both a light and a heavy app, and risky with
rx.memobodies), and enabling the opportunistic lock by default (55 to 34 redis commands, about 15 ms, but a locking-behavior change better left opt-in).Tests
hydrate_and_loadsingle-lock flow and cookie reset/apply, diffing against compiled defaults and the full-snapshot fallbacks, the one-command lock-checked tree save and its discard when the lock changed hands (run against both the mock and a real redis), and the boot event carried inon_connectauth.tests/units/reflex_cliexcluded: the hosting CLI rejects the dev version string, identical on the base commit).🤖 Generated with Claude Code
https://claude.ai/code/session_01JHj3TDXkYX4QNoUm28RLWG