From 8172159e7033d195b5dd7747ecb54c8b437a0df9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 18 Sep 2020 20:03:44 -0400 Subject: [PATCH] Efficiently track locations of incoming data to be merged. Instead of recursively searching for FieldValueToBeMerged wrapper objects anywhere in the incoming data, processSelectionSet and processFieldValue can build a sparse tree specifying just the paths of fields that need to be merged, and then applyMerges can use that tree to traverse only the parts of the data where merge functions need to be called. These changes effectively revert #5880, since the idea of giving merge functions a chance to transform their child data before calling nested merge functions no longer makes as much sense. Instead, applyMerges will be recursively called on the child data before parent merge functions run, the way it used to be (before #5880). --- src/cache/inmemory/entityStore.ts | 15 ++- src/cache/inmemory/helpers.ts | 57 +--------- src/cache/inmemory/policies.ts | 102 +++--------------- src/cache/inmemory/types.ts | 14 ++- src/cache/inmemory/writeToStore.ts | 167 ++++++++++++++++++++++++----- 5 files changed, 181 insertions(+), 174 deletions(-) diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts index c2c135f08da..9bdc401c419 100644 --- a/src/cache/inmemory/entityStore.ts +++ b/src/cache/inmemory/entityStore.ts @@ -258,7 +258,7 @@ export abstract class EntityStore implements NormalizedCache { public abstract getStorage( idOrObj: string | StoreObject, - storeFieldName: string, + storeFieldName: string | number, ): StorageType; // Maps root entity IDs to the number of times they have been retained, minus @@ -353,7 +353,7 @@ export abstract class EntityStore implements NormalizedCache { // Bound function that can be passed around to provide easy access to fields // of Reference objects as well as ordinary objects. public getFieldValue = ( - objectOrReference: StoreObject | Reference, + objectOrReference: StoreObject | Reference | undefined, storeFieldName: string, ) => maybeDeepFreeze( isReference(objectOrReference) @@ -484,9 +484,14 @@ export namespace EntityStore { public readonly storageTrie = new KeyTrie(canUseWeakMap); public getStorage( idOrObj: string | StoreObject, - storeFieldName: string, + storeFieldName: string | number, ): StorageType { - return this.storageTrie.lookup(idOrObj, storeFieldName); + return this.storageTrie.lookup( + idOrObj, + // Normalize numbers to strings, so we don't accidentally end up + // with multiple storage objects for the same field. + String(storeFieldName), + ); } } } @@ -555,7 +560,7 @@ class Layer extends EntityStore { public getStorage( idOrObj: string | StoreObject, - storeFieldName: string, + storeFieldName: string | number, ): StorageType { return this.parent.getStorage(idOrObj, storeFieldName); } diff --git a/src/cache/inmemory/helpers.ts b/src/cache/inmemory/helpers.ts index 47d9ee28e5c..dc12bc65838 100644 --- a/src/cache/inmemory/helpers.ts +++ b/src/cache/inmemory/helpers.ts @@ -1,4 +1,4 @@ -import { FieldNode, SelectionSetNode } from 'graphql'; +import { SelectionSetNode } from 'graphql'; import { NormalizedCache } from './types'; import { @@ -8,7 +8,6 @@ import { StoreObject, isField, DeepMerger, - ReconcilerFunction, resultKeyNameFromField, shouldInclude, } from '../../utilities'; @@ -57,17 +56,6 @@ export function selectionSetMatchesResult( return false; } -// Invoking merge functions needs to happen after processSelectionSet has -// finished, but requires information that is more readily available -// during processSelectionSet, so processSelectionSet embeds special -// objects of the following shape within its result tree, which then must -// be removed by calling Policies#applyMerges. -export interface FieldValueToBeMerged { - __field: FieldNode; - __typename: string; - __value: StoreValue; -} - export function storeValueIsStoreObject( value: StoreValue, ): value is StoreObject { @@ -77,47 +65,6 @@ export function storeValueIsStoreObject( !Array.isArray(value); } -export function isFieldValueToBeMerged( - value: any, -): value is FieldValueToBeMerged { - const field = value && value.__field; - return field && isField(field); -} - export function makeProcessedFieldsMerger() { - // A DeepMerger that merges arrays and objects structurally, but otherwise - // prefers incoming scalar values over existing values. Provides special - // treatment for FieldValueToBeMerged objects. Used to accumulate fields - // when processing a single selection set. - return new DeepMerger(reconcileProcessedFields); -} - -const reconcileProcessedFields: ReconcilerFunction<[]> = function ( - existingObject, - incomingObject, - property, -) { - const existing = existingObject[property]; - const incoming = incomingObject[property]; - - if (isFieldValueToBeMerged(existing)) { - existing.__value = this.merge( - existing.__value, - isFieldValueToBeMerged(incoming) - // TODO Check compatibility of __field and __typename properties? - ? incoming.__value - : incoming, - ); - return existing; - } - - if (isFieldValueToBeMerged(incoming)) { - incoming.__value = this.merge( - existing, - incoming.__value, - ); - return incoming; - } - - return this.merge(existing, incoming); + return new DeepMerger; } diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index 8266e9971ae..0c8f359da0d 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -23,12 +23,10 @@ import { canUseWeakMap, compact, } from '../../utilities'; -import { IdGetter, ReadMergeModifyContext } from "./types"; +import { IdGetter, ReadMergeModifyContext, MergeInfo } from "./types"; import { hasOwn, fieldNameFromStoreName, - FieldValueToBeMerged, - isFieldValueToBeMerged, storeValueIsStoreObject, selectionSetMatchesResult, TypeOrFieldNameRegExp, @@ -715,21 +713,17 @@ export class Policies { return !!(policy && policy.merge); } - public applyMerges( - existing: T | Reference, - incoming: T | FieldValueToBeMerged, + public runMergeFunction( + existing: StoreValue, + incoming: StoreValue, + { field, typename }: MergeInfo, context: ReadMergeModifyContext, - storageKeys?: [string | StoreObject, string], - ): T { - if (isFieldValueToBeMerged(incoming)) { - const field = incoming.__field; - const fieldName = field.name.value; - // This policy and its merge function are guaranteed to exist - // because the incoming value is a FieldValueToBeMerged object. - const { merge } = this.getFieldPolicy( - incoming.__typename, fieldName, false)!; - - incoming = merge!(existing, incoming.__value, makeFieldFunctionOptions( + storage?: StorageType, + ) { + const fieldName = field.name.value; + const { merge } = this.getFieldPolicy(typename, fieldName, false)!; + if (merge) { + return merge(existing, incoming, makeFieldFunctionOptions( this, // Unlike options.readField for read functions, we do not fall // back to the current object if no foreignObjOrRef is provided, @@ -743,69 +737,13 @@ export class Policies { // However, readField(name, ref) is useful for merge functions // that need to deduplicate child objects and references. void 0, - { typename: incoming.__typename, + { typename, fieldName, field, variables: context.variables }, context, - storageKeys - ? context.store.getStorage(...storageKeys) - : Object.create(null), - )) as T; - } - - if (Array.isArray(incoming)) { - return incoming!.map(item => this.applyMerges( - // Items in the same position in different arrays are not - // necessarily related to each other, so there is no basis for - // merging them. Passing void here means any FieldValueToBeMerged - // objects within item will be handled as if there was no existing - // data. Also, we do not pass storageKeys because the array itself - // is never an entity with a __typename, so its indices can never - // have custom read or merge functions. - void 0, - item, - context, - )) as T; - } - - if (storeValueIsStoreObject(incoming)) { - const e = existing as StoreObject | Reference; - const i = incoming as StoreObject; - - // If the existing object is a { __ref } object, e.__ref provides a - // stable key for looking up the storage object associated with - // e.__ref and storeFieldName. Otherwise, storage is enabled only if - // existing is actually a non-null object. It's less common for a - // merge function to use options.storage, but it's conceivable that a - // pair of read and merge functions might want to cooperate in - // managing their shared options.storage object. - const firstStorageKey = isReference(e) - ? e.__ref - : typeof e === "object" && e; - - let newFields: StoreObject | undefined; - - Object.keys(i).forEach(storeFieldName => { - const incomingValue = i[storeFieldName]; - const appliedValue = this.applyMerges( - context.store.getFieldValue(e, storeFieldName), - incomingValue, - context, - // Avoid enabling options.storage when firstStorageKey is falsy, - // which implies no options.storage object has ever been created - // for a read/merge function for this field. - firstStorageKey ? [firstStorageKey, storeFieldName] : void 0, - ); - if (appliedValue !== incomingValue) { - newFields = newFields || Object.create(null); - newFields![storeFieldName] = appliedValue; - } - }); - - if (newFields) { - return { ...i, ...newFields } as typeof incoming; - } + storage || Object.create(null), + )); } return incoming; @@ -872,21 +810,15 @@ function makeFieldFunctionOptions( const iType = getFieldValue(incoming, "__typename"); const typesDiffer = eType && iType && eType !== iType; - const applied = policies.applyMerges( - typesDiffer ? void 0 : existing, - incoming, - context, - ); - if ( typesDiffer || !storeValueIsStoreObject(existing) || - !storeValueIsStoreObject(applied) + !storeValueIsStoreObject(incoming) ) { - return applied; + return incoming; } - return { ...existing, ...applied }; + return { ...existing, ...incoming }; } return incoming; diff --git a/src/cache/inmemory/types.ts b/src/cache/inmemory/types.ts index bed9b7adb60..3004ea8cc3a 100644 --- a/src/cache/inmemory/types.ts +++ b/src/cache/inmemory/types.ts @@ -1,4 +1,4 @@ -import { DocumentNode } from 'graphql'; +import { DocumentNode, FieldNode } from 'graphql'; import { Transaction } from '../core/cache'; import { @@ -65,7 +65,7 @@ export interface NormalizedCache { getStorage( idOrObj: string | StoreObject, - storeFieldName: string, + storeFieldName: string | number, ): StorageType; } @@ -101,6 +101,16 @@ export type ApolloReducerConfig = { addTypename?: boolean; }; +export interface MergeInfo { + field: FieldNode; + typename: string | undefined; +}; + +export interface MergeTree { + info?: MergeInfo; + map: Map; +}; + export interface ReadMergeModifyContext { store: NormalizedCache; variables?: Record; diff --git a/src/cache/inmemory/writeToStore.ts b/src/cache/inmemory/writeToStore.ts index 93c181620cd..bba6c0969fb 100644 --- a/src/cache/inmemory/writeToStore.ts +++ b/src/cache/inmemory/writeToStore.ts @@ -22,8 +22,8 @@ import { cloneDeep, } from '../../utilities'; -import { NormalizedCache, ReadMergeModifyContext } from './types'; -import { makeProcessedFieldsMerger, FieldValueToBeMerged, fieldNameFromStoreName } from './helpers'; +import { NormalizedCache, ReadMergeModifyContext, MergeTree } from './types'; +import { makeProcessedFieldsMerger, fieldNameFromStoreName, storeValueIsStoreObject } from './helpers'; import { StoreReader } from './readFromStore'; import { InMemoryCache } from './inMemoryCache'; @@ -41,9 +41,7 @@ interface ProcessSelectionSetOptions { result: Record; selectionSet: SelectionSetNode; context: WriteContext; - out?: { - shouldApplyMerges: boolean; - }; + mergeTree: MergeTree; } export interface WriteToStoreOptions { @@ -93,6 +91,7 @@ export class StoreWriter { result: result || Object.create(null), dataId, selectionSet: operationDefinition.selectionSet, + mergeTree: { map: new Map }, context: { store, written: Object.create(null), @@ -123,9 +122,7 @@ export class StoreWriter { context, // This object allows processSelectionSet to report useful information // to its callers without explicitly returning that information. - out = { - shouldApplyMerges: false, - }, + mergeTree, }: ProcessSelectionSetOptions): StoreObject | Reference { const { policies } = this.cache; @@ -203,23 +200,20 @@ export class StoreWriter { variables: context.variables, }); + const childTree = getChildMergeTree(mergeTree, storeFieldName); + let incomingValue = - this.processFieldValue(value, selection, context, out); + this.processFieldValue(value, selection, context, childTree); if (policies.hasMergeFunction(typename, selection.name.value)) { - // If a custom merge function is defined for this field, store - // a special FieldValueToBeMerged object, so that we can run - // the merge function later, after all processSelectionSet - // work is finished. - incomingValue = { - __field: selection, - __typename: typename, - __value: incomingValue, - } as FieldValueToBeMerged; - - // Communicate to the caller that newFields contains at - // least one FieldValueToBeMerged. - out.shouldApplyMerges = true; + childTree.info = { + // TODO Check compatibility against any existing + // childTree.field? + field: selection, + typename, + }; + } else { + maybeRecycleChildMergeTree(mergeTree, storeFieldName); } incomingFields = context.merge(incomingFields, { @@ -273,8 +267,8 @@ export class StoreWriter { if ("string" === typeof dataId) { const entityRef = makeReference(dataId); - if (out.shouldApplyMerges) { - incomingFields = policies.applyMerges(entityRef, incomingFields, context); + if (mergeTree.map.size) { + incomingFields = this.applyMerges(mergeTree, entityRef, incomingFields, context); } if (process.env.NODE_ENV !== "production") { @@ -305,7 +299,7 @@ export class StoreWriter { value: any, field: FieldNode, context: WriteContext, - out: ProcessSelectionSetOptions["out"], + mergeTree: MergeTree, ): StoreValue { if (!field.selectionSet || value === null) { // In development, we need to clone scalar values so that they can be @@ -315,16 +309,135 @@ export class StoreWriter { } if (Array.isArray(value)) { - return value.map(item => this.processFieldValue(item, field, context, out)); + return value.map((item, i) => { + const value = this.processFieldValue( + item, field, context, getChildMergeTree(mergeTree, i)); + maybeRecycleChildMergeTree(mergeTree, i); + return value; + }); } return this.processSelectionSet({ result: value, selectionSet: field.selectionSet, context, - out, + mergeTree, }); } + + private applyMerges( + mergeTree: MergeTree, + existing: StoreValue, + incoming: T, + context: ReadMergeModifyContext, + storageKeys?: [string | StoreObject, string | number], + ): T { + if (mergeTree.map.size && !isReference(incoming)) { + const e: StoreObject | Reference | undefined = ( + // Items in the same position in different arrays are not + // necessarily related to each other, so when incoming is an array + // we process its elements as if there was no existing data. + !Array.isArray(incoming) && + // Likewise, existing must be either a Reference or a StoreObject + // in order for its fields to be safe to merge with the fields of + // the incoming object. + (isReference(existing) || storeValueIsStoreObject(existing)) + ) ? existing : void 0; + + // This narrowing is implied by mergeTree.map.size > 0 and + // !isReference(incoming), though TypeScript understandably cannot + // hope to infer this type. + const i = incoming as StoreObject | StoreValue[]; + + // The options.storage objects provided to read and merge functions + // are derived from the identity of the parent object and the + // storeFieldName string of each field to be merged. Since the + // parent identity remains the same for each iteration of the loop + // below, we can precompute it here. + const firstStorageKey: string | StoreObject | undefined = + isReference(e) ? e.__ref : e; + + // It's possible that applying merge functions to this subtree will + // not change the incoming data, so this variable tracks the fields + // that did change, so we can create a new incoming object when (and + // only when) at least one incoming field has changed. We use a Map + // to preserve the type of numeric keys. + let changedFields: Map | undefined; + + const getValue = ( + from: typeof e | typeof i, + name: string | number, + ): StoreValue => { + return Array.isArray(from) + ? (typeof name === "number" ? from[name] : void 0) + : context.store.getFieldValue(from, String(name)) + }; + + mergeTree.map.forEach((childTree, storeFieldName) => { + const eVal = getValue(e, storeFieldName); + const iVal = getValue(i, storeFieldName); + const aVal = this.applyMerges( + childTree, + eVal, + iVal, + context, + firstStorageKey ? [ + firstStorageKey, + storeFieldName, + ] : void 0, + ); + if (aVal !== iVal) { + changedFields = changedFields || new Map; + changedFields.set(storeFieldName, aVal); + } + }); + + if (changedFields) { + // Shallow clone i so we can add changed fields to it. + incoming = (Array.isArray(i) ? i.slice(0) : { ...i }) as T; + changedFields.forEach((value, name) => { + (incoming as any)[name] = value; + }); + } + } + + if (mergeTree.info) { + return this.cache.policies.runMergeFunction( + existing, + incoming, + mergeTree.info, + context, + storageKeys && context.store.getStorage(...storageKeys), + ); + } + + return incoming; + } +} + +const emptyMergeTreePool: MergeTree[] = []; + +function getChildMergeTree( + { map }: MergeTree, + name: string | number, +): MergeTree { + if (!map.has(name)) { + map.set(name, emptyMergeTreePool.pop() || { map: new Map }); + } + return map.get(name)!; +} + +function maybeRecycleChildMergeTree( + { map }: MergeTree, + name: string | number, +) { + const childTree = map.get(name); + if (childTree && + !childTree.info && + !childTree.map.size) { + emptyMergeTreePool.push(childTree); + map.delete(name); + } } const warnings = new Set();