improve compile perf - #6804
Conversation
Greptile SummaryThis PR replaces the node-by-node
Confidence Score: 5/5Safe to merge; the hash output is unchanged and the new encoding path is well-tested. The change is a pure performance optimization with no behavioral change to the resulting hashes. The encoder dispatch table and frozen-dataclass identity cache are logically sound, the LRU eviction is correct, and the test suite covers collisions, subclass normalization, cache hits, eviction, and hash stability. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/components/component.py | Replaces the incremental hasher-update approach with a buffer-based encoder dispatch table; introduces a frozen-dataclass encoding cache and a type-to-encoder memoization dict. Logic is correct for all tested types; two minor design concerns noted. |
| tests/units/components/test_component.py | Adds comprehensive tests for the new encoding path: collision detection, dict-ordering normalization, subclass normalization, frozen-dataclass caching, LRU eviction, and hash stability across eviction cycles. |
| packages/reflex-base/news/6804.performance.md | Changelog entry accurately describing the ~3.7x speedup and the unchanged hash output guarantee. |
Reviews (5): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile
Merging this PR will improve performance by 5.27%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
2 issues found across 2 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="tests/units/components/test_component.py">
<violation number="1" location="tests/units/components/test_component.py:2475">
P3: This test's name and docstring claim it verifies that the frozen-dataclass encoding cache is reused by identity, but the two equality assertions would pass even if no cache existed at all (equal-but-distinct instances always encode to the same bytes). Either rename/reword it as a plain equality contract test, or actually exercise the cache path, e.g. clear component._ENCODED_DATACLASSES, hash `shared` once, then assert id(shared) is present in the cache and that a second hash of the same object short-circuits.</violation>
</file>
<file name="packages/reflex-base/src/reflex_base/components/component.py">
<violation number="1" location="packages/reflex-base/src/reflex_base/components/component.py:688">
P2: The new `_ENCODED_DATACLASSES` cache pins up to 8192 frozen dataclass instances and copies of their encoded bytes in a module-global for the whole process lifetime, released only by a full `clear()` once the cap is hit. Because `_deterministic_hash` runs for every component hash during a compile, these scalar-only frozen instances (and their otherwise-transient byte encodings) no longer get garbage-collected, adding persistent memory that was not retained before. The full-clear-on-capacity eviction also drops the entire cache at once and forces re-encoding of the whole working set right at the boundary, so the cache both holds memory and thrashes. Consider evicting/limiting per-entry (e.g. only cache the bytes keyed by object identity while bounding total retained bytes, or evict entries gradually instead of a wholesale clear).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
No description provided.