Reduce dev startup memory by deferring unused integrations - #7049
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Greptile SummaryThis PR reduces development startup time and memory by deferring optional integrations, avoiding unnecessary database/compiler imports, and preloading application modules only for forked Granian workers. It also preserves optional serializer compatibility, SQLModel relationship serialization, state mutation tracking, and database usage accounting.
Confidence Score: 5/5The PR appears safe to merge, with no actionable new correctness, security, or repository-rule violations identified. The latest changes preserve serializer precedence and compatibility without mutating registries during lookup, bound fork-sensitive Plotly initialization to integrations already loaded, and keep database accounting consistent with its model-base semantics.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/utils/serializers.py | Replaces lazy registry mutation with identity-based optional serializer resolution while preserving override and inheritance precedence. |
| packages/reflex-base/src/reflex_base/utils/_serializer_types.py | Adds runtime-only resolution of optional serializer annotation types through a fixed internal mapping. |
| reflex/istate/proxy.py | Recognizes already-loaded SQLAlchemy and Pydantic model classes without eagerly importing their packages. |
| reflex/utils/exec.py | Restricts app preloading to non-strict Granian development workers using the fork start method. |
| reflex/utils/telemetry_accounting.py | Counts loaded database model bases without importing Reflex's optional database integration. |
| reflex/model.py | Preserves the historical SQLModel serializer export while sharing the deferred serializer implementation. |
| reflex/utils/path_ops.py | Hosts the lightweight unchanged-content-aware file-writing helper. |
| tests/units/reflex_base/utils/test_lazy_serializer_regressions.py | Adds isolated regressions for SQLModel serialization, custom overrides, registry reentrancy, and fork-time optional imports. |
Reviews (4): Last reviewed commit: "Fix cold serializer compatibility and de..." | Re-trigger Greptile
Merging this PR will improve performance by 15.98%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_import_reflex |
4.9 ms | 4.2 ms | +15.98% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing codex/dev-mode-import-performance (4ee7a1c) with main (c49a85d)
Footnotes
-
8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
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
There was a problem hiding this comment.
2 issues found across 9 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="packages/reflex-base/src/reflex_base/utils/serializers.py">
<violation number="1" location="packages/reflex-base/src/reflex_base/utils/serializers.py:608">
P2: When a SQLModel class is checked for ObjectVar support, `can_use_in_object_var` rejects it because this `dict[str, Any]` marker does not equal the requested `dict` type. Normalize generic-dict output types in `has_serializer`, or keep the SQLModel marker compatible with the existing `dict` check.</violation>
</file>
<file name="tests/units/utils/test_telemetry_accounting.py">
<violation number="1" location="tests/units/utils/test_telemetry_accounting.py:109">
P2: This assertion fails: only CustomBase is a nonempty registered base, so get_models() (include_empty=False) filters out EmptyBase and returns length 1, not 2. _get_db_model_count() also calls get_models() with the same default, so the chain resolves to 1 == 1 == 2, which is False. Change the expected count to 1 (or register a second nonempty base).</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| "Figure": dict, | ||
| "Template": dict, | ||
| "Image": str, | ||
| "SQLModel": dict[str, Any], |
There was a problem hiding this comment.
P2: When a SQLModel class is checked for ObjectVar support, can_use_in_object_var rejects it because this dict[str, Any] marker does not equal the requested dict type. Normalize generic-dict output types in has_serializer, or keep the SQLModel marker compatible with the existing dict check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/utils/serializers.py, line 608:
<comment>When a SQLModel class is checked for ObjectVar support, `can_use_in_object_var` rejects it because this `dict[str, Any]` marker does not equal the requested `dict` type. Normalize generic-dict output types in `has_serializer`, or keep the SQLModel marker compatible with the existing `dict` check.</comment>
<file context>
@@ -571,85 +554,70 @@ def serialize_image(image: _serializer_types.Image) -> str:
+ "Figure": dict,
+ "Template": dict,
+ "Image": str,
+ "SQLModel": dict[str, Any],
}
-_OPTIONAL_SERIALIZER_FUNCTIONS = (
</file context>
| class EmptyBase(DeclarativeBase): | ||
| pass | ||
|
|
||
| assert telemetry_accounting._get_db_model_count() == len(ModelRegistry.get_models()) == 2 |
There was a problem hiding this comment.
P2: This assertion fails: only CustomBase is a nonempty registered base, so get_models() (include_empty=False) filters out EmptyBase and returns length 1, not 2. _get_db_model_count() also calls get_models() with the same default, so the chain resolves to 1 == 1 == 2, which is False. Change the expected count to 1 (or register a second nonempty base).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/utils/test_telemetry_accounting.py, line 109:
<comment>This assertion fails: only CustomBase is a nonempty registered base, so get_models() (include_empty=False) filters out EmptyBase and returns length 1, not 2. _get_db_model_count() also calls get_models() with the same default, so the chain resolves to 1 == 1 == 2, which is False. Change the expected count to 1 (or register a second nonempty base).</comment>
<file context>
@@ -23,23 +24,94 @@
+class EmptyBase(DeclarativeBase):
+ pass
+
+assert telemetry_accounting._get_db_model_count() == len(ModelRegistry.get_models()) == 2
+
+ModelRegistry.models.clear()
</file context>
| assert telemetry_accounting._get_db_model_count() == len(ModelRegistry.get_models()) == 2 | |
| assert telemetry_accounting._get_db_model_count() == len(ModelRegistry.get_models()) == 1 |
Reflex dev startup imports optional data, plotting, image, database, and admin integrations even when the app does not use them. Spawned Granian workers also cannot reuse app modules preloaded by the supervisor, leaving an unnecessary copy resident between reloads.
Resolve optional serializers from already-loaded class identities, defer unused database/admin imports, and preload the app only for forked Granian workers. Preserve custom serializer overrides, exact-type/subclass precedence, runtime annotations, and cold direct-SQLModel relationship data.
This PR now also includes the backend-only follow-up:
MUTABLE_TYPESremain supported.write_filehelper into the lightweight path-operations module, preserving its old compiler reexport. This keeps the compiler/state dependency graph out of the backend supervisor.Latest backend-only measurements
Database-free blank app, Granian dev mode, macOS on Apple M5 Pro, Python 3.14.5, with optional dependencies installed. These are additional improvements over this PR's initial commit, measured again on both sides:
Each side uses 5 fresh starts and 15 reloads; values are medians. The final source was verified unchanged after benchmarking. Startup ranged from 332.6–351.9 ms, reloads from 380.4–421.2 ms, and post-reload RSS from 163.91–164.28 MiB.
After the regression fixes in
b2bf7009, a fresh 5-start/15-reload check measured 164.00 MiB startup RSS, 338.6 ms startup, 400.5 ms reload, and 164.03 MiB post-reload RSS. This retains the earlier savings; it is a separate-session sanity check, not an interleaved A/B benchmark.After the second-audit fixes, an alternating same-session comparison against
b2bf7009(5 fresh starts and 15 reloads per revision, separate excluded warm-ups) measured 377.35 → 372.56 ms startup, 428.70 → 429.55 ms reload, 164.66 → 164.34 MiB startup RSS, and 164.59 → 164.41 MiB post-reload RSS. This small sample shows no noticeable regression; it does not establish a new speedup. Parent and spawned-worker import paths were verified for each revision. A separate final standalone run measured 163.89 MiB startup RSS.For context, the earlier measurement of the first commit showed:
The original baseline was
c57b32cc8b0cbcb61ffe303b18a94f5cb3ca1a10; the follow-up baseline wascc9107032f71aa83b31427764d3cd2a0694899ec. The initial-commit values above come from separate benchmark passes, so small timing differences are expected.Startup waits for
/ping; reload timing starts at a Python source-file touch and waits for a replacement worker and a successful/ping. The compile-skip marker is removed before each fresh start. Telemetry and version checks are disabled on both sides.RSS sums the supervisor, backend worker, and multiprocessing helper; it excludes Vite/browser memory and can double-count shared pages. Savings depend on installed and used integrations; database-using apps still pay their database import cost. These are fresh-process launches with warm filesystem caches. Linux/fork, other Python versions, large-app compile time, production performance, and long-duration leak freedom have not been established.
Validation
The latest fixes address all eight findings from the second adversarial audit:
reflex.modelno longer silently replaces a custom SQLModel serializer or broad custom fallback. This deliberately removes the old import-order-dependent override; both cases have regression coverage.pandasdo not trigger dependency imports.orjsonimports.Validation of the current fixes:
orjsoncase skips because that package is not installed in the Python 3.14 environment).orjsoninstalled; the JSON-helper andorjsonfork cases also passed 8 fresh-process repeats each.pandasnow compiles on both revisions.orjsonhang.The full Playwright, docs, and public-app runs preceded the final narrowly scoped Plotly JSON-engine warm-up adjustment. The full unit suite, focused cross-environment fork tests, and startup benchmark were rerun after it.
Earlier broad validation on
46f9c7bfincluded large-state apps through 10,000 fields and normal/strict Granian plus Uvicorn reloads. Its selected state/event matrix reached 98 distinct passing cases and 10 skips across initial and isolated runs; failures in the initial combined batch also reproduced on base (including macOS select-all and fixture-isolation issues).CI on preceding head
b2bf7009finished with 109 successes and 2 skips, including docs production checks. New-commit CI must run separately. Local full docs frontend/browser validation still requires Enterprise login; backend and docs-unit checks are not substitutes for it.The fork tests cover the identified deferred-import races, not a blanket guarantee that arbitrary third-party code is safe to fork from a multithreaded process. Direct mutation of internal serializer registries is not a supported compatibility contract.
Changes to core features
types.py.