perf: preload the app and fork production granian workers - #7079
perf: preload the app and fork production granian workers#7079FarhanAliRaza wants to merge 1 commit into
Conversation
On Linux the production backend now forces the "fork" start method, imports the app in the supervisor, and freezes the GC before granian spawns workers, so every worker shares the framework and app pages copy-on-write instead of re-importing ~1,800 modules privately. A forked child inherits the telemetry ThreadPoolExecutor without its worker thread, so events submitted in a worker would never be sent; an at-fork hook drops the inherited pool so the child creates its own. REFLEX_BACKEND_START_METHOD overrides the start method for apps that are not fork-safe. Blank app, GRANIAN_WORKERS=4, median of 3 runs (PSS over the process tree from /proc/<pid>/smaps_rollup after serving requests): before: 721 MB PSS, 692 MB private dirty, port up in 1.82 s after: 223 MB PSS, 70 MB private dirty, port up in 1.07 s Claude-Session: https://claude.ai/code/session_01CEb3ocfFLeHkjkAKKH8YCy
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.
1 issue found across 7 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="reflex/utils/telemetry.py">
<violation number="1" location="reflex/utils/telemetry.py:501">
P1: When `fork()` runs while another thread is inside `_get_telemetry_executor()`, the child inherits `_executor_lock` in its locked state. This callback clears `_executor`, so the child’s first telemetry submission blocks forever while reacquiring that lock; recreate `_executor_lock` in the child as well.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| global _executor | ||
| _executor = None |
There was a problem hiding this comment.
P1: When fork() runs while another thread is inside _get_telemetry_executor(), the child inherits _executor_lock in its locked state. This callback clears _executor, so the child’s first telemetry submission blocks forever while reacquiring that lock; recreate _executor_lock in the child as well.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/utils/telemetry.py, line 501:
<comment>When `fork()` runs while another thread is inside `_get_telemetry_executor()`, the child inherits `_executor_lock` in its locked state. This callback clears `_executor`, so the child’s first telemetry submission blocks forever while reacquiring that lock; recreate `_executor_lock` in the child as well.</comment>
<file context>
@@ -496,6 +496,16 @@ def _get_telemetry_executor() -> ThreadPoolExecutor:
+def _reset_executor_after_fork() -> None:
+ """Drop the inherited executor; its worker thread does not exist in the child."""
+ global _executor
+ _executor = None
+
</file context>
| global _executor | |
| _executor = None | |
| global _executor, _executor_lock | |
| _executor = None | |
| _executor_lock = threading.Lock() |
Merging this PR will not alter performance
Comparing Footnotes
|
masenf
left a comment
There was a problem hiding this comment.
nice improvement.
the telemetry stuff does slightly make me nervous though. i think it's probably worth it to defer sending the "run" telemetry until after the fork and attempt to drain the executor (which theoretically should be empty on the run path if we defer the cli telemetry call). forking with threads is more than just a warning, it can lead to weird and hard-to-debug issues; if we can avoid it, then we should
Summary
run_granian_backend_prodnow forces theforkstart method, imports the app in the supervisor, and runsgc.collect(); gc.freeze()before granian starts workers. Workers share the framework and app pages copy-on-write instead of each re-importing ~1,800 modules.REFLEX_BACKEND_START_METHOD(fork/spawn/forkserver) overrides the choice for apps that are not fork-safe. Unset keeps the interpreter default on macOS and Windows (spawn), so nothing changes there.ThreadPoolExecutorbut not its worker thread, so any event submitted from a worker would queue forever. Anos.register_at_forkhook drops the inherited pool in the child. This also affected dev mode on Python <= 3.13, whereforkwas already the Linux default.Measurement
Blank
reflex initapp,GRANIAN_WORKERS=4,reflex run --env prod --backend-only, Python 3.14, granian 2.8.1. PSS summed over the whole process tree from/proc/<pid>/smaps_rollup, after 32/pingrequests, median of 3 runs.Per worker: 137 MB PSS → 41 MB PSS. The supervisor goes from a 137 MB private copy to 47 MB PSS with 172 MB shared.
Functional check: 8 socket.io clients against the forked workers, each sending two state events and receiving the expected deltas (
count1 then 2), no backend errors.Notes for review
telemetry.send("run-prod")leaves the telemetry thread alive at fork time, so Python emits its hidden-by-defaultDeprecationWarningabout forking a multi-threaded process. This is the same situation dev mode has had on Python <= 3.13. Draining the executor before forking would block startup on the telemetry HTTP request, so I did not add that.workers_max_rss) fork from the supervisor again, which still holds the pristine preloaded app.cpu × 2 + 1) and the lazy-import work in Reduce dev startup memory by deferring unused integrations #7049 are separate; this PR gains more once Reduce dev startup memory by deferring unused integrations #7049 shrinks the shared image, but has no code dependency on it.