core(computed-artifact): use deep equality over strict

lighthouse-core/gather/computed/computed-artifact.js

@@ -5,13 +5,16 @@
  */
 'use strict';
 
+const ArbitraryEqualityMap = require('../../lib/arbitrary-equality-map');
+
 class ComputedArtifact {
   /**
    * @param {!ComputedArtifacts} allComputedArtifacts
    */
   constructor(allComputedArtifacts) {
     /** @private {!Map} */
-    this._cache = new Map();
+    this._cache = new ArbitraryEqualityMap();
+    this._cache.setEqualityFn(ArbitraryEqualityMap.deepEquals);
 
     /** @private {!ComputedArtifacts} */
     this._allComputedArtifacts = allComputedArtifacts;
@@ -35,74 +38,6 @@ class ComputedArtifact {
     throw new Error('compute_() not implemented for computed artifact ' + this.name);
   }
 
-  /**
-   * This is a helper function for performing cache operations and is responsible for maintaing the
-   * internal cache structure. This function accepts a path of artifacts, used to find the correct
-   * nested cache object, and an operation to perform on that cache with the final key.
-   *
-   * The cache is structured with the first argument occupying the keys of the toplevel cache that point
-   * to the Map of keys for the second argument and so forth until the 2nd to last argument's Map points
-   * to result values rather than further nesting. In the simplest case of a single argument, there
-   * is no nesting and the keys point directly to result values.
-   *
-   *  Map(
-   *    argument1A -> Map(
-   *      argument2A -> result1A2A
-   *      argument2B -> result1A2B
-   *    )
-   *    argument1B -> Map(
-   *      argument2A -> result1B2A
-   *    )
-   *  )
-   *
-   * @param {!Array<*>} artifacts
-   * @param {function(!Map, *)} cacheOperation
-   */
-  _performCacheOperation(artifacts, cacheOperation) {
-    artifacts = artifacts.slice();
-
-    let cache = this._cache;
-    while (artifacts.length > 1) {
-      const nextKey = artifacts.shift();
-      if (cache.has(nextKey)) {
-        cache = cache.get(nextKey);
-      } else {
-        const nextCache = new Map();
-        cache.set(nextKey, nextCache);
-        cache = nextCache;
-      }
-    }
-
-    return cacheOperation(cache, artifacts.shift());
-  }
-
-  /**
-   * Performs a cache.has operation, see _performCacheOperation for more.
-   * @param {!Array<*>} artifacts
-   * @return {boolean}
-   */
-  _cacheHas(artifacts) {
-    return this._performCacheOperation(artifacts, (cache, key) => cache.has(key));
-  }
-
-  /**
-   * Performs a cache.get operation, see _performCacheOperation for more.
-   * @param {!Array<*>} artifacts
-   * @return {*}
-   */
-  _cacheGet(artifacts) {
-    return this._performCacheOperation(artifacts, (cache, key) => cache.get(key));
-  }
-
-  /**
-   * Performs a cache.set operation, see _performCacheOperation for more.
-   * @param {!Array<*>} artifacts
-   * @param {*} result
-   */
-  _cacheSet(artifacts, result) {
-    return this._performCacheOperation(artifacts, (cache, key) => cache.set(key, result));
-  }
-
   /**
    * Asserts that the length of the array is the same as the number of inputs the class expects
    * @param {!Array<*>} artifacts
@@ -125,13 +60,13 @@ class ComputedArtifact {
    */
   request(...artifacts) {
     this._assertCorrectNumberOfArtifacts(artifacts);
-    if (this._cacheHas(artifacts)) {
-      return Promise.resolve(this._cacheGet(artifacts));
+    if (this._cache.has(artifacts)) {
+      return Promise.resolve(this._cache.get(artifacts));
     }
 
     const artifactPromise = Promise.resolve()
       .then(_ => this.compute_(...artifacts, this._allComputedArtifacts));
-    this._cacheSet(artifacts, artifactPromise);
+    this._cache.set(artifacts, artifactPromise);
 
     return artifactPromise;
   }

lighthouse-core/lib/arbitrary-equality-map.js

@@ -0,0 +1,62 @@
+/**
+ * @license Copyright 2018 Google Inc. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+'use strict';
+
+const isEqual = require('lodash.isequal');
+
+/**
+ * @fileoverview This class is designed to allow maps with arbitrary equality functions.
+ * It is not meant to be performant and is well-suited to use cases where the number of entries is
+ * likely to be small (like computed artifacts).
+ */
+module.exports = class ArbitraryEqualityMap {
+  constructor() {
+    this._equalsFn = (a, b) => a === b;
+    this._entries = [];
+  }
+
+  /**
+   * @param {function():boolean} equalsFn
+   */
+  setEqualityFn(equalsFn) {
+    this._equalsFn = equalsFn;
+  }
+
+  has(key) {
+    return this._findIndexOf(key) !== -1;
+  }
+
+  get(key) {
+    const entry = this._entries[this._findIndexOf(key)];
+    return entry && entry.value;
+  }
+
+  set(key, value) {
+    let index = this._findIndexOf(key);
+    if (index === -1) index = this._entries.length;
+    this._entries[index] = {key, value};
+  }
+
+  _findIndexOf(key) {
+    for (let i = 0; i < this._entries.length; i++) {
+      if (this._equalsFn(key, this._entries[i].key)) return i;
+    }
+
+    return -1;
+  }
+
+  /**
+   * Determines whether two objects are deeply equal. Defers to lodash isEqual, but is kept here for
+   * easy usage by consumers.
+   * See https://lodash.com/docs/4.17.5#isEqual.
+   * @param {*} objA
+   * @param {*} objB
+   * @return {boolean}
+   */
+  static deepEquals(objA, objB) {
+    return isEqual(objA, objB);
+  }
+};

lighthouse-core/test/gather/computed/computed-artifact-test.js

@@ -47,20 +47,18 @@ describe('ComputedArtifact base class', () => {
       .then(_ => assert.throws(() => multiInputArtifact.request(1)));
   });
 
-  it('caches computed artifacts', () => {
-    const testComputedArtifact = new TestComputedArtifact();
+  it('caches computed artifacts by strict equality', () => {
+    const computedArtifact = new TestComputedArtifact();
 
-    const obj0 = {};
-    const obj1 = {};
-
-    return testComputedArtifact.request(obj0).then(result => {
+    return computedArtifact.request({x: 1}).then(result => {
       assert.equal(result, 0);
-    }).then(_ => testComputedArtifact.request(obj1)).then(result => {
+    }).then(_ => computedArtifact.request({x: 2})).then(result => {
       assert.equal(result, 1);
-    }).then(_ => testComputedArtifact.request(obj0)).then(result => {
+    }).then(_ => computedArtifact.request({x: 1})).then(result => {
       assert.equal(result, 0);
-    }).then(_ => testComputedArtifact.request(obj1)).then(result => {
+    }).then(_ => computedArtifact.request({x: 2})).then(result => {
       assert.equal(result, 1);
+      assert.equal(computedArtifact.computeCounter, 2);
     });
   });
 
@@ -80,6 +78,7 @@ describe('ComputedArtifact base class', () => {
       .then(_ => assert.deepEqual(computedArtifact.lastArguments, [obj1, obj2, mockComputed]))
       .then(_ => computedArtifact.request(obj0, obj1))
       .then(result => assert.equal(result, 0))
-      .then(_ => assert.deepEqual(computedArtifact.lastArguments, [obj1, obj2, mockComputed]));
+      .then(_ => assert.deepEqual(computedArtifact.lastArguments, [obj1, obj2, mockComputed]))
+      .then(_ => assert.equal(computedArtifact.computeCounter, 2));
   });
 });

lighthouse-core/test/lib/arbitrary-equality-map-test.js

@@ -0,0 +1,63 @@
+/**
+ * @license Copyright 2018 Google Inc. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+'use strict';
+
+/* eslint-env mocha */
+
+const assert = require('assert');
+const ArbitraryEqualityMap = require('../../lib/arbitrary-equality-map.js');
+const trace = require('../fixtures/traces/progressive-app-m60.json');
+
+describe('ArbitraryEqualityMap', () => {
+  it('creates a map', () => {
+    const map = new ArbitraryEqualityMap();
+    assert.equal(map.has(1), false);
+    assert.equal(map.get(1), undefined);
+    map.set(1, 2);
+    assert.equal(map.has(1), true);
+    assert.equal(map.get(1), 2);
+  });
+
+  it('uses custom equality function', () => {
+    // create a map which stores 1 value per type
+    const map = new ArbitraryEqualityMap();
+    map.setEqualityFn((a, b) => typeof a === typeof b);
+    map.set(true, 1);
+    map.set('foo', 2);
+    map.set({}, 3);
+    map.set('bar', 4);
+
+    assert.equal(map.has(1), false);
+    assert.equal(map.has(false), true);
+    assert.equal(map.has(''), true);
+    assert.equal(map.has({x: 1}), true);
+    assert.equal(map.get('foo'), 4);
+  });
+
+  it('is not hella slow', () => {
+    const map = new ArbitraryEqualityMap();
+    map.setEqualityFn(ArbitraryEqualityMap.deepEquals);
+    for (let i = 0; i < 100; i++) {
+      map.set({i}, i);
+    }
+
+    for (let j = 0; j < 1000; j++) {
+      const i = j % 100;
+      assert.equal(map.get({i}), i);
+    }
+  }).timeout(1000);
+
+  it('is fast for expected usage', () => {
+    const map = new ArbitraryEqualityMap();
+    map.setEqualityFn(ArbitraryEqualityMap.deepEquals);
+    map.set([trace, {x: 0}], 'foo');
+    map.set([trace, {x: 1}], 'bar');
+
+    for (let i = 0; i < 10000; i++) {
+      assert.equal(map.get([trace, {x: i % 2}]), i % 2 ? 'bar' : 'foo');
+    }
+  }).timeout(1000);
+});

package.json

@@ -90,6 +90,7 @@
     "jpeg-js": "0.1.2",
     "js-library-detector": "^4.3.1",
     "lighthouse-logger": "^1.0.0",
+    "lodash.isequal": "^4.5.0",
     "metaviewport-parser": "0.2.0",
     "mkdirp": "0.5.1",
     "opn": "4.0.2",

yarn.lock

@@ -2669,6 +2669,10 @@ lodash.isempty@^4.2.1:
   version "4.4.0"
   resolved "https://registry.yarnpkg.com/lodash.isempty/-/lodash.isempty-4.4.0.tgz#6f86cbedd8be4ec987be9aaf33c9684db1b31e7e"
 
+lodash.isequal@^4.5.0:
+  version "4.5.0"
+  resolved "https://registry.yarnpkg.com/lodash.isequal/-/lodash.isequal-4.5.0.tgz#415c4478f2bcc30120c22ce10ed3226f7d3e18e0"
+
 lodash.isplainobject@^4.0.4:
   version "4.0.6"
   resolved "https://registry.yarnpkg.com/lodash.isplainobject/-/lodash.isplainobject-4.0.6.tgz#7c526a52d89b45c45cc690b88163be0497f550cb"