Skip to content

Reduce dev startup memory by deferring unused integrations - #7049

Open
Alek99 wants to merge 4 commits into
mainfrom
codex/dev-mode-import-performance
Open

Reduce dev startup memory by deferring unused integrations#7049
Alek99 wants to merge 4 commits into
mainfrom
codex/dev-mode-import-performance

Conversation

@Alek99

@Alek99 Alek99 commented Sep 5, 2026

Copy link
Copy Markdown
Member

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:

  • State mutation tracking recognizes already-loaded model classes without eagerly importing SQLAlchemy. Model import order, cached classification, and explicit/wildcard access to MUTABLE_TYPES remain supported.
  • Move the pure write_file helper 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:

Metric Initial commit, remeasured With follow-up Additional improvement
Startup process-tree RSS 209.11 MiB 163.95 MiB 21.6% lower
Startup to ready 456.1 ms 336.5 ms 26.2% faster
Python reload to ready 524.8 ms 398.4 ms 24.1% faster
Post-reload process-tree RSS 209.13 MiB 164.02 MiB 21.6% lower

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:

Metric Original framework Initial PR commit
Process-tree RSS 375.27 MiB 209.13 MiB
Startup to ready 0.906 s 0.446 s
Python reload to ready 1.070 s 0.533 s

The original baseline was c57b32cc8b0cbcb61ffe303b18a94f5cb3ca1a10; the follow-up baseline was cc9107032f71aa83b31427764d3cd2a0694899ec. 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:

  • Preserve SQLModel relationship data in cold state/event payloads, and preserve the historical serializer import, keyword argument, annotations, and pickle path.
  • A later compatibility import of reflex.model no 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.
  • Recognize actual optional-library classes by identity, so unrelated user packages named pandas do not trigger dependency imports.
  • Resolve defaults without importing libraries or rebuilding the registry. This removes the global serializer lock, destructive reordering, custom-hash lock inversion, and reentrant registration loss.
  • Finish Plotly's deferred JSON/engine initialization before fork only when Plotly Figure/Template classes are already loaded. Regression tests overlap fork with Template, JSON-helper, and optional orjson imports.
  • Count directly imported SQLModel database usage without loading the unused database integration.
  • Bound all serializer subprocess tests with timeouts.

Validation of the current fixes:

  • Final full unit suite: 8,360 passed, 19 skipped; 75.96% coverage on Python 3.14.5.
  • Focused serializer modules: 88 passed, 1 skipped (the explicit orjson case skips because that package is not installed in the Python 3.14 environment).
  • Two additional test-only broad-fallback cases subsequently passed; the expanded cold-process module has 15 passing cases and 1 optional-orjson skip on Python 3.14.
  • Final cold-process regression module: 16 passed on Python 3.12.13 with orjson installed; the JSON-helper and orjson fork cases also passed 8 fresh-process repeats each.
  • Plotly auto/json/orjson configurations and simulated unavailable-orjson fallback passed. No installed dependencies were changed for the fallback checks.
  • Full Playwright suite: 253 passed.
  • Docs suite on Python 3.12: 419 passed, 1 skipped, 1 expected failure.
  • Real docs backend: 8 WebSocket assertions across initial startup and 3 worker reloads.
  • Public counter, SQLite CRUD, and Excel/pandas/SQLModel data-visualization apps: all 8 dev/production runs passed, including browser interactions, reload/state survival, and a separate fresh Excel ingestion fixture.
  • 100 seeded base/current comparisons (6,700 mixed operations per revision) matched exactly. Missing optional dependencies, annotations, pickle, and repeated module reload checks passed; a real user app named pandas now compiles on both revisions.
  • Independent review also matched 624 optional/default/override inheritance combinations against the original base.
  • All configured pre-commit hooks passed: Ruff, codespell, stub generation, Pyright, ty, and Biome.
  • Regression failures were captured before implementing their fixes, including removing only the final warm-up call to reproduce the orjson hang.
  • News fragments included for both touched packages.

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 46f9c7bf included 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 b2bf7009 finished 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

Review in cubic

@Alek99
Alek99 requested a review from a team as a code owner September 5, 2026 21:25
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@greptile-apps

greptile-apps Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This 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.

  • Resolves optional serializers from already-loaded class identities without eagerly importing integrations.
  • Adds pre-fork Plotly initialization for integrations already in use.
  • Lazily recognizes mutable model classes and defers database/admin imports.
  • Moves the lightweight file-writing helper into path operations while retaining its compiler re-export.
  • Adds extensive cold-process, import-order, concurrency, and compatibility regression coverage.

Confidence Score: 5/5

The 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.

Important Files Changed

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

@codspeed-hq

codspeed-hq Bot commented Sep 5, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 15.98%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 31 untouched benchmarks
⏩ 8 skipped benchmarks1

Performance Changes

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)

Open in CodSpeed

Footnotes

  1. 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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 15 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread tests/units/utils/test_serializers.py

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
assert telemetry_accounting._get_db_model_count() == len(ModelRegistry.get_models()) == 2
assert telemetry_accounting._get_db_model_count() == len(ModelRegistry.get_models()) == 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant