perf: fork the dev backend reload worker instead of using a forkserver - #7113
perf: fork the dev backend reload worker instead of using a forkserver#7113FarhanAliRaza wants to merge 3 commits 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
On Python 3.14 Linux the default start method is forkserver, so the `reflex run` supervisor kept a forkserver and resource tracker alive and the reload worker re-imported the whole framework privately. The dev path now follows the same platform rule as the production supervisor: force fork where the interpreter already defaults to fork or forkserver, keep spawn under REFLEX_STRICT_HOT_RELOAD, and honour REFLEX_BACKEND_START_METHOD. The start method is fixed before the compile pool starts, otherwise that pool creates the forkserver pair first and it survives for the session. The supervisor freezes its heap before granian forks, but never imports the user app, so every reload still loads it fresh in the worker. Measured on a blank app (PSS over the session tree, Python 3.14, on top of the prod fork branch): python 330 MB -> 201 MB (supervisor 152 -> 105, worker 151 -> 96, forkserver pair 25 -> 0), whole dev tree cold ~1110 -> ~990 MB and warm ~740 -> ~610 MB, backend ready 4.4 s -> 3.0 s. Claude-Session: https://claude.ai/code/session_015Gi5wTWZNBA61pVDdPLu8u
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. |
|
| if environment.REFLEX_STRICT_HOT_RELOAD.get(): | ||
| multiprocessing.set_start_method("spawn", force=True) | ||
| elif (start_method := _backend_start_method()) is not None: | ||
| multiprocessing.set_start_method(start_method, force=True) |
There was a problem hiding this comment.
When both variables are set, REFLEX_STRICT_HOT_RELOAD forces spawn before _backend_start_method() can read REFLEX_BACKEND_START_METHOD. As a result, an explicit REFLEX_BACKEND_START_METHOD=fork or forkserver setting is silently ignored in development, and the backend uses different process semantics than requested. The tests also omit this conflicting-variable case.
| if environment.REFLEX_STRICT_HOT_RELOAD.get(): | |
| multiprocessing.set_start_method("spawn", force=True) | |
| elif (start_method := _backend_start_method()) is not None: | |
| multiprocessing.set_start_method(start_method, force=True) | |
| if (start_method := environment.REFLEX_BACKEND_START_METHOD.get()) is not None: | |
| multiprocessing.set_start_method(start_method, force=True) | |
| elif environment.REFLEX_STRICT_HOT_RELOAD.get(): | |
| multiprocessing.set_start_method("spawn", force=True) | |
| elif (start_method := _backend_start_method()) is not None: | |
| multiprocessing.set_start_method(start_method, force=True) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9824978dbc
ℹ️ 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".
| if multiprocessing.get_start_method() in ("fork", "forkserver"): | ||
| return "fork" |
There was a problem hiding this comment.
Avoid forcing fork after starting the telemetry thread
On Linux with Python 3.14 and default telemetry enabled, this converts the safer forkserver default to fork, but both _run_dev and _run_prod call telemetry.send() immediately before Granian serves, permanently starting the reflex-telemetry worker thread. Granian therefore forks a multithreaded supervisor, which Python explicitly warns may deadlock; resetting _executor only after the fork cannot repair locks inherited while another thread held them. Drain and shut down telemetry before forking, defer telemetry until workers exist, or retain a thread-safe start method.
Useful? React with 👍 / 👎.
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
4 issues found and verified against the latest diff
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">
P2: When a first telemetry submission races with Granian's fork, the child can inherit `_executor_lock` held by the vanished parent thread. Reset `_executor_lock` in the child callback along with `_executor` to prevent reload startup from hanging on the next telemetry send.</violation>
</file>
<file name="tests/units/test_telemetry.py">
<violation number="1" location="tests/units/test_telemetry.py:810">
P3: This test never forks, so it only proves `_reset_executor_after_fork()` nulls the global and a new executor is created; it does not exercise the `os.register_at_fork(after_in_child=...)` registration that this PR depends on. Removing that registration would leave the test green. Consider a child-process/integration check that verifies the inherited pool is dropped after an actual fork, or at minimum assert the registration is present.</violation>
</file>
<file name="reflex/utils/exec.py">
<violation number="1" location="reflex/utils/exec.py:805">
P1: On Python 3.14 Linux with telemetry enabled, forcing `fork` makes Granian fork a supervisor that already has the telemetry worker thread, so inherited locks can deadlock the backend. Retain `forkserver` when it is the interpreter default, or drain telemetry before forking.</violation>
<violation number="2" location="reflex/utils/exec.py:820">
P2: When `REFLEX_STRICT_HOT_RELOAD` and `REFLEX_BACKEND_START_METHOD` are both set, `set_dev_start_method` always forces `spawn` and ignores the explicit method. Check the explicit backend method first, then use strict hot reload as the fallback.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # Python defaults to fork (<3.14) or forkserver (3.14+) on Linux and to | ||
| # spawn elsewhere; only where fork is already the platform norm do we rely | ||
| # on it so workers can share the supervisor's pages. | ||
| if multiprocessing.get_start_method() in ("fork", "forkserver"): |
There was a problem hiding this comment.
P1: On Python 3.14 Linux with telemetry enabled, forcing fork makes Granian fork a supervisor that already has the telemetry worker thread, so inherited locks can deadlock the backend. Retain forkserver when it is the interpreter default, or drain telemetry before forking.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/utils/exec.py, line 805:
<comment>On Python 3.14 Linux with telemetry enabled, forcing `fork` makes Granian fork a supervisor that already has the telemetry worker thread, so inherited locks can deadlock the backend. Retain `forkserver` when it is the interpreter default, or drain telemetry before forking.</comment>
<file context>
@@ -785,6 +789,66 @@ def run_uvicorn_backend_prod(
+ # Python defaults to fork (<3.14) or forkserver (3.14+) on Linux and to
+ # spawn elsewhere; only where fork is already the platform norm do we rely
+ # on it so workers can share the supervisor's pages.
+ if multiprocessing.get_start_method() in ("fork", "forkserver"):
+ return "fork"
+ return None
</file context>
| if multiprocessing.get_start_method() in ("fork", "forkserver"): | |
| + if multiprocessing.get_start_method() == "fork": |
| global _executor | ||
| _executor = None |
There was a problem hiding this comment.
P2: When a first telemetry submission races with Granian's fork, the child can inherit _executor_lock held by the vanished parent thread. Reset _executor_lock in the child callback along with _executor to prevent reload startup from hanging on the next telemetry send.
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 a first telemetry submission races with Granian's fork, the child can inherit `_executor_lock` held by the vanished parent thread. Reset `_executor_lock` in the child callback along with `_executor` to prevent reload startup from hanging on the next telemetry send.</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() |
| if environment.REFLEX_STRICT_HOT_RELOAD.get(): | ||
| multiprocessing.set_start_method("spawn", force=True) | ||
| elif (start_method := _backend_start_method()) is not None: | ||
| multiprocessing.set_start_method(start_method, force=True) |
There was a problem hiding this comment.
P2: When REFLEX_STRICT_HOT_RELOAD and REFLEX_BACKEND_START_METHOD are both set, set_dev_start_method always forces spawn and ignores the explicit method. Check the explicit backend method first, then use strict hot reload as the fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At reflex/utils/exec.py, line 820:
<comment>When `REFLEX_STRICT_HOT_RELOAD` and `REFLEX_BACKEND_START_METHOD` are both set, `set_dev_start_method` always forces `spawn` and ignores the explicit method. Check the explicit backend method first, then use strict hot reload as the fallback.</comment>
<file context>
@@ -785,6 +789,66 @@ def run_uvicorn_backend_prod(
+ """
+ import multiprocessing
+
+ if environment.REFLEX_STRICT_HOT_RELOAD.get():
+ multiprocessing.set_start_method("spawn", force=True)
+ elif (start_method := _backend_start_method()) is not None:
</file context>
| if environment.REFLEX_STRICT_HOT_RELOAD.get(): | |
| multiprocessing.set_start_method("spawn", force=True) | |
| elif (start_method := _backend_start_method()) is not None: | |
| multiprocessing.set_start_method(start_method, force=True) | |
| if (start_method := environment.REFLEX_BACKEND_START_METHOD.get()) is not None: | |
| multiprocessing.set_start_method(start_method, force=True) | |
| elif environment.REFLEX_STRICT_HOT_RELOAD.get(): | |
| multiprocessing.set_start_method("spawn", force=True) | |
| elif (start_method := _backend_start_method()) is not None: | |
| multiprocessing.set_start_method(start_method, force=True) |
| """A forked child drops the inherited pool, whose thread it does not own.""" | ||
| inherited = telemetry._get_telemetry_executor() | ||
|
|
||
| telemetry._reset_executor_after_fork() |
There was a problem hiding this comment.
P3: This test never forks, so it only proves _reset_executor_after_fork() nulls the global and a new executor is created; it does not exercise the os.register_at_fork(after_in_child=...) registration that this PR depends on. Removing that registration would leave the test green. Consider a child-process/integration check that verifies the inherited pool is dropped after an actual fork, or at minimum assert the registration is present.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/test_telemetry.py, line 810:
<comment>This test never forks, so it only proves `_reset_executor_after_fork()` nulls the global and a new executor is created; it does not exercise the `os.register_at_fork(after_in_child=...)` registration that this PR depends on. Removing that registration would leave the test green. Consider a child-process/integration check that verifies the inherited pool is dropped after an actual fork, or at minimum assert the registration is present.</comment>
<file context>
@@ -801,3 +801,14 @@ def test_flush_returns_false_when_worker_does_not_drain_in_time():
+ """A forked child drops the inherited pool, whose thread it does not own."""
+ inherited = telemetry._get_telemetry_executor()
+
+ telemetry._reset_executor_after_fork()
+
+ fresh = telemetry._get_telemetry_executor()
</file context>
Stacked on #7079 (its commit is included in this diff until it merges). Follow-up to #7112, which measured this change and dropped it so the fork helpers land once, in #7079.
Summary
reflex runnow applies the same start-method rule as the prod supervisor:forkwhere the interpreter already defaults toforkorforkserver(Linux),spawnunderREFLEX_STRICT_HOT_RELOAD, andREFLEX_BACKEND_START_METHODoverrides both. macOS and Windows keepspawn, so nothing changes there.forkserver, so the supervisor kept a forkserver plus resource tracker alive and the single reload worker re-imported the whole framework privately. Now granian forks the worker from the supervisor and framework pages are shared copy-on-write._compile_appbefore the compileProcessPoolExecutorstarts. Otherwise that pool creates the forkserver pair first and it survives for the whole session.gc.collect(); gc.freeze()before granian forks (shared_freeze_for_fork, also used by the prod preload), but never imports the user app. Every reload still imports it fresh in the worker. Verified with a route that changed between reloads: the new value was served 0.5 s after the edit, and the tree stays at two Python processes.reflex.appin the supervisor only when the start method is alreadyfork. With this PR that condition holds on 3.14, so the two compose.Measurement
Blank app,
reflex run, page loaded once in headless Chrome, PSS over the session tree from/proc/<pid>/smaps_rollup, 15 s settle, Python 3.14, granian 2.8.1. Before is the #7079 branch as-is; after adds this commit. Node numbers are unchanged by this PR (see #7112 for the vite side).Dev runs one worker, so this is a fixed ~130 MB and ~1.4 s, not a per-worker multiple. On Python 3.13 and older, Linux already forks, so the only change there is the freeze.
Test plan
uv run pytest tests/units(8353 passed, 76.15% coverage)uv run pre-commit run --all-filesREFLEX_STRICT_HOT_RELOAD=1still spawnshttps://claude.ai/code/session_015Gi5wTWZNBA61pVDdPLu8u