feat(zql): make applyChange immutable for React.memo optimization#5462
feat(zql): make applyChange immutable for React.memo optimization#5462arv merged 39 commits intorocicorp:mainfrom
Conversation
|
Someone is attempting to deploy a commit to the Rocicorp Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks, amazing. Have you actually done the manual test yet? This is going to require careful review. These files are super sensitive and have been a source of subtle bugs in the past. One of us is going to have to set aside a few hours to work through it. Maybe next week? Appreciate the contribution. |
Nope, was giving this a shot using Ralph loop and Claude hahaha. Not ready for review yet! Will do manual reviews. I'm not super comfortable with Solid so it definitely needs some testing on that front (there are bits in the code that I don't understand too well). |
|
How does this handle the case where every single row changes? Does it intelligently replace the view or does it path-copy for every single modification? It looks like it does the latter. On a quick scan (didn't think too hard since the PR is still draft), it looks like the current PR is in this situation -- The parent spreads at each level: return {...parentEntry, [relationship]: newView}; So for a single change at depth D in a tree with array sizes N₁, N₂, ... Nᴅ:
For K changes in one transaction, each touching the same level-1 array of size N:
|
Ah, good point! I think I can optimize this by batching mutations. |
I tried to implement batching to get O(N + K) complexity but ran into a blocker. The issue is that consume(source.push({type: 'add', ...}));
expect(view.data).toEqual(...); // no flush() callTrue batching would mean That said, I ran some benchmarks and the O(K × N) cost might be fine in practice:
Even the worst case (N=2000, K=200) is under 0.1ms, which is way under the 16ms frame budget. On a slow device (10x multiplier), still under 1ms. The upside is that immutability preserves reference identity for unchanged items, so React.memo / Solid reactivity can skip re-rendering rows that didn't change. Not sure if that fully justifies the complexity tradeoff, but it seems reasonable? |
|
Hey @tantaman, figured out a way to make batching work! Added an auto-flush safety net in the
Ran benchmarks comparing all 3 approaches:
At larger scales the batched approach actually beats mutable because it avoids K splice operations that each shift elements. Wasn't expecting that! All 2202 zql tests pass with the new implementation. |
Enables React.memo with shallow comparison by preserving object identity for unchanged rows. Removes the need for deepClone in React consumer. Changes: - applyChange now returns Entry instead of void - Uses toSpliced/with for immutable array operations - Uses spread for immutable object operations - Unchanged rows keep their reference identity - ArrayView stores returned value from applyChange - React: removes deepClone, unchanged rows preserve identity - Solid: uses reconcile instead of produce Tests: - Added 8 object identity tests using toBe (reference equality) - All 1184 zql tests passing - All 37 React tests passing - All 15 Solid tests passing Closes rocicorp#3036
Why: Make the immutable update pattern easier to understand for future maintainers and improve type safety by reducing unchecked casts. * Add ASCII diagrams explaining immutable propagation from leaf to root * Add ASCII diagrams showing object identity preservation for React.memo * Add inline comments explaining each change type (add/remove/edit/child) * Create asMutableArray() helper for TypeScript's readonly array limitation * Create getOptionalSingularEntry() type guard to reduce `as` casts * Create getChildEntryList() helper with proper typing
Why: Runtime type guards (isMetaEntry, assertMetaEntry, etc.) add function call overhead on every operation. Since we control all entry creation paths, direct casts are safe and faster. * Replace helper functions with inline `as` casts throughout * Add ES2023.Array to tsconfig for with()/toSpliced() types * Add detailed comments explaining previous vs current Solid behavior * Document cast safety rationale in file header comment
* Add entries() and at() test helpers to avoid repetitive casts * Replace all (x['key'] as Entry[])[i] patterns with at(x, 'key', i)
Remove unnecessary readonly modifier from MetaEntryList since toSpliced/with methods aren't defined on ReadonlyArray in TS. This eliminates ~8 casts throughout the file.
The test was using the apply() helper function but it wasn't defined in that test block's scope.
Improve performance and code clarity based on oracle review: * Return pos from add() to eliminate O(n) indexOf scan after insertion * Optimize initializeRelationships: build arrays in-place with mutable splice for non-hidden plural relationships, avoiding intermediate parent object allocations on large fan-out * Condense comments throughout: keep ASCII visualizations, remove verbose explanations, make helper docs single-line * Add tests for add with initialized relationships to verify pos-based array updates work correctly
Remove before/after comparisons, keep compact explanations with inline ASCII example showing how reconcile merges properties.
solid-view.ts: * Add flowchart showing Solid update flow (push → commit → reconcile) * Remove React.memo reference (wrong framework) view-apply-change.ts: * Make doc framework-agnostic (React.memo, Solid, etc.) * Add ASCII state diagram for refCount lifecycle in ADD * Improve EDIT comment with "why ghost stays" explanation * Better immutable propagation path visualization
Remove verbose before/after explanation, keep compact "why" focused comment.
* Remove "runtime assertions" meta-comment (noise) * Restore good original comments for special case and move logic * Keep ASCII viz that adds value
view-apply-change.test.ts: * Add section headers with ASCII diagrams explaining test categories * Add per-test ASCII diagrams showing state transitions * Visualize refCount lifecycle, identity preservation, edit moves solid-view.ts: * Add context explaining the challenge (batching for perf) * Document the two code paths with concrete perf numbers * Simplify the ASCII flowchart
This reverts commit 5fd2799.
Test app demonstrating that immutable applyChange works with SolidJS. Includes render count badges to visually verify only modified rows re-render when data changes. Results documented in RENDER-TEST-RESULTS.md showing: - Modified row: render count 1 → 2 - Other rows: render count stays at 1
- Rename app from zsolid to zbugs-lite-solid - Remove local shared/ folder with duplicate schema - Create src/zero.ts that imports schema, mutators, queries from zbugs - App is now a pure Solid rendering layer on top of zbugs Zero infra
Buffer changes in push() and apply them all at once in flush(), achieving O(N + K) complexity instead of O(K × N) when K changes arrive in one transaction. Key changes: - Add #pendingChanges buffer to queue ViewChanges - Eagerly expand lazy relationship generators at push time to capture source state - Apply batched changes in flush() using applyChanges() - Add auto-flush safety net in .data getter for backwards compatibility - Export ExpandedNode/ViewNode types and getChildNodes helper The auto-flush ensures existing code that reads .data without explicit flush() continues to work, while code that follows the push/flush pattern benefits from O(N + K) batching.
9f92632 to
ad7386c
Compare
CleanShot.2026-02-12.at.10.54.15.mp4This works great. Look at those render counts!!! |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@devagrawal09 FYI |
|
Thanks for the ping @arv. Happy to see that the |
Remove the recursive eager expansion of relationship thunks that was introduced in rocicorp#5462. Instead of buffering expanded changes and applying them in batch at flush time, apply each change immediately in push() using the immutable applyChange. This preserves correctness (thunks are evaluated while source state is current) and eliminates the O(tree) recursive expansion overhead. The getChildNodes() helper in view-apply-change.ts already handles lazy thunks natively, so no changes are needed there. * Remove expandNode and expandChange functions * Remove mapValues import (only used by expandNode) * Remove change buffering (#pendingChanges array, #applyPendingChanges) * Apply changes immediately in push() via applyChange (immutable) * Update test: singular format error now throws at push time, not flush * All 2212 ZQL tests pass (132 files, 2 pre-existing skips)
…tion (#5462)" (#5616) This reverts commit a40a69b. Per regressions were discovered. We will work to fix those and reland this with better benchmarks for confidence. https://rocicorp.slack.com/archives/C0A8NUDMDM1/p1771983306782649?thread_ts=1771969137.598129&cid=C0A8NUDMDM1
Immutable
applyChangewith O(N + K) BatchingMakes
applyChangereturn new objects instead of mutating in place. Unchanged rows keep their object reference, changed rows get new references. This lets React.memo and Solid reactivity actually work.Right now Zero deep clones every snapshot because
applyChangemutates in place. Every row gets a new reference on every sync, even if nothing changed.With immutable
applyChange, unchanged rows keep their reference:Performance
Started with a naive immutable implementation. @tantaman pointed out it was O(K × N) for K changes to an N-item array:
Ran benchmarks, it was ~4x slower at scale. Still under 0.1ms but felt like we could do better.
Tried batching but hit a blocker: tests expected
.datato reflect changes immediately afterpush(). Solution: auto-flush in the.datagetter.The
.datagetter auto-flushes if there are pending changes, so existing code just works.At larger scales batched beats mutable because we do not have to copy the objects over and
over
Changes
Manual Testing: React (zbugs)
To verify React.memo actually works with stable references, I modified zbugs' list page:
IssueRowoutside the component and wrapped withReact.memostyleis new object each render)Then ran the app and updated rows directly via psql while watching console:
Key insight: Zero maintains stable object references for unchanged issues. The
sameRef: true/falselogging confirmed this. When only the title changes, that row gets a new reference, but all other rows keep their original reference.Manual Testing: Solid (zbugs-lite-solid)
(this is no longer part of the PR)
Created a minimal Solid app that imports Zero schema/queries from zbugs. Added render count badges to each row.
Solid's fine-grained reactivity handles the in-place updates efficiently via produce. No special memo config needed.
Related