Skip to content

Commit

Permalink
Merge pull request #5648 from apollographql/reduce-result-caching-key…
Browse files Browse the repository at this point in the history 
…-diversity

Improve optimistic result caching by limiting cache key diversity.
  • Loading branch information
@benjamn
benjamn authored Dec 3, 2019
2 parents d809c8f + 0c26be9 commit 8416f62
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 84 deletions.
2 changes: 1 addition & 1 deletion src/cache/inmemory/__tests__/optimistic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ describe('optimistic cache layers', () => {
const spinelessAfterRemovingBuzz = readSpinelessFragment();
expect(spinelessBeforeRemovingBuzz).toEqual(spinelessAfterRemovingBuzz);
expect(spinelessBeforeRemovingBuzz).not.toBe(spinelessAfterRemovingBuzz);
expect(spinelessBeforeRemovingBuzz.author).not.toBe(
expect(spinelessBeforeRemovingBuzz.author).toBe(
spinelessAfterRemovingBuzz.author,
);

Expand Down
144 changes: 83 additions & 61 deletions src/cache/inmemory/entityStore.ts
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { dep, OptimisticDependencyFunction } from 'optimism';
import { dep, OptimisticDependencyFunction, KeyTrie } from 'optimism';
import { invariant } from 'ts-invariant';
import { isReference, StoreValue } from '../../utilities/graphql/storeUtils';
import {
DeepMerger,
ReconcilerFunction,
} from '../../utilities/common/mergeDeep';
import { isEqual } from '../../utilities/common/isEqual';
import { canUseWeakMap } from '../../utilities/common/canUse';
import { NormalizedCache, NormalizedCacheObject, StoreObject } from './types';
import {
getTypenameFromStoreObject,
Expand All @@ -14,41 +15,10 @@ import {

const hasOwn = Object.prototype.hasOwnProperty;

type DependType = OptimisticDependencyFunction<string> | null;

function makeDepKey(dataId: string, storeFieldName?: string) {
const parts = [dataId];
if (typeof storeFieldName === "string") {
parts.push(fieldNameFromStoreName(storeFieldName));
}
return JSON.stringify(parts);
}

function depend(store: EntityStore, dataId: string, storeFieldName?: string) {
if (store.depend) {
store.depend(makeDepKey(dataId, storeFieldName));
}
}

function dirty(store: EntityStore, dataId: string, storeFieldName?: string) {
if (store.depend) {
store.depend.dirty(
typeof storeFieldName === "string"
? makeDepKey(dataId, storeFieldName)
: makeDepKey(dataId),
);
}
}

export abstract class EntityStore implements NormalizedCache {
protected data: NormalizedCacheObject = Object.create(null);

// It seems like this property ought to be protected rather than public,
// but TypeScript doesn't realize it's inherited from a shared base
// class by both Root and Layer classes, so Layer methods are forbidden
// from accessing the .depend property of an arbitrary EntityStore
// instance, because it might be a Root instance (and vice-versa).
public readonly depend: DependType = null;
public readonly group: CacheGroup;

public abstract addLayer(
layerId: string,
Expand All @@ -70,12 +40,12 @@ export abstract class EntityStore implements NormalizedCache {
}

public get(dataId: string): StoreObject {
depend(this, dataId);
this.group.depend(dataId);
return this.data[dataId];
}

public getFieldValue(dataId: string, storeFieldName: string): StoreValue {
depend(this, dataId, storeFieldName);
this.group.depend(dataId, storeFieldName);
const storeObject = this.data[dataId];
return storeObject && storeObject[storeFieldName];
}
Expand All @@ -87,15 +57,15 @@ export abstract class EntityStore implements NormalizedCache {
if (merged !== existing) {
this.data[dataId] = merged;
delete this.refs[dataId];
if (this.depend) {
if (this.group.caching) {
// First, invalidate any dependents that called get rather than
// getFieldValue.
dirty(this, dataId);
this.group.dirty(dataId);
// Now invalidate dependents who called getFieldValue for any
// fields that are changing as a result of this merge.
Object.keys(incoming).forEach(storeFieldName => {
if (!existing || incoming[storeFieldName] !== existing[storeFieldName]) {
dirty(this, dataId, storeFieldName);
this.group.dirty(dataId, storeFieldName);
}
});
}
Expand Down Expand Up @@ -172,10 +142,10 @@ export abstract class EntityStore implements NormalizedCache {
});
}

if (this.depend) {
dirty(this, dataId);
if (this.group.caching) {
this.group.dirty(dataId);
Object.keys(fieldsToDirty).forEach(fieldName => {
dirty(this, dataId, fieldName);
this.group.dirty(dataId, fieldName);
});
}
}
Expand Down Expand Up @@ -278,17 +248,71 @@ export abstract class EntityStore implements NormalizedCache {
}
return this.refs[dataId];
}

// Used to compute cache keys specific to this.group.
public makeCacheKey(...args: any[]) {
return this.group.keyMaker.lookupArray(args);
}
}

// A single CacheGroup represents a set of one or more EntityStore objects,
// typically the Root store in a CacheGroup by itself, and all active Layer
// stores in a group together. A single EntityStore object belongs to only
// one CacheGroup, store.group. The CacheGroup is responsible for tracking
// dependencies, so store.group is helpful for generating unique keys for
// cached results that need to be invalidated when/if those dependencies
// change. If we used the EntityStore objects themselves as cache keys (that
// is, store rather than store.group), the cache would become unnecessarily
// fragmented by all the different Layer objects. Instead, the CacheGroup
// approach allows all optimistic Layer objects in the same linked list to
// belong to one CacheGroup, with the non-optimistic Root object belonging
// to another CacheGroup, allowing resultCaching dependencies to be tracked
// separately for optimistic and non-optimistic entity data.
class CacheGroup {
private d: OptimisticDependencyFunction<string> | null = null;

constructor(public readonly caching: boolean) {
this.d = caching ? dep<string>() : null;
}

public depend(dataId: string, storeFieldName?: string) {
if (this.d) {
this.d(makeDepKey(dataId, storeFieldName));
}
}

public dirty(dataId: string, storeFieldName?: string) {
if (this.d) {
this.d.dirty(
typeof storeFieldName === "string"
? makeDepKey(dataId, storeFieldName)
: makeDepKey(dataId),
);
}
}

// Used by the EntityStore#makeCacheKey method to compute cache keys
// specific to this CacheGroup.
public readonly keyMaker = new KeyTrie<object>(canUseWeakMap);
}

function makeDepKey(dataId: string, storeFieldName?: string) {
const parts = [dataId];
if (typeof storeFieldName === "string") {
parts.push(fieldNameFromStoreName(storeFieldName));
}
return JSON.stringify(parts);
}

export namespace EntityStore {
// Refer to this class as EntityStore.Root outside this namespace.
export class Root extends EntityStore {
// Although each Root instance gets its own unique this.depend
// function, any Layer instances created by calling addLayer need to
// share a single distinct dependency function. Since this shared
// function must outlast the Layer instances themselves, it needs to
// be created and owned by the Root instance.
private sharedLayerDepend: DependType = null;
// Although each Root instance gets its own unique CacheGroup object,
// any Layer instances created by calling addLayer need to share a
// single distinct CacheGroup object. Since this shared object must
// outlast the Layer instances themselves, it needs to be created and
// owned by the Root instance.
private sharedLayerGroup: CacheGroup = null;

constructor({
resultCaching = true,
Expand All @@ -298,11 +322,8 @@ export namespace EntityStore {
seed?: NormalizedCacheObject;
}) {
super();
if (resultCaching) {
// Regard this.depend as publicly readonly but privately mutable.
(this as any).depend = dep<string>();
this.sharedLayerDepend = dep<string>();
}
(this.group as any) = new CacheGroup(resultCaching);
this.sharedLayerGroup = new CacheGroup(resultCaching);
if (seed) this.replace(seed);
}

Expand All @@ -311,7 +332,7 @@ export namespace EntityStore {
replay: (layer: EntityStore) => any,
): EntityStore {
// The replay function will be called in the Layer constructor.
return new Layer(layerId, this, replay, this.sharedLayerDepend);
return new Layer(layerId, this, replay, this.sharedLayerGroup);
}

public removeLayer(layerId: string): Root {
Expand All @@ -328,7 +349,7 @@ class Layer extends EntityStore {
public readonly id: string,
public readonly parent: Layer | EntityStore.Root,
public readonly replay: (layer: EntityStore) => any,
public readonly depend: DependType,
public readonly group: CacheGroup,
) {
super();
replay(this);
Expand All @@ -338,7 +359,7 @@ class Layer extends EntityStore {
layerId: string,
replay: (layer: EntityStore) => any,
): EntityStore {
return new Layer(layerId, this, replay, this.depend);
return new Layer(layerId, this, replay, this.group);
}

public removeLayer(layerId: string): EntityStore {
Expand All @@ -348,7 +369,7 @@ class Layer extends EntityStore {
if (layerId === this.id) {
// Dirty every ID we're removing.
// TODO Some of these IDs could escape dirtying if value unchanged.
if (this.depend) {
if (this.group.caching) {
Object.keys(this.data).forEach(dataId => this.delete(dataId));
}
return parent;
Expand Down Expand Up @@ -379,8 +400,9 @@ class Layer extends EntityStore {
// from calling this.depend for every optimistic layer we examine, but
// ensures we call this.depend in the last optimistic layer before we
// reach the root layer.
if (this.depend && this.depend !== this.parent.depend) {
depend(this, dataId);

if (this.group.caching && this.group !== this.parent.group) {
this.group.depend(dataId);
}

return this.parent.get(dataId);
Expand All @@ -394,8 +416,8 @@ class Layer extends EntityStore {
}
}

if (this.depend && this.depend !== this.parent.depend) {
depend(this, dataId, storeFieldName);
if (this.group.caching && this.group !== this.parent.group) {
this.group.depend(dataId, storeFieldName);
}

return this.parent.getFieldValue(dataId, storeFieldName);
Expand Down Expand Up @@ -473,7 +495,7 @@ const storeObjectReconciler: ReconcilerFunction<[EntityStore]> = function (

export function supportsResultCaching(store: any): store is EntityStore {
// When result caching is disabled, store.depend will be null.
return !!(store instanceof EntityStore && store.depend);
return !!(store instanceof EntityStore && store.group.caching);
}

export function defaultNormalizedCacheFactory(
Expand Down
7 changes: 1 addition & 6 deletions src/cache/inmemory/inMemoryCache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ import './fixPolyfills';

import { DocumentNode } from 'graphql';
import { wrap } from 'optimism';
import { KeyTrie } from 'optimism';

import { ApolloCache, Transaction } from '../core/cache';
import { Cache } from '../core/types/Cache';
import { addTypenameToDocument } from '../../utilities/graphql/transform';
import { canUseWeakMap } from '../../utilities/common/canUse';
import {
ApolloReducerConfig,
NormalizedCacheObject,
Expand Down Expand Up @@ -49,7 +47,6 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
private typenameDocumentCache = new Map<DocumentNode, DocumentNode>();
private storeReader: StoreReader;
private storeWriter: StoreWriter;
private cacheKeyRoot = new KeyTrie<object>(canUseWeakMap);

// Set this while in a transaction to prevent broadcasts...
// don't forget to turn it back on!
Expand Down Expand Up @@ -86,7 +83,6 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {

this.storeReader = new StoreReader({
addTypename: this.addTypename,
cacheKeyRoot: this.cacheKeyRoot,
policies: this.policies,
});

Expand All @@ -101,9 +97,8 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
const store = c.optimistic ? cache.optimisticData : cache.data;
if (supportsResultCaching(store)) {
const { optimistic, rootId, variables } = c;
return cache.cacheKeyRoot.lookup(
return store.makeCacheKey(
c.query,
store,
JSON.stringify({ optimistic, rootId, variables }),
);
}
Expand Down
23 changes: 7 additions & 16 deletions src/cache/inmemory/readFromStore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
InlineFragmentNode,
SelectionSetNode,
} from 'graphql';
import { wrap, KeyTrie } from 'optimism';
import { wrap } from 'optimism';
import { invariant } from 'ts-invariant';

import {
Expand All @@ -17,7 +17,6 @@ import {
makeReference,
StoreValue,
} from '../../utilities/graphql/storeUtils';
import { canUseWeakMap } from '../../utilities/common/canUse';
import { createFragmentMap, FragmentMap } from '../../utilities/graphql/fragments';
import { shouldInclude } from '../../utilities/graphql/directives';
import {
Expand Down Expand Up @@ -75,20 +74,12 @@ type ExecSubSelectedArrayOptions = {

export interface StoreReaderConfig {
addTypename?: boolean;
cacheKeyRoot?: KeyTrie<object>;
policies: Policies;
}

export class StoreReader {
constructor(private config: StoreReaderConfig) {
const cacheKeyRoot =
config && config.cacheKeyRoot || new KeyTrie<object>(canUseWeakMap);

this.config = {
addTypename: true,
cacheKeyRoot,
...config,
};
this.config = { addTypename: true, ...config };

const {
executeSelectionSet,
Expand All @@ -104,11 +95,12 @@ export class StoreReader {
context,
}: ExecSelectionSetOptions) {
if (supportsResultCaching(context.store)) {
return cacheKeyRoot.lookup(
context.store,
return context.store.makeCacheKey(
selectionSet,
JSON.stringify(context.variables),
isReference(objectOrReference) ? objectOrReference.__ref : objectOrReference,
isReference(objectOrReference)
? objectOrReference.__ref
: objectOrReference,
);
}
}
Expand All @@ -119,8 +111,7 @@ export class StoreReader {
}, {
makeCacheKey({ field, array, context }) {
if (supportsResultCaching(context.store)) {
return cacheKeyRoot.lookup(
context.store,
return context.store.makeCacheKey(
field,
array,
JSON.stringify(context.variables),
Expand Down

0 comments on commit 8416f62

Please sign in to comment.