diff --git a/.changeset/memo-commit-pins.md b/.changeset/memo-commit-pins.md new file mode 100644 index 000000000000..101c4eab45e9 --- /dev/null +++ b/.changeset/memo-commit-pins.md @@ -0,0 +1,9 @@ +--- +'@data-client/normalizr': patch +'@data-client/core': patch +'@data-client/react': patch +--- + +Keep results that a committed render read while a transition retries + +`DataProvider` now tells the memo which store version committed, so results a committed render read stay cached through any number of pending-transition retries and optimistic updates. Custom `Controller` memos can implement the new optional `commit(entities, settled)` method; ones without it keep working. diff --git a/examples/benchmark/README.md b/examples/benchmark/README.md index d10b1067183d..d7139a1f5728 100644 --- a/examples/benchmark/README.md +++ b/examples/benchmark/README.md @@ -40,6 +40,8 @@ Both arguments are optional: - `denormalizeLong withCache update first|last row` - steady state: each op builds a new `entities` with one project replaced, then reads the cached list; the changed row is first or last in the list - `denormalizeLong Scalar` (cold) and `denormalizeLong Scalar withCache update one cell` - the 700-row `Scalar` list in a new `MemoCache`, and read again after each op replaces one `Scalar` cell - `denormalizeLong two Scalars, 25% Stocks missing` (and `withCache`) - a `Holding` list with two `Scalar` lenses (one through its `Stock`); two stores missing different Stocks are read in turn, so list nodes keep more than one `argsKey` branch + - `... withCache committed` - the `denormalizeLong`, `Values`, `Scalar` and `All` withCache cases on a `MemoCache` that has `commit()`ed the `entities` it reads, so reads take the committed path + - `denormalizeShort 500 endpoints withCache` (and `then commit`) - each op reads 500 small cached endpoints with a new `entities`, then commits it; the difference is the cost of `commit()` - `spread` - degenerate-case writes where spread cost scales with store size (single-entity `setResponse` into 1k/10k/100k entity stores, 10k cached endpoint keys, collection push onto 10k items, invalidateAll/expireAll). One case (`setOneEntity in 10k entity store`) is tracked over time in CI via `.github/workflows/benchmark-spread.yml`; the rest are manual-only. - `micro` - isolated microbenchmarks for testing specific operations - `old-normalizr` - runs equivalent benchmarks using the normalizr package @@ -124,9 +126,10 @@ yarn start:memo-tradeoffs [filter] Measures where caching fewer versions could cost reads or memory: one list read against several store snapshots in turn (hit rate and time per read), -heap the memo keeps beyond the store after a run of updates, and one list read -under many `argsKey` values (hit rate and heap held). Local-only — not tracked -in CI. +heap the memo keeps beyond the store after a run of updates, an optimistic +streak (the settled store committed, then 1, 3 or 8 unsettled commits, then +the settled store read again: hit rate and heap held), and one list read under +many `argsKey` values (hit rate and heap held). Local-only — not tracked in CI. ### Profiling diff --git a/examples/benchmark/memo-tradeoffs.js b/examples/benchmark/memo-tradeoffs.js index 75f47193bb5a..b6f39815a6ea 100644 --- a/examples/benchmark/memo-tradeoffs.js +++ b/examples/benchmark/memo-tradeoffs.js @@ -6,6 +6,9 @@ * a different snapshot. Reports how many reads hit and time per read. * - retained memory: heap the memo holds beyond the store itself, after a run * of updates. Entity churn (every row replaced) is the worst case. + * - optimistic streak: the settled store is committed, then M optimistic + * updates each render and commit unsettled, then the store settles back. + * Reports how often the settled result is still cached, and heap held. * - argsKey buckets: one list read under N different `argsKey` values (a * portfolio app switching between N portfolios), all cached, and the heap * the memo holds for them. @@ -200,6 +203,26 @@ const memoryCases = [ return store; }, }, + { + // a trade tape: old trades leave the store, so nothing should keep their + // cached denormalized rows alive + name: 'retained: 100-row trade tape, 3 new trades per flush x 600', + run(memo) { + let store; + let trades = []; + for (let flush = 0; flush < 600; flush++) { + const fresh = Array.from({ length: 3 }, (_, j) => ({ + id: flush * 3 + j, + name: `trade ${flush * 3 + j}`, + value: flush, + })); + trades = [...fresh, ...trades].slice(0, 100); + store = normalize([Row], trades); + memo.denormalize([Row], store.result, store.entities); + } + return store; + }, + }, ...[1000, 10000].map(n => ({ name: `retained: ${n}-row list refetched (every row replaced) x 20`, run(memo) { @@ -226,6 +249,46 @@ for (const { name, run } of memoryCases) { }); } +// --- optimistic streak --------------------------------------------------------- + +const STREAKS = 20; +for (const flipBack of [false, true]) { + for (const m of [1, 3, 8]) { + const name = `optimistic streak: ${ROWS}-row list, ${m} unsettled commits${flipBack ? ' and a flip back' : ''}, then the settled store x ${STREAKS}`; + if (!want(name)) continue; + let hits = 0; + const bytes = retained(memo => { + const read = state => + memo.denormalize(StockSchema, base.result, state.entities, stockArgs) + .data; + const settled = read(base); + memo.commit(base.entities, true); + for (let streak = 0; streak < STREAKS; streak++) { + // each optimistic update renders and commits a store with one more + // row changed; then they all settle and the store is `base` again + const states = snapshots(m + 1).slice(1); + // a pending transition rendered between the first two commits; reading + // it back moves the flip-back pin off `base`, leaving only the settled pin + const pending = flipBack && snapshots(2)[1]; + if (pending) read(pending); + states.forEach((state, i) => { + read(state); + memo.commit(state.entities, false); + if (pending && i === 0) read(pending); + }); + if (read(base) === settled) hits++; + memo.commit(base.entities, true); + } + return base; + }); + rows.push({ + scenario: name, + 'hit rate': `${((hits / STREAKS) * 100).toFixed(0)}%`, + 'memo retains': `${(bytes / 1024 / 1024).toFixed(2)} MB`, + }); + } +} + // --- argsKey buckets ------------------------------------------------------------ for (const n of [1, 10, 100, 1000]) { diff --git a/examples/benchmark/normalizr.js b/examples/benchmark/normalizr.js index 40c638456933..a469a7fe2ff9 100644 --- a/examples/benchmark/normalizr.js +++ b/examples/benchmark/normalizr.js @@ -26,6 +26,7 @@ import { buildStockData, buildStockScalarUpdate, PortfolioScalar, + BuildTypeDescription, HoldingSchema, buildHoldingData, } from './schemas.js'; @@ -122,6 +123,16 @@ const withMissingStocks = offset => { }; const holdingStores = [withMissingStocks(0), withMissingStocks(2)]; +const BuildTypeSchema = { buildType: BuildTypeDescription }; + +/** A MemoCache that has read `input` with `ents`, then committed `ents`. */ +function committedMemo(schema, input, ents, args) { + const memo = new MemoCache(); + memo.denormalize(schema, input, ents, args); + memo.commit(ents); + return memo; +} + export default function addNormlizrSuite(suite, filter) { const memo = new MemoCache(); // prime the cache @@ -372,6 +383,78 @@ export default function addNormlizrSuite(suite, filter) { [], ); }); + // committed: each MemoCache has committed the `entities` it reads, so reads + // take the path a committed render's re-reads take + const memoCommitted = committedMemo(ProjectSchema, result, entities, []); + add('denormalizeLong withCache committed', () => { + return memoCommitted.denormalize(ProjectSchema, result, entities, []); + }); + const memoValuesCommitted = committedMemo( + ProjectSchemaValues, + valuesState.result, + valuesState.entities, + [], + ); + add('denormalizeLong Values withCache committed', () => { + return memoValuesCommitted.denormalize( + ProjectSchemaValues, + valuesState.result, + valuesState.entities, + [], + ); + }); + const memoStockCommitted = committedMemo( + StockSchema, + stockState.result, + stockState.entities, + stockArgs, + ); + add('denormalizeLong Scalar withCache committed', () => { + return memoStockCommitted.denormalize( + StockSchema, + stockState.result, + stockState.entities, + stockArgs, + ); + }); + const memoAllCommitted = committedMemo( + AllProjects, + queryState.result, + queryState.entities, + [], + ); + add('denormalizeLong All withCache committed', () => { + return memoAllCommitted.denormalize( + AllProjects, + queryState.result, + queryState.entities, + [], + ); + }); + // commit() cost: each op reads 500 small endpoints with a new `entities`, + // then (in the second case) commits it + const buildTypeInputs = Object.keys(entities.BuildTypeDescription) + .slice(0, 500) + .map(id => ({ buildType: id })); + const memoReads = new MemoCache(); + const memoCommits = new MemoCache(); + for (const input of buildTypeInputs) { + memoReads.denormalize(BuildTypeSchema, input, entities, []); + memoCommits.denormalize(BuildTypeSchema, input, entities, []); + } + add('denormalizeShort 500 endpoints withCache', () => { + const next = { ...entities }; + for (let i = 0; i < 500; ++i) { + memoReads.denormalize(BuildTypeSchema, buildTypeInputs[i], next, []); + } + }); + add('denormalizeShort 500 endpoints withCache then commit', () => { + const next = { ...entities }; + for (let i = 0; i < 500; ++i) { + memoCommits.denormalize(BuildTypeSchema, buildTypeInputs[i], next, []); + } + memoCommits.commit(next); + }); add('denormalizeLongAndShort withEntityCacheOnly', () => { memo.endpoints = new WeakDependencyMap(); memo.denormalize(ProjectSchema, result, entities); diff --git a/packages/core/src/controller/Controller.ts b/packages/core/src/controller/Controller.ts index 5afa12a14a33..9f1c963e36cc 100644 --- a/packages/core/src/controller/Controller.ts +++ b/packages/core/src/controller/Controller.ts @@ -49,12 +49,18 @@ import type { ActionTypes, State } from '../types.js'; export type GenericDispatch = (value: any) => Promise; export type DataClientDispatch = (value: ActionTypes) => Promise; +/** Optional: lets the memo keep results from committed stores (see `MemoCache.commit`). */ +interface MemoCommit { + commit?(entities: object, settled?: boolean): void; +} + export interface ControllerConstructorProps< D extends GenericDispatch = DataClientDispatch, > { dispatch?: D; getState?: () => State; - memo?: Pick; + memo?: Pick & + MemoCommit; gcPolicy?: GCInterface; } @@ -98,7 +104,8 @@ export default class Controller< declare readonly memo: Pick< MemoCache, 'denormalize' | 'query' | 'buildQueryKey' - >; + > & + MemoCommit; /** * Handles garbage collection diff --git a/packages/normalizr/AGENTS.md b/packages/normalizr/AGENTS.md index ab7fef03ed76..e44bef600c9e 100644 --- a/packages/normalizr/AGENTS.md +++ b/packages/normalizr/AGENTS.md @@ -19,7 +19,7 @@ Every change to `src/memo/`, or to a schema's `denormalize`, must keep these. Te 2. **Complete dependencies.** Every entity and `argsKey` a computation reads is recorded, including deps replayed from a nested hit, and including entities that are missing, `null` or `INVALID`. 3. **Deterministic.** The same reads lead to the same further reads and an equal result. All args-dependence goes through a pure, referentially stable `argsKey` fn. 4. **Complete journey.** The paths returned with a hit cover every entity the value depends on; subscriptions and GC count refs from them. Callers never mutate it. -5. **Weak, bounded retention.** A value lives only while its first dependency is reachable, with at most `MAX_ENTRIES` values per first dependency and `argsKey` results. +5. **Weak, bounded retention.** A value lives only while its first dependency is reachable, with at most the two most recently used values, plus three pinned ones, per first dependency and `argsKey` results. 6. **Stable identity.** The same reads return the same object while it is cached. 7. **One `WeakMap` level.** Never nest a `WeakMap` per dependency. Hermes resolves `WeakMap` entries one nesting level per stop-the-world marking pass, and each pass rescans every entry in the app, so N nested levels cost N passes (multi-second freezes). `yarn workspace example-benchmark start:ephemerons` measures it. @@ -32,7 +32,9 @@ Accepted limitations (intentional; don't "fix"): - A render that applies a pending store update rebuilds every entity that update touches (`useReducer` has no eager state). Those reads are one-offs. - Within one committed store, a reader re-reads at most one version unchanged: renders read the committed `optimisticState`, managers and callbacks read `controller.getState()`. Every commit starts a new version. Rolling back an optimistic update returns to the older real entities. -- So `WeakDependencyMap` keeps the most recently used entries plus the one the store last flipped back to. Bound: `useSuspense` serves urgent re-renders of an unchanged store from its own `useMemo`, so during a retrying transition a newly committed version can be recomputed once. +- So `WeakDependencyMap` keeps the two most recently used entries, and never evicts three more: the one the store last flipped back to, the one a committed render last read, and the one a committed render of a store with no optimistic updates last read. Each optimistic update rebuilds the entities it touches on every store change, so once the last of any number in flight settles, that last one is the only earlier result the store can return to. +- Commits come from `MemoCache.commit(entities, settled)`, which `DataStore` calls from an effect (effects never run for discarded renders). Reads are buffered per `entities` (one per node) until it commits; reads with the latest committed `entities` are kept right away, so late mounts are covered. Recency alone can't find the committed result: `useSuspense` serves urgent re-renders of an unchanged store from its own `useMemo`, so those reads never reach the memo. +- Pins go on the result's own entry: in the endpoints tier, or in the top-level entity's map when the input is a pk (single-entity endpoints, entity and Collection queries). Only those `get`/`set` calls pass `track`, and `MemoCache` reads and clears the record right after. Nested entity maps never track: they live as long as the entity cache, so a recorded entry nobody clears would never be freed. - `yarn workspace example-benchmark start:memo-tradeoffs` measures hit rates and retained memory for these choices. ## Schema-tree traversal pattern diff --git a/packages/normalizr/src/__tests__/MemoCache.ts b/packages/normalizr/src/__tests__/MemoCache.ts index 4e1c6f7092e1..d7e62fec7390 100644 --- a/packages/normalizr/src/__tests__/MemoCache.ts +++ b/packages/normalizr/src/__tests__/MemoCache.ts @@ -15,6 +15,7 @@ import { IQueryDelegate } from '../interface'; import MemoCache from '../memo/MemoCache'; import { MemoPolicy as POJOPolicy } from '../memo/Policy'; import { MemoPolicy as ImmPolicy } from '../memo/Policy.imm'; +import { normalize } from '../normalize/normalize'; class IDEntity extends Entity { id = ''; @@ -1117,3 +1118,196 @@ describe('MemoCache nested entity dependencies', () => { expect(second.data).toBe(first.data); }); }); + +describe('MemoCache.commit', () => { + class Todo extends Entity { + id = ''; + title = ''; + } + const input = ['1', '2']; + const store = (title: string) => ({ + Todo: { '1': { id: '1', title }, '2': { id: '2', title: 'b' } }, + }); + + it('keeps a committed result through any number of one-off reads', () => { + const memo = new MemoCache(); + const committed = store('committed'); + const first = memo.denormalize([Todo], input, committed).data; + memo.commit(committed); + // a pending transition: each retry builds new entities, read once + for (let i = 0; i < 5; i++) + memo.denormalize([Todo], input, store(`retry ${i}`)); + expect(memo.denormalize([Todo], input, committed).data).toBe(first); + }); + + it('keeps a result first read after its store committed', () => { + const memo = new MemoCache(); + memo.denormalize([Todo], input, store('older')); + const committed = store('committed'); + memo.commit(committed); + // a late mount reads the committed store + const late = memo.denormalize([Todo], input, committed).data; + for (let i = 0; i < 5; i++) + memo.denormalize([Todo], input, store(`retry ${i}`)); + expect(memo.denormalize([Todo], input, committed).data).toBe(late); + }); + + it('keeps a result first read after an unsettled commit', () => { + // optimistic updates were pending when the store committed + const memo = new MemoCache(); + memo.denormalize([Todo], input, store('older')); + const committed = store('committed'); + memo.commit(committed, false); + const late = memo.denormalize([Todo], input, committed).data; + for (let i = 0; i < 5; i++) + memo.denormalize([Todo], input, store(`retry ${i}`)); + expect(memo.denormalize([Todo], input, committed).data).toBe(late); + }); + + it('does not bring back a result evicted before its commit', () => { + const memo = new MemoCache(); + memo.denormalize([Todo], input, store('first')); + const committed = store('committed'); + const first = memo.denormalize([Todo], input, committed).data; + for (let i = 0; i < 5; i++) + memo.denormalize([Todo], input, store(`retry ${i}`)); + memo.commit(committed); + const again = memo.denormalize([Todo], input, committed).data; + expect(again).not.toBe(first); + expect(again).toEqual(first); + }); + + it('does not track reads without an entities table', () => { + const memo = new MemoCache(); + expect(memo.denormalize({ data: Todo }, { data: null }, null).data).toEqual( + { data: null }, + ); + }); + + describe('results whose input is a pk', () => { + // a detail endpoint stores just the pk, and queries build one; each retry + // rewrites a nested child while the top-level entity object stays + class Author extends Entity { + id = ''; + name = ''; + } + class Post extends Entity { + id = ''; + author = Author.fromJS(); + static schema = { author: Author }; + } + const post = { id: '1', author: '2' }; + const withAuthor = (name: string) => ({ + Post: { '1': post }, + Author: { '2': { id: '2', name } }, + }); + + it('keeps a committed single-entity result through one-off reads', () => { + const memo = new MemoCache(); + memo.denormalize(Post, '1', withAuthor('older')); + const committed = withAuthor('committed'); + const first = memo.denormalize(Post, '1', committed).data; + memo.commit(committed); + for (let i = 0; i < 5; i++) + memo.denormalize(Post, '1', withAuthor(`retry ${i}`)); + expect(memo.denormalize(Post, '1', committed).data).toBe(first); + }); + + it('keeps a committed query result through one-off reads', () => { + const memo = new MemoCache(); + const state = (name: string) => ({ + entities: withAuthor(name), + indexes: {}, + }); + memo.query(Post, [{ id: '1' }], state('older')); + const committed = state('committed'); + const first = memo.query(Post, [{ id: '1' }], committed).data; + memo.commit(committed.entities); + for (let i = 0; i < 5; i++) + memo.query(Post, [{ id: '1' }], state(`retry ${i}`)); + expect(memo.query(Post, [{ id: '1' }], committed).data).toBe(first); + }); + + it('keeps a committed Collection result through one-off reads', () => { + const memo = new MemoCache(); + const posts = new schemas.Collection([Post]); + const args = [{ userId: 'u' }]; + const { entities } = normalize( + posts, + [{ id: '1', author: { id: '2', name: 'older' } }], + args, + ); + const state = (name: string) => ({ + entities: { + ...entities, + Author: { '2': { id: '2', name } }, + }, + indexes: {}, + }); + memo.query(posts, args, state('older')); + const committed = state('committed'); + const first = memo.query(posts, args, committed).data; + memo.commit(committed.entities); + for (let i = 0; i < 5; i++) memo.query(posts, args, state(`retry ${i}`)); + expect(memo.query(posts, args, committed).data).toBe(first); + }); + }); + + it('keeps the settled result after committed and flip-back pins move on', () => { + const memo = new MemoCache(); + const s0 = store('settled'); + const r0 = memo.denormalize([Todo], input, s0).data; + memo.commit(s0, true); + // an optimistic update commits: s1 takes the committed pin + const s1 = store('optimistic'); + const r1 = memo.denormalize([Todo], input, s1).data; + memo.commit(s1, false); + // one-off reads, and a flip back to one of them, which takes the kept pin + const x0 = store('x0'); + memo.denormalize([Todo], input, x0); + memo.denormalize([Todo], input, store('x1')); + memo.denormalize([Todo], input, x0); + memo.denormalize([Todo], input, store('x2')); + memo.denormalize([Todo], input, store('x3')); + // only the settled pin holds r0 now + expect(memo.denormalize([Todo], input, s0).data).toBe(r0); + expect(memo.denormalize([Todo], input, s1).data).toBe(r1); + }); + + it('releases old pins once newer commits replace them', () => { + const memo = new MemoCache(); + // the first entry is kept until a flip back, so start with another one + memo.denormalize([Todo], input, store('older')); + const s0 = store('first'); + const r0 = memo.denormalize([Todo], input, s0).data; + memo.commit(s0); + const s1 = store('second'); + memo.denormalize([Todo], input, s1); + memo.commit(s1); + for (let i = 0; i < 4; i++) + memo.denormalize([Todo], input, store(`one-off ${i}`)); + // bounded retention: nothing pins r0 any more + const again = memo.denormalize([Todo], input, s0).data; + expect(again).toEqual(r0); + expect(again).not.toBe(r0); + }); + + it('repeat reads with the same entities do not grow the buffer', () => { + const memo = new MemoCache(); + const pending = store('pending'); + for (let i = 0; i < 50; i++) memo.denormalize([Todo], input, pending); + memo.denormalize([Todo], ['2'], pending); + // one per cached input, however many reads + expect((memo as any).reads.get(pending).size).toBe(2); + }); + + it('without commit, one-off reads push out a result read only once', () => { + const memo = new MemoCache(); + memo.denormalize([Todo], input, store('first')); + const committed = store('committed'); + const first = memo.denormalize([Todo], input, committed).data; + for (let i = 0; i < 5; i++) + memo.denormalize([Todo], input, store(`retry ${i}`)); + expect(memo.denormalize([Todo], input, committed).data).not.toBe(first); + }); +}); diff --git a/packages/normalizr/src/__tests__/WeakDependencyMap.test.ts b/packages/normalizr/src/__tests__/WeakDependencyMap.test.ts index 3d11eb35359e..99418b030cf0 100644 --- a/packages/normalizr/src/__tests__/WeakDependencyMap.test.ts +++ b/packages/normalizr/src/__tests__/WeakDependencyMap.test.ts @@ -111,9 +111,9 @@ describe('WeakDependencyMap', () => { ).toBeUndefined(); }); - it('keeps three entries, never evicting the one the store flipped back to', () => { + it('keeps the two most recent entries, plus the one the store flipped back to', () => { const wem = new WeakDependencyMap(); - const [b2, b3, b4, b5] = [[2], [3], [4], [5]]; + const [b2, b3, b4, b5, b6] = [[2], [3], [4], [5], [6]]; const withB = (entity: object) => MemoPolicy.getEntities({ ...state, B: { '2': entity } }); const setB = (entity: object, value: string) => @@ -121,18 +121,16 @@ describe('WeakDependencyMap', () => { setB(b, 'b'); setB(b2, 'b2'); setB(b3, 'b3'); - // full: the least recently used goes, except the first entry, kept until a flip + // the first entry is pinned until a flip; b2 is the third most recent setB(b4, 'b4'); expect(wem.get(a, withB(b2))[0]).toBeUndefined(); expect(wem.get(a, withB(b))[0]).toBe('b'); - // reading b3 back after newer entries makes it the kept one instead; - // most recently used now: b3, b, b4 + // reading b3 back after newer entries pins it instead of b expect(wem.get(a, withB(b3))[0]).toBe('b3'); setB(b5, 'b5'); expect(wem.get(a, withB(b4))[0]).toBeUndefined(); - setB(b4, 'b4 again'); - // most recently used: b4, b5, b3 (kept), b; b goes + setB(b6, 'b6'); expect(wem.get(a, withB(b))[0]).toBeUndefined(); expect(wem.get(a, withB(b3))[0]).toBe('b3'); }); @@ -178,6 +176,22 @@ describe('WeakDependencyMap', () => { expect(depKey(null)).not.toBe(depKey(INVALID)); }); + it('records hits and writes only when asked to track', () => { + // an untracked call must not hold its entry: the per-entity maps live as + // long as the entity cache, so it would keep every entity they cached + const getEntity = MemoPolicy.getEntities(state); + for (const track of [false, true]) { + const wem = new WeakDependencyMap(); + wem.set([depA, depB], 'value', [], undefined, track); + const reads = new Map(); + wem.trackLast(reads); + expect(reads.size).toBe(track ? 1 : 0); + wem.get(a, getEntity, [], track); + wem.trackLast(reads); + expect(reads.size).toBe(track ? 1 : 0); + } + }); + it('considers empty key list invalid', () => { const wem = new WeakDependencyMap(); expect(() => wem.set([], 'whatever')).toThrowErrorMatchingInlineSnapshot( diff --git a/packages/normalizr/src/memo/MemoCache.ts b/packages/normalizr/src/memo/MemoCache.ts index 9f5c03917027..6d02f02b2c41 100644 --- a/packages/normalizr/src/memo/MemoCache.ts +++ b/packages/normalizr/src/memo/MemoCache.ts @@ -1,5 +1,6 @@ import GlobalCache from './globalCache.js'; import WeakDependencyMap from './WeakDependencyMap.js'; +import type { KeepReads } from './WeakDependencyMap.js'; import buildQueryKey from '../buildQueryKey.js'; import { GetEntityCache, getEntityCaches } from './entitiesCache.js'; import { MemoPolicy } from './Policy.js'; @@ -23,6 +24,12 @@ export default class MemoCache { declare protected _getCache: GetEntityCache; /** Caches the final denormalized form based on input, entities */ protected endpoints: EndpointsCache = new WeakDependencyMap(); + /** Results read per store `entities`, until that store commits. */ + private reads = new WeakMap(); + /** `entities` from the latest `commit`; reads with it are kept right away. */ + private committedEntities: object | undefined = undefined; + /** Whether that store had no optimistic updates. */ + private committedSettled = true; /** Caches the queryKey based on schema, args, and any used entities or indexes */ protected queryKeys: Map> = new Map(); @@ -56,11 +63,51 @@ export default class MemoCache { } const getEntity = this.policy.getEntities(entities); - return getUnvisit( + const tracks = typeof entities === 'object' && entities !== null; + const cache = new GlobalCache( getEntity, - new GlobalCache(getEntity, this._getCache, this.endpoints, args), + this._getCache, + this.endpoints, args, - )(schema, input); + tracks, + ); + const result = getUnvisit(getEntity, cache, args)(schema, input); + // the result's own entry: in the endpoints tier, or the top-level + // entity's map when the input is a pk (single-entity endpoints, queries) + const tracked = cache.tracked; + if (tracked) { + if (entities === this.committedEntities) { + tracked.keepLast(this.committedSettled); + } else { + let reads = this.reads.get(entities); + if (!reads) this.reads.set(entities, (reads = new Map())); + tracked.trackLast(reads); + } + } + return result; + } + + /** + * Marks `entities` as the committed store version. + * + * Every result already read with `entities` is kept through later one-off + * reads (such as each retry of a pending transition, which builds new + * entities), and so is every later read with the same `entities`. + * + * Call it once a render of that store has committed. `DataProvider` calls + * it from an effect, which never runs for a discarded render. Without it, + * results are kept by recency alone. + * + * @param entities The `entities` of the committed state, by reference. + * @param settled Whether that state has no optimistic updates. + */ + commit(entities: object, settled = true): void { + this.committedEntities = entities; + this.committedSettled = settled; + const reads = this.reads.get(entities); + if (!reads) return; + this.reads.delete(entities); + this.endpoints.keep(reads, settled); } /** Compute denormalized form maintaining referential equality for same inputs */ diff --git a/packages/normalizr/src/memo/WeakDependencyMap.ts b/packages/normalizr/src/memo/WeakDependencyMap.ts index 7ed2d32d1034..5502ba0cddbd 100644 --- a/packages/normalizr/src/memo/WeakDependencyMap.ts +++ b/packages/normalizr/src/memo/WeakDependencyMap.ts @@ -58,16 +58,28 @@ export default class WeakDependencyMap< private readonly next = new WeakMap>(); /** Sticky: true once any function-typed (`argsKey`) dep has been stored. */ private hasStr = false; + /** The entry the latest `get` or `set` with `track` hit or wrote, for + * `trackLast()`/`keepLast()`. Only tracked calls record it, and their caller + * reads it right away: an entry recorded and never cleared would stay alive + * for as long as the map does. */ + private lastNode: Node | undefined = undefined; + private lastEntry: Entry | undefined = undefined; get( entity: K, getDependency: GetDependency, args: readonly any[] = [], + /** record the hit for `trackLast()`/`keepLast()` */ + track = false, ) { const root = this.next.get(depKey(entity) as K); const node = root && find(root, getDependency, args); if (!node) return EMPTY; const entry = node.entries[0]; + if (track) { + this.lastNode = node; + this.lastEntry = entry; + } return [entry.value, entry.journey] as readonly [V, Path[]]; } @@ -82,6 +94,8 @@ export default class WeakDependencyMap< * payoff. The array becomes a shared reference held by every subsequent * cache hit; callers MUST NOT mutate it. */ journey?: Path[], + /** record the write for `trackLast()`/`keepLast()` */ + track = false, ) { if (dependencies.length < 1) throw new KeySize(); const root = depKey(dependencies[0].entity) as K; @@ -108,7 +122,7 @@ export default class WeakDependencyMap< // Past MAX_FORKS, start this root over instead. if (node.branches.length && ++top!.forks > MAX_FORKS) { this.next.delete(root); - return this.set(dependencies, value, args, journey); + return this.set(dependencies, value, args, journey, track); } branch = { fn: dep.path as KeyFn, children: new Map() }; node.branches.push(branch); @@ -130,10 +144,14 @@ export default class WeakDependencyMap< // we could recompute this on get, but it would have a cost and we optimize for `get` journey: journey ?? (dependencies.map(d => d.path) as Path[]), }; + if (track) this.lastEntry = entry; if (!node) { - this.next.set(root, newNode(entry)); + node = newNode(entry); + this.next.set(root, node); + if (track) this.lastNode = node; return; } + if (track) this.lastNode = node; // A store can flip back to a version it already showed: React renders the // committed store between retries of a pending transition, which build new // entities each time, and rolling back an optimistic update returns to the @@ -142,9 +160,44 @@ export default class WeakDependencyMap< const { entries } = node; if (!node.kept) node.kept = entry; entries.unshift(entry); - if (entries.length > MAX_ENTRIES) { - const last = entries.length - 1; - entries.splice(entries[last] === node.kept ? last - 1 : last, 1); + // beyond the pinned entries, keep only the most recently used few + let unpinned = 0; + for (let i = 0; i < entries.length; i++) { + const e = entries[i]; + if (e === node.kept || e === node.committed || e === node.settled) + continue; + if (++unpinned > UNPINNED) entries.splice(i--, 1); + } + } + + /** Records the entry the latest `get` hit or `set` wrote into `reads`, to + * pass to `keep()` later. One per node (the last read wins), so repeat + * reads don't grow it. */ + trackLast(reads: KeepReads): void { + const node = this.lastNode; + if (!node) return; + (reads as Map, Entry>).set(node, this.lastEntry!); + this.lastNode = this.lastEntry = undefined; + } + + /** Never evict the entry the latest `get` hit or `set` wrote; see `keep()`. */ + keepLast(settled: boolean): void { + const node = this.lastNode; + if (!node) return; + node.committed = this.lastEntry; + if (settled) node.settled = this.lastEntry; + this.lastNode = this.lastEntry = undefined; + } + + /** Never evict these entries: their results were committed. React serves + * re-renders of a committed store from hook memos without asking the + * cache, so recency alone can't tell a committed result from a one-off + * render of a pending transition. */ + keep(reads: KeepReads, settled: boolean): void { + for (const [node, entry] of reads as Map, Entry>) { + if (!node.entries.includes(entry)) continue; + node.committed = entry; + if (settled) node.settled = entry; } } @@ -232,8 +285,13 @@ export interface Dep { const EMPTY = [undefined, undefined] as const; -/** Entries kept per (first dependency, `argsKey` results). */ -const MAX_ENTRIES = 3; +/** Opaque record of cached entries read, at most one per node; see + * `WeakDependencyMap.trackLast()`. */ +export type KeepReads = Map; + +/** Unpinned entries kept per (first dependency, `argsKey` results), most + * recently used. Up to three more are pinned. */ +const UNPINNED = 2; /** Nodes under one first dependency that may gain a second `argsKey` branch. */ const MAX_FORKS = 8; @@ -244,6 +302,14 @@ interface Node { entries: Entry[]; /** Never evicted: the entry most recently read back after a newer one. */ kept: Entry | undefined; + /** Never evicted: the entry most recently read by a committed render. */ + committed: Entry | undefined; + /** Never evicted: the entry most recently read by a committed render of a + * store with no optimistic updates. Every optimistic update rebuilds the + * entities it touches on each store change, so once the last one settles, + * this is the only earlier result the store can come back to, however + * many were in flight. */ + settled: Entry | undefined; /** One per next `argsKey` dependency; its result picks the child. */ branches: Branch[] | undefined; /** On a first dependency's node: extra branches made anywhere below it. */ @@ -259,6 +325,8 @@ function newNode(entry: Entry | undefined): Node { return { entries: entry ? [entry] : [], kept: entry, + committed: undefined, + settled: undefined, branches: undefined, forks: 0, }; diff --git a/packages/normalizr/src/memo/globalCache.ts b/packages/normalizr/src/memo/globalCache.ts index 1c2358abe79e..abc50bf3fbdd 100644 --- a/packages/normalizr/src/memo/globalCache.ts +++ b/packages/normalizr/src/memo/globalCache.ts @@ -26,17 +26,25 @@ export default class GlobalCache implements Cache { /** Set true once `argsKey` is called for this denormalize frame. Gates * function-stripping fast paths in `paths()` / `getResults()`. */ private _hasArgsKey = false; + /** Whether to record the result's own memo entry, in `tracked`. */ + declare private _track: boolean; + /** True until the top-level `getEntity` of a pk input takes it. */ + private trackTop = false; + /** The map whose last entry is this result's, once `_track` recorded it. */ + tracked: WeakDependencyMap | undefined = undefined; constructor( getEntity: DenormGetEntity, getCache: GetEntityCache, resultCache: EndpointsCache, args: readonly any[] = [], + track = false, ) { this._getEntity = getEntity; this._getCache = getCache; this._resultCache = resultCache; this._args = args; + this._track = track; } /** Records `fn(args)` as a string-keyed dependency for the surrounding @@ -64,6 +72,8 @@ export default class GlobalCache implements Cache { computeValue: (localCacheKey: Map) => void, ): object | undefined | typeof INVALID { const key = schema.key; + const track = this.trackTop; + this.trackTop = false; // cycleCache is deferred to the branch that actually needs it // to avoid unnecessary allocations. const localCacheKey = this.getOrCreateLocalCache(key); @@ -78,7 +88,9 @@ export default class GlobalCache implements Cache { entity, this._getEntity, this._args, + track, ); + if (track) this.tracked = globalCache; // TODO: what if this just returned the deps - then we don't need to store them if (cachePath) { @@ -119,7 +131,7 @@ export default class GlobalCache implements Cache { dependencies: localKey, value: localCacheKey.get(pk), }; - globalCache.set(localKey, cacheValue, this._args); + globalCache.set(localKey, cacheValue, this._args, undefined, track); // start of cycle - reset cycle detection if (this.cycleIndex === trackingIndex) { @@ -167,14 +179,20 @@ export default class GlobalCache implements Cache { paths: EntityPath[]; } { if (!cachable) { - return { data: computeValue(), paths: this.paths() }; + // a pk input: the top-level entity's own entry is the result's + this.trackTop = this._track; + const data = computeValue(); + this.trackTop = false; + return { data, paths: this.paths() }; } let [data, paths] = this._resultCache.get( input, this._getEntity, this._args, + this._track, ); + if (this._track) this.tracked = this._resultCache; if (paths === undefined) { data = computeValue(); @@ -187,7 +205,13 @@ export default class GlobalCache implements Cache { paths = this.paths(); // fill pre-allocated slot 0 with the input reference (chain key only) this.dependencies[0] = { path: { key: '', pk: '' }, entity: input }; - this._resultCache.set(this.dependencies, data, this._args, paths); + this._resultCache.set( + this.dependencies, + data, + this._args, + paths, + this._track, + ); } // hit branch: `paths` aliases the stored Link.journey — return as-is. // Callers must not mutate it (entityExpiresAt and createCountRef both diff --git a/packages/react/src/__tests__/concurrent-memo.web.tsx b/packages/react/src/__tests__/concurrent-memo.web.tsx index a8497f7f21cf..c56f6c2a71d8 100644 --- a/packages/react/src/__tests__/concurrent-memo.web.tsx +++ b/packages/react/src/__tests__/concurrent-memo.web.tsx @@ -30,6 +30,26 @@ class Note extends Entity { text = ''; } +/** A single-entity read whose nested child changes while the parent object stays. */ +class Owner extends Entity { + id = ''; + name = ''; +} +class Project extends Entity { + id = ''; + owner = Owner.fromJS(); + static schema = { owner: Owner }; +} +const getProject = new Endpoint( + ({ id }: { id: string }) => + Promise.resolve({ id, owner: { id: 'o', name: 'v1' } }), + { schema: Project, name: 'getProject' }, +); +const getOwner = new Endpoint( + ({ id }: { id: string }) => Promise.resolve({ id, name: id }), + { schema: Owner, name: 'getOwner' }, +); + const ROWS = 50; const rows = (): TodoItem[] => Array.from({ length: ROWS }, (_, i) => ({ id: `${i}`, title: `${i}` })); @@ -99,6 +119,19 @@ function List() { return
{todos.length}
; } +function ProjectView() { + const project = useSuspense(getProject, { id: 'p' }); + const state = useCacheState(); + const table = (key: string, pk: string) => + idOf((state.entities[key] as Record)[pk]); + reads.push({ + versions: `p${table(Project.key, 'p')}:o${table(Owner.key, 'o')}`, + data: project, + who: 'project', + }); + return
{project.owner.name}
; +} + function Detail({ id }: { id: string }) { const todo = useSuspense(getDetail, { id }); return
{todo.title}
; @@ -109,7 +142,13 @@ let startTransition: (fn: () => void) => void; let showDetail: (id: string) => void; let typeText: (text: string) => void; -function App({ consumers }: { consumers: number }) { +function App({ + consumers, + project = false, +}: { + consumers: number; + project?: boolean; +}) { controller = useController(); const [detail, setDetail] = useState([]); const [text, setText] = useState(''); @@ -125,6 +164,7 @@ function App({ consumers }: { consumers: number }) { {Array.from({ length: consumers }, (_, i) => ( ))} + {project && } loading detail

}> {detail.map(id => ( @@ -177,16 +217,21 @@ function trace(name: string) { ); } -function mount(consumers: number) { +function mount(consumers: number, project = false) { return render( - + , ); } @@ -338,11 +383,9 @@ describeConcurrent.each([1, 2])( await finishTransition('Y'); trace('second transition'); expect(flipBacks(reads)).toBeGreaterThan(0); - // The committed rows changed when the first write committed, and typing - // re-renders are served by useSuspense's useMemo, so the memo never - // sees them re-read before the transition's retries push them out. - // That costs one recompute, after which they are kept. - expect(avoidableMisses(reads)).toBeLessThanOrEqual(1); + // Typing re-renders are served by useSuspense's useMemo and never + // reach the memo, so only the commit signal keeps the new rows. + expect(avoidableMisses(reads)).toBe(0); }); it('reads outside render during optimistic updates, across a resolved mutation', async () => { @@ -393,11 +436,127 @@ describeConcurrent.each([1, 2])( await finishTransition('X'); trace('typing after two committed writes'); expect(flipBacks(reads)).toBeGreaterThan(0); - // The committed rows changed when the first write committed, and typing - // re-renders are served by useSuspense's useMemo, so the memo never - // sees them re-read before the transition's retries push them out. - // That costs one recompute, after which they are kept. - expect(avoidableMisses(reads)).toBeLessThanOrEqual(1); + // Typing re-renders are served by useSuspense's useMemo and never + // reach the memo, so only the commit signal keeps the new rows. + expect(avoidableMisses(reads)).toBe(0); + }); + + it('a single-entity read whose nested child each transition retry rewrites', async () => { + mount(consumers, true); + const writeOwner = (name: string) => { + controller.setResponse(getOwner, { id: 'o' }, { id: 'o', name }); + }; + act(() => writeOwner('v2')); + act(() => writeOwner('v3')); + act(() => { + startTransition(() => { + writeOwner('from transition'); + showDetail('X'); + }); + }); + for (let i = 0; i < 3; i++) act(() => typeText(`a${i}`)); + await finishTransition('X'); + const projectReads = reads.filter(r => r.who === 'project'); + trace('single-entity read'); + // a pk input: commit pins the top-level entity's own entry + expect(avoidableMisses(projectReads)).toBe(0); + expect(avoidableMisses(reads.filter(r => r.who !== 'project'))).toBe(0); + }); + + // Optimistic updates stack: each one rebuilds the entities it touches on + // every store change, so only the store from before all of them can be + // read again. However many are in flight, settling the last one must + // return to that result, and the list must show the right rows. + + const SCRAMBLED = [3, 7, 1, 9, 0, 5, 2, 8, 4, 6]; + + /** `n` optimistic edits to rows 10.., with an unrelated write after each. */ + function beginMany(n: number) { + for (let i = 0; i < n; i++) { + beginOptimistic(`${10 + i}`, `optimistic ${i}`); + writeNote(100 + i); + } + } + + /** Titles the latest render showed. */ + function lastTitles() { + const last = [...reads].reverse().find(r => r.who === 'render'); + return (last?.data as { title: string }[]).map(todo => todo.title); + } + + it('ten optimistic updates in flight, rolled back out of order', async () => { + mount(consumers); + beginMany(10); + for (const i of SCRAMBLED) { + await rollBack(`${10 + i}`, `optimistic ${i}`); + writeNote(200 + i); + } + trace('ten optimistic rolled back'); + expect(lastTitles()).toEqual(rows().map(row => row.title)); + expect(avoidableMisses(reads)).toBe(0); + }); + + it('ten optimistic updates in flight, half resolved and half rolled back', async () => { + mount(consumers); + beginMany(10); + for (const i of SCRAMBLED) { + if (i % 2) await rollBack(`${10 + i}`, `optimistic ${i}`); + else await resolveOptimistic(`${10 + i}`, `optimistic ${i}`); + writeNote(200 + i); + } + trace('ten optimistic resolved or rolled back'); + expect(lastTitles()).toEqual( + rows().map(({ id, title }) => { + const i = Number(id) - 10; + return i >= 0 && i < 10 && i % 2 === 0 ? `optimistic ${i}` : title; + }), + ); + expect(avoidableMisses(reads)).toBe(0); + }); + + it('ten optimistic updates rolled back, after earlier flips moved what the memo keeps', async () => { + mount(consumers); + beginTransition('5', 'X'); + for (let i = 0; i < 3; i++) writeNote(i); + await finishTransition('X'); + readRaw(); + writeNote(3); + readRaw(); + beginMany(10); + for (const i of SCRAMBLED) { + await rollBack(`${10 + i}`, `optimistic ${i}`); + readRaw(); + } + trace('ten optimistic after flips'); + expect(avoidableMisses(reads)).toBe(0); + }); + + it('ten optimistic updates rolled back while a transition is pending', async () => { + mount(consumers); + beginMany(5); + beginTransition('5', 'X'); + for (let i = 5; i < 10; i++) { + beginOptimistic(`${10 + i}`, `optimistic ${i}`); + writeNote(100 + i); + } + for (const i of SCRAMBLED) await rollBack(`${10 + i}`, `optimistic ${i}`); + await finishTransition('X'); + trace('ten optimistic during a transition'); + expect(avoidableMisses(reads)).toBe(0); + }); + + it('fifty optimistic updates in flight still settle to the right rows', async () => { + mount(consumers); + for (let i = 0; i < 50; i++) + beginOptimistic(`${i % ROWS}`, `optimistic ${i}`); + for (let i = 49; i >= 0; i -= 2) + await rollBack(`${i % ROWS}`, `optimistic ${i}`); + for (let i = 0; i < 50; i += 2) + await rollBack(`${i % ROWS}`, `optimistic ${i}`); + writeNote(0); + trace('fifty optimistic'); + expect(lastTitles()).toEqual(rows().map(row => row.title)); + expect(avoidableMisses(reads)).toBe(0); }); }, ); diff --git a/packages/react/src/components/DataStore.tsx b/packages/react/src/components/DataStore.tsx index 90e39ff2ccc4..5a1e1c76697f 100644 --- a/packages/react/src/components/DataStore.tsx +++ b/packages/react/src/components/DataStore.tsx @@ -42,6 +42,15 @@ function DataStore({ // only run once everything is prepared useEffect(mgrEffect, [mgrEffect]); + // Effects only run for committed renders: tell the memo which store + // version committed, so pending-transition renders can't evict its results. + useEffect(() => { + controller.memo.commit?.( + optimisticState.entities, + state.optimistic.length === 0, + ); + }, [controller, optimisticState, state]); + return ( }> diff --git a/packages/react/src/components/__tests__/provider.tsx b/packages/react/src/components/__tests__/provider.tsx index b08a9aabeb32..15b50b197d54 100644 --- a/packages/react/src/components/__tests__/provider.tsx +++ b/packages/react/src/components/__tests__/provider.tsx @@ -5,6 +5,8 @@ import { Middleware, Controller, } from '@data-client/core'; +import { Endpoint, Entity } from '@data-client/endpoint'; +import { MemoCache } from '@data-client/normalizr'; import { act, render } from '@testing-library/react'; import { CoolerArticleResource } from '__tests__/new'; import nock from 'nock'; @@ -149,6 +151,96 @@ describe('', () => { expect((ctrl as MyController).doSomething()).toBe('hi'); }); + describe('memo commit', () => { + class Item extends Entity { + id = ''; + title = ''; + } + let release: (() => void) | undefined; + const updateItem = new Endpoint( + ({ id, title }: { id: string; title: string }) => + new Promise<{ id: string; title: string }>(resolve => { + release = () => resolve({ id, title }); + }), + { + schema: Item, + sideEffect: true, + name: 'updateItem', + getOptimisticResponse(_snap, { id, title }) { + return { id, title }; + }, + }, + ); + + it('commits each committed store, unsettled while optimistic updates are in flight', async () => { + const memo = new MemoCache(); + const commit = jest.spyOn(memo, 'commit'); + class MemoController extends Controller { + constructor(options: ConstructorParameters[0]) { + super({ ...options, memo }); + } + } + let ctrl: Controller | undefined; + let state: any; + function ContextTester() { + ctrl = useController(); + state = useContext(StateContext); + return null; + } + render( + + + , + ); + expect(commit).toHaveBeenLastCalledWith(state.entities, true); + let done: Promise | undefined; + act(() => { + done = ctrl?.fetch(updateItem, { id: '1', title: 'optimistic' }); + }); + // the rendered store holds the optimistic update + expect(state.entities.Item['1'].title).toBe('optimistic'); + expect(commit).toHaveBeenLastCalledWith(state.entities, false); + await act(async () => { + release?.(); + await done; + }); + expect(commit).toHaveBeenLastCalledWith(state.entities, true); + }); + + it('works with a custom memo that has no commit', () => { + const inner = new MemoCache(); + const memo = { + denormalize: inner.denormalize.bind(inner), + query: inner.query.bind(inner), + buildQueryKey: inner.buildQueryKey.bind(inner), + }; + class MemoController extends Controller { + constructor(options: ConstructorParameters[0]) { + super({ ...options, memo }); + } + } + let ctrl: Controller | undefined; + function ContextTester() { + ctrl = useController(); + return null; + } + render( + + + , + ); + expect(() => + act(() => { + ctrl?.setResponse( + CoolerArticleResource.get, + { id: 5 }, + { id: 5, title: 'hi', content: 'more things here' }, + ); + }), + ).not.toThrow(); + }); + }); + it('should change state', () => { jest.useFakeTimers({ now: 50 }); let ctrl: Controller | undefined = undefined;