perf: cut socket-layer per-event overhead (header cache, route LRU, deferred event routing)#6744
perf: cut socket-layer per-event overhead (header cache, route LRU, deferred event routing)#6744Alek99 wants to merge 5 commits into
Conversation
Four independent per-event costs on the websocket hot path: - on_event re-decoded every ASGI header from bytes and re-parsed x-forwarded-for on every event, though headers are fixed for a connection's lifetime. Cache decoded headers + client IP per sid (12.5x faster; cache cleared on disconnect, a fresh dict is handed out per event so router_data mutations cannot leak into the cache). - the router re-ran a linear regex scan over all routes per event for a path that rarely changes. lru_cache the per-router path resolution (93x faster on a 69-route app). - every outgoing update was encoded with stdlib json.dumps plus a Python default hook. Add an orjson fast path for the socket wire format (new required dependency) with passthrough options that keep datetimes/dataclasses/non-str keys routed through reflex serializers exactly like stdlib, falling back to stdlib for payloads orjson cannot represent (e.g. ints beyond 64 bits). Parity is pinned by unit tests; a CodSpeed benchmark measures both encoders on the same large packet. - the final delta encode+emit ran while holding the state lock. The event processor now resolves the delta and validates chained events under the lock, but performs the actual emissions (socket encode + send + chained-event enqueue) after releasing it, shortening lock hold ~3x for a 2ms encode. Intermediate (streamed) yields still emit inline; background-task and middleware-preprocess flows keep their semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sns7EuBPYvfGCxRSRZ1bUx
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sns7EuBPYvfGCxRSRZ1bUx
Greptile SummaryThis PR reduces websocket event overhead in the socket and routing paths. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "refactor: drop orjson wire encoder from ..." | Re-trigger Greptile |
Merging this PR will not alter performance
Comparing Footnotes
|
Capture (substate, root_state) explicitly for the background-task path instead of relying on control flow pyright cannot narrow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sns7EuBPYvfGCxRSRZ1bUx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbf451db35
ℹ️ 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".
| for emit in deferred_emits: | ||
| await emit() |
There was a problem hiding this comment.
Snapshot deltas before releasing the state lock
When a foreground handler changes a mutable state var while a background task is waiting on the same state lock, this releases the lock before the deferred ctx.emit_delta serializes the delta. BaseState.get_delta() builds the delta with self.get_value(prop), so list/dict values are still the original mutable objects; a background task can acquire the lock and mutate those objects before this deferred emit runs, causing the client to receive the background mutation in the foreground update or out of order. Please either emit/encode while the lock is held or copy/freeze the resolved delta before releasing it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in ac961f3 — get_value does return the live containers, and emit_lost_and_found awaits redis before pickling, so the window was real on multi-instance deployments. Deltas now emit inline under the lock again (exactly as before this PR); only the routing of final chained events is deferred past the lock, which is safe because event payloads are detached from state at creation time.
Generated by Claude Code
Two review/CI findings on the socket-layer changes: - Delta values reference live state containers (get_value unwraps proxies without copying), so emitting the delta after releasing the state lock could serialize data mutated by the next lock holder (e.g. emit_lost_and_found awaits redis before pickling). Deltas now emit inline under the lock again; only the routing of final chained events (whose payloads are detached) is deferred past the lock. - The frontend renders non-finite floats from stdlib's NaN/Infinity tokens (pinned by test_computed_vars), which orjson silently turns into null. The wire encoder now falls back to the stdlib encoder for payloads containing non-finite floats or Decimals, detected only when the orjson output contains null at all, so typical updates skip the scan entirely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sns7EuBPYvfGCxRSRZ1bUx
|
related to my orjson pr #6116 |
masenf
left a comment
There was a problem hiding this comment.
can we decouple the orjson changes from this patch, i'd rather take those through @benedikt-bartscher more complete PR
Per maintainer request (masenf), the orjson socket-encoding change is decoupled from this PR to be taken through a more complete separate PR. This reverts the wire encoder, its non-finite-float fallback, the parity tests, the benchmark, and the orjson dependency edge -- the socket.io server encodes with format.json_dumps as before. The remaining per-event optimizations are unaffected: per-connection header/client-IP caching, cached route resolution, and deferring chained-event routing until after the state lock is released. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sns7EuBPYvfGCxRSRZ1bUx
|
Done — dropped the orjson wire encoder (and its parity tests, benchmark, and dependency edge) in 0c8d27e, so it can go through @benedikt-bartscher's PR instead. This PR now scopes to just the per-connection header cache, route LRU, and deferring chained-event routing past the state lock (delta emit stays under the lock, as discussed above). Generated by Claude Code |
All Submissions:
Type of change
Changes To Core Features:
Three per-event costs on the websocket hot path (ENG-10098):
1. Per-sid header cache (
reflex/app.py) —on_eventre-decoded every ASGI header from bytes and re-parsedx-forwarded-foron every event, though headers are fixed for a connection's lifetime. Now decoded once per sid, evicted on disconnect. A fresh dict is handed out per event sorouter_datamutations can't leak into the cache.2. Route resolution LRU (
reflex/route.py) — the router ran a linear regex scan over all routes per event for a path that rarely changes. The per-routerget_routeclosure is nowlru_cache(maxsize=1024); the route set is fixed for a router's lifetime (it was already baked in at closure creation), so no staleness is introduced.3. Deferred chained-event routing (
base_state_processor.py) — the final chained-event routing (frontend event encode+emit, backend event enqueue) ran while holding the state lock; it now runs after release. Deltas intentionally still emit under the lock: delta values reference live state containers (get_valueunwraps proxies without copying), and e.g.emit_lost_and_foundawaits redis before pickling, so a post-release emit could serialize data mutated by the next lock holder (thanks @masenf / the Codex review for flagging this — the delta-emit deferral from an earlier iteration was reverted). Chained-event payloads are proxy-detached at creation, so their deferred routing is safe. Per-token ordering is unchanged (the next event for a token dispatches only after the current task finishes, deferred routing included).Measured results (Python 3.13, real before/after code with stubbed config):
Existing processor/route/app unit tests plus the integration suites cover the behavior.
Linear: ENG-10098
🤖 Generated with Claude Code
https://claude.ai/code/session_01Sns7EuBPYvfGCxRSRZ1bUx