Efficiently canonicalize InMemoryCache result objects.

#4141 (comment)

src/__tests__/client.ts

@@ -3037,11 +3037,9 @@ describe('@connection', () => {
     client.cache.evict({ fieldName: "a" });
     await wait();
 
-    // The results are structurally the same, but the result objects have
-    // been recomputed for queries that involved the ROOT_QUERY.a field.
-    expect(checkLastResult(aResults, a456)).not.toBe(a456);
+    expect(checkLastResult(aResults, a456)).toBe(a456);
     expect(checkLastResult(bResults, bOyez)).toBe(bOyez);
-    expect(checkLastResult(abResults, a456bOyez)).not.toBe(a456bOyez);
+    expect(checkLastResult(abResults, a456bOyez)).toBe(a456bOyez);
 
     const cQuery = gql`{ c }`;
     // Passing cache-only as the fetchPolicy allows the { c: "see" }
@@ -3090,25 +3088,19 @@ describe('@connection', () => {
       { a: 123 },
       { a: 234 },
       { a: 456 },
-      // Delivered again because we explicitly called resetLastResults.
-      { a: 456 },
     ]);
 
     expect(bResults).toEqual([
       { b: "asdf" },
       { b: "ASDF" },
       { b: "oyez" },
-      // Delivered again because we explicitly called resetLastResults.
-      { b: "oyez" },
     ]);
 
     expect(abResults).toEqual([
       { a: 123, b: "asdf" },
       { a: 234, b: "asdf" },
       { a: 234, b: "ASDF" },
       { a: 456, b: "oyez" },
-      // Delivered again because we explicitly called resetLastResults.
-      { a: 456, b: "oyez" },
     ]);
 
     expect(cResults).toEqual([

src/cache/inmemory/__tests__/cache.ts

@@ -1728,8 +1728,8 @@ describe("InMemoryCache#modify", () => {
     })).toBe(false); // Nothing actually modified.
 
     const resultAfterAuthorInvalidation = read();
-    expect(resultAfterAuthorInvalidation).not.toBe(initialResult);
     expect(resultAfterAuthorInvalidation).toEqual(initialResult);
+    expect(resultAfterAuthorInvalidation).toBe(initialResult);
 
     expect(cache.modify({
       id: cache.identify({
@@ -1743,8 +1743,8 @@ describe("InMemoryCache#modify", () => {
     })).toBe(false); // Nothing actually modified.
 
     const resultAfterBookInvalidation = read();
-    expect(resultAfterBookInvalidation).not.toBe(resultAfterAuthorInvalidation);
     expect(resultAfterBookInvalidation).toEqual(resultAfterAuthorInvalidation);
+    expect(resultAfterBookInvalidation).toBe(resultAfterAuthorInvalidation);
     expect(resultAfterBookInvalidation.currentlyReading.author).toEqual({
       __typename: "Author",
       name: "Maria Dahvana Headley",
@@ -2591,9 +2591,8 @@ describe("ReactiveVar and makeVar", () => {
     });
 
     const result2 = cache.readQuery({ query });
-    // Without resultCaching, equivalent results will not be ===.
-    expect(result2).not.toBe(result1);
     expect(result2).toEqual(result1);
+    expect(result2).toBe(result1);
 
     expect(nameVar()).toBe("Ben");
     expect(nameVar("Hugh")).toBe("Hugh");

src/cache/inmemory/__tests__/optimistic.ts

@@ -431,7 +431,7 @@ describe('optimistic cache layers', () => {
 
     const resultAfterRemovingBuzzLayer = readWithAuthors();
     expect(resultAfterRemovingBuzzLayer).toEqual(resultWithBuzz);
-    expect(resultAfterRemovingBuzzLayer).not.toBe(resultWithBuzz);
+    expect(resultAfterRemovingBuzzLayer).toBe(resultWithBuzz);
     resultWithTwoAuthors.books.forEach((book, i) => {
       expect(book).toEqual(resultAfterRemovingBuzzLayer.books[i]);
       expect(book).toBe(resultAfterRemovingBuzzLayer.books[i]);

src/cache/inmemory/__tests__/policies.ts

@@ -4692,19 +4692,8 @@ describe("type policies", function () {
     });
 
     const thirdFirstBookResult = readFirstBookResult();
-
-    // A change in VW's books field triggers rereading of result objects
-    // that previously involved her books field.
-    expect(thirdFirstBookResult).not.toBe(secondFirstBookResult);
-
-    // However, since the new Book was not the earliest published, the
-    // second and third results are structurally the same.
     expect(thirdFirstBookResult).toEqual(secondFirstBookResult);
-
-    // In fact, the original author.firstBook object has been reused!
-    expect(thirdFirstBookResult.author.firstBook).toBe(
-      secondFirstBookResult.author.firstBook,
-    );
+    expect(thirdFirstBookResult).toBe(secondFirstBookResult);
   });
 
   it("readField can read fields with arguments", function () {

src/cache/inmemory/canon.ts

@@ -0,0 +1,115 @@
+import { KeyTrie } from "optimism";
+import { canUseWeakMap } from "../../utilities";
+import { objToStr } from "./helpers";
+
+// When we say an object is "canonical" in programming, we mean it has been
+// admitted into some abstract "canon" of official/blessed objects. This
+// Canon class is a representation of such a collection, with the property
+// that canon.admit(value1) === canon.admit(value2) if value1 and value2 are
+// deeply equal to each other. The canonicalization process involves looking
+// at every property in the provided object tree, so it takes the same order
+// of time as deep equality checking (linear time), but already-admitted
+// objects are returned immediately from canon.admit, so ensuring subtrees
+// have already been canonized tends to speed up canonicalization. Of
+// course, since canonized objects may be shared widely between unrelated
+// consumers, it's important to regard them as immutable. No detection of
+// cycles is needed by the StoreReader class right now, so we don't bother
+// keeping track of objects we've already seen during the recursion of the
+// admit method. Objects whose internal class name is neither Array nor
+// Object can be included in the value tree, but they will not be replaced
+// with a canonical version (to put it another way, they are assumed to be
+// canonical already). We can easily add additional cases to the switch
+// statement to handle other common object types, such as "[object Date]"
+// objects, as needed.
+export class Canon {
+  // All known objects this Canon has admitted.
+  private known = new (canUseWeakMap ? WeakSet : Set)<object>();
+
+  // Efficient storage/lookup structure for admitting objects.
+  private pool = new KeyTrie<{
+    array?: any[];
+    object?: Record<string, any>;
+    keys?: SortedKeysInfo;
+  }>(canUseWeakMap);
+
+  // Returns the canonical version of value.
+  public admit<T>(value: T): T;
+  public admit(value: any) {
+    if (value && typeof value === "object") {
+      switch (objToStr.call(value)) {
+        case "[object Array]": {
+          if (this.known.has(value)) return value;
+          const array: any[] = value.map(this.admit, this);
+          // Arrays are looked up in the KeyTrie using their recursively
+          // canonicalized elements, and the known version of the array is
+          // preserved as node.array.
+          const node = this.pool.lookupArray(array);
+          if (!node.array) {
+            this.known.add(node.array = array);
+            if (process.env.NODE_ENV !== "production") {
+              Object.freeze(array);
+            }
+          }
+          return node.array;
+        }
+
+        case "[object Object]": {
+          if (this.known.has(value)) return value;
+          const proto = Object.getPrototypeOf(value);
+          const array = [proto];
+          const keys = this.sortedKeys(value);
+          array.push(keys.json);
+          keys.sorted.forEach(key => {
+            array.push(this.admit(value[key]));
+          });
+          // Objects are looked up in the KeyTrie by their prototype
+          // (which is *not* recursively canonicalized), followed by a
+          // JSON representation of their (sorted) keys, followed by the
+          // sequence of recursively canonicalized values corresponding to
+          // those keys. To keep the final results unambiguous with other
+          // sequences (such as arrays that just happen to contain [proto,
+          // keys.json, value1, value2, ...]), the known version of the
+          // object is stored as node.object.
+          const node = this.pool.lookupArray(array);
+          if (!node.object) {
+            const obj = node.object = Object.create(proto);
+            this.known.add(obj);
+            keys.sorted.forEach((key, i) => {
+              obj[key] = array[i + 2];
+            });
+            if (process.env.NODE_ENV !== "production") {
+              Object.freeze(obj);
+            }
+          }
+          return node.object;
+        }
+      }
+    }
+    return value;
+  }
+
+  // It's worthwhile to cache the sorting of arrays of strings, since the
+  // same initial unsorted arrays tend to be encountered many times.
+  // Fortunately, we can reuse the KeyTrie machinery to look up the sorted
+  // arrays in linear time (which is faster than sorting large arrays).
+  private sortedKeys(obj: object) {
+    const keys = Object.keys(obj);
+    const node = this.pool.lookupArray(keys);
+    if (!node.keys) {
+      keys.sort();
+      const json = JSON.stringify(keys);
+      if (!(node.keys = this.keysByJSON.get(json))) {
+        this.keysByJSON.set(json, node.keys = { sorted: keys, json });
+      }
+    }
+    return node.keys;
+  }
+  // Arrays that contain the same elements in a different order can share
+  // the same SortedKeysInfo object, to save memory.
+  private keysByJSON = new Map<string, SortedKeysInfo>();
+}
+
+type SortedKeysInfo = {
+  sorted: string[];
+  json: string;
+};

src/cache/inmemory/helpers.ts

@@ -12,7 +12,10 @@ import {
   shouldInclude,
 } from '../../utilities';
 
-export const hasOwn = Object.prototype.hasOwnProperty;
+export const {
+  hasOwnProperty: hasOwn,
+  toString: objToStr,
+} = Object.prototype;
 
 export function getTypenameFromStoreObject(
   store: NormalizedCache,

src/cache/inmemory/readFromStore.ts

@@ -36,6 +36,7 @@ import { getTypenameFromStoreObject } from './helpers';
 import { Policies } from './policies';
 import { InMemoryCache } from './inMemoryCache';
 import { MissingFieldError } from '../core/types/common';
+import { Canon } from './canon';
 
 export type VariableMap = { [name: string]: any };
 
@@ -324,11 +325,7 @@ export class StoreReader {
 
     // Perform a single merge at the end so that we can avoid making more
     // defensive shallow copies than necessary.
-    finalResult.result = mergeDeepArray(objectsToMerge);
-
-    if (process.env.NODE_ENV !== 'production') {
-      Object.freeze(finalResult.result);
-    }
+    finalResult.result = this.canon.admit(mergeDeepArray(objectsToMerge));
 
     // Store this result with its selection set so that we can quickly
     // recognize it again in the StoreReader#isFresh method.
@@ -337,6 +334,8 @@ export class StoreReader {
     return finalResult;
   }
 
+  private canon = new Canon;
+
   private knownResults = new WeakMap<Record<string, any>, SelectionSetNode>();
 
   // Cached version of execSubSelectedArrayImpl.
@@ -377,7 +376,7 @@ export class StoreReader {
       array = array.filter(context.store.canRead);
     }
 
-    array = array.map((item, i) => {
+    array = this.canon.admit(array.map((item, i) => {
       // null value in array
       if (item === null) {
         return null;
@@ -410,11 +409,7 @@ export class StoreReader {
       invariant(context.path.pop() === i);
 
       return item;
-    });
-
-    if (process.env.NODE_ENV !== 'production') {
-      Object.freeze(array);
-    }
+    }));
 
     return { result: array, missing };
   }

src/core/__tests__/QueryManager/index.ts

@@ -902,10 +902,7 @@ describe('QueryManager', () => {
               break;
             case 2:
               expect(stripSymbols(result.data)).toEqual(data3);
-              expect(result.data).not.toBe(firstResultData);
-              expect(result.data.b).toEqual(firstResultData.b);
-              expect(result.data.d).not.toBe(firstResultData.d);
-              expect(result.data.d.f).toEqual(firstResultData.d.f);
+              expect(result.data).toBe(firstResultData);
               resolve();
               break;
             default: