diff --git a/.changeset/pink-games-grin.md b/.changeset/pink-games-grin.md new file mode 100644 index 000000000..8c0bf912c --- /dev/null +++ b/.changeset/pink-games-grin.md @@ -0,0 +1,5 @@ +--- +'providence-analytics': patch +--- + +improve memoization diff --git a/packages-node/providence-analytics/src/program/analyzers/helpers/track-down-identifier.js b/packages-node/providence-analytics/src/program/analyzers/helpers/track-down-identifier.js index 59a0dc0d2..01edbc6bb 100644 --- a/packages-node/providence-analytics/src/program/analyzers/helpers/track-down-identifier.js +++ b/packages-node/providence-analytics/src/program/analyzers/helpers/track-down-identifier.js @@ -7,7 +7,7 @@ const { const { resolveImportPath } = require('../../utils/resolve-import-path.js'); const { AstService } = require('../../services/AstService.js'); const { LogService } = require('../../services/LogService.js'); -const { memoizeAsync } = require('../../utils/memoize.js'); +const { memoize } = require('../../utils/memoize.js'); /** * @typedef {import('../../types/core').RootFile} RootFile @@ -233,7 +233,7 @@ async function trackDownIdentifierFn(source, identifierName, currentFilePath, ro return /** @type { RootFile } */ { file: rootFilePath, specifier: rootSpecifier }; } -trackDownIdentifier = memoizeAsync(trackDownIdentifierFn); +trackDownIdentifier = memoize(trackDownIdentifierFn); /** * @param {BabelPath} astPath @@ -265,7 +265,7 @@ async function trackDownIdentifierFromScopeFn( return rootFile; } -const trackDownIdentifierFromScope = memoizeAsync(trackDownIdentifierFromScopeFn); +const trackDownIdentifierFromScope = memoize(trackDownIdentifierFromScopeFn); module.exports = { trackDownIdentifier, diff --git a/packages-node/providence-analytics/src/program/utils/memoize.js b/packages-node/providence-analytics/src/program/utils/memoize.js index 400deda77..dd32cc158 100644 --- a/packages-node/providence-analytics/src/program/utils/memoize.js +++ b/packages-node/providence-analytics/src/program/utils/memoize.js @@ -1,54 +1,45 @@ const { InputDataService } = require('../services/InputDataService.js'); +function isObject(arg) { + return !Array.isArray(arg) && typeof arg === 'object'; +} + +function createCachableArg(arg) { + if (isObject(arg)) { + try { + return JSON.stringify(arg); + } catch { + return arg; + } + } + return arg; +} + /** - * @param {function} func - * @param {object} [storage] + * @param {function} functionToMemoize + * @param {{ storage:object; serializeObjects: boolean }} [opts] */ -function memoize(func, storage = {}) { +function memoize(functionToMemoize, { storage = {}, serializeObjects = false } = {}) { // eslint-disable-next-line func-names return function () { // eslint-disable-next-line prefer-rest-params const args = [...arguments]; + const cachableArgs = !serializeObjects ? args : args.map(createCachableArg); // Allow disabling of cache for testing purposes // @ts-ignore - if (!InputDataService.cacheDisabled && args in storage) { + if (!InputDataService.cacheDisabled && cachableArgs in storage) { // @ts-ignore - return storage[args]; + return storage[cachableArgs]; } // @ts-ignore - const outcome = func.apply(this, args); + const outcome = functionToMemoize.apply(this, args); // @ts-ignore // eslint-disable-next-line no-param-reassign - storage[args] = outcome; - return outcome; - }; -} - -/** - * @param {function} func - * @param {object} [storage] - */ -function memoizeAsync(func, storage = {}) { - // eslint-disable-next-line func-names - return async function () { - // eslint-disable-next-line prefer-rest-params - const args = [...arguments]; - // Allow disabling of cache for testing purposes - // @ts-ignore - if (!InputDataService.cacheDisabled && args in storage) { - // @ts-ignore - return storage[args]; - } - // @ts-ignore - const outcome = await func.apply(this, args); - // @ts-ignore - // eslint-disable-next-line no-param-reassign - storage[args] = outcome; + storage[cachableArgs] = outcome; return outcome; }; } module.exports = { memoize, - memoizeAsync, }; diff --git a/packages-node/providence-analytics/src/program/utils/resolve-import-path.js b/packages-node/providence-analytics/src/program/utils/resolve-import-path.js index d9422a11b..8e9d2d82f 100644 --- a/packages-node/providence-analytics/src/program/utils/resolve-import-path.js +++ b/packages-node/providence-analytics/src/program/utils/resolve-import-path.js @@ -12,7 +12,7 @@ const pathLib = require('path'); const { nodeResolve } = require('@rollup/plugin-node-resolve'); const { LogService } = require('../services/LogService.js'); -const { memoizeAsync } = require('./memoize.js'); +const { memoize } = require('./memoize.js'); const { toPosixPath } = require('./to-posix-path.js'); const fakePluginContext = { @@ -64,6 +64,6 @@ async function resolveImportPath(importee, importer, opts = {}) { * @param {{customResolveOptions?: {preserveSymlinks:boolean}}} [opts] nodeResolve options * @returns {Promise} the resolved file system path, like '/my/project/node_modules/@lion/core/index.js' */ -const resolveImportPathMemoized = memoizeAsync(resolveImportPath); +const resolveImportPathMemoized = memoize(resolveImportPath); module.exports = { resolveImportPath: resolveImportPathMemoized }; diff --git a/packages-node/providence-analytics/test-node/program/utils/memoize.test.js b/packages-node/providence-analytics/test-node/program/utils/memoize.test.js new file mode 100644 index 000000000..e0f4ff5ba --- /dev/null +++ b/packages-node/providence-analytics/test-node/program/utils/memoize.test.js @@ -0,0 +1,320 @@ +const { expect } = require('chai'); +const { memoize } = require('../../../src/program/utils/memoize.js'); +const { InputDataService } = require('../../../src/program/services/InputDataService.js'); + +const cacheDisabledInitialValue = InputDataService.cacheDisabled; + +describe('Memoize', () => { + beforeEach(() => { + // This is important, since memoization only works + InputDataService.cacheDisabled = false; + }); + afterEach(() => { + InputDataService.cacheDisabled = cacheDisabledInitialValue; + }); + + describe('With primitives', () => { + describe('Numbers', () => { + it(`returns cached result when called with same parameters`, async () => { + let sumCalled = 0; + function sum(/** @type {number} a */ a, /** @type {number} a */ b) { + sumCalled += 1; + return a + b; + } + const sumMemoized = memoize(sum); + + // Put in cache for args combination + expect(sumMemoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + + // Return from cache + expect(sumMemoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + + // Put in cache for args combination + expect(sumMemoized(1, 3)).to.equal(4); + expect(sumCalled).to.equal(2); + + // Return from cache + expect(sumMemoized(1, 3)).to.equal(4); + expect(sumCalled).to.equal(2); + }); + + it(`returns cached result per function for same args`, async () => { + let sumCalled = 0; + function sum(/** @type {number} a */ a, /** @type {number} a */ b) { + sumCalled += 1; + return a + b; + } + const sumMemoized = memoize(sum); + let sum2Called = 0; + function sum2(/** @type {number} a */ a, /** @type {number} a */ b) { + sum2Called += 1; + return a + b; + } + const sum2Memoized = memoize(sum2); + + expect(sumMemoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(0); + + expect(sum2Memoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + + // Both cached + expect(sumMemoized(1, 2)).to.equal(3); + expect(sum2Memoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + }); + }); + describe('Strings', () => { + it(`returns cached result when called with same parameters`, async () => { + let sumCalled = 0; + function sum(/** @type {string} a */ a, /** @type {string} a */ b) { + sumCalled += 1; + return a + b; + } + const sumMemoized = memoize(sum); + + // Put in cache for args combination + expect(sumMemoized('1', '2')).to.equal('12'); + expect(sumCalled).to.equal(1); + + // Return from cache + expect(sumMemoized('1', '2')).to.equal('12'); + expect(sumCalled).to.equal(1); + + // Put in cache for args combination + expect(sumMemoized('1', '3')).to.equal('13'); + expect(sumCalled).to.equal(2); + + // Return from cache + expect(sumMemoized('1', '3')).to.equal('13'); + expect(sumCalled).to.equal(2); + }); + + it(`returns cached result per function for same args`, async () => { + let sumCalled = 0; + function sum(/** @type {string} a */ a, /** @type {string} a */ b) { + sumCalled += 1; + return a + b; + } + const sumMemoized = memoize(sum); + let sum2Called = 0; + function sum2(/** @type {string} a */ a, /** @type {string} a */ b) { + sum2Called += 1; + return a + b; + } + const sum2Memoized = memoize(sum2); + + expect(sumMemoized('1', '2')).to.equal('12'); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(0); + + expect(sum2Memoized('1', '2')).to.equal('12'); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + + // Both cached + expect(sumMemoized('1', '2')).to.equal('12'); + expect(sum2Memoized('1', '2')).to.equal('12'); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + }); + }); + }); + describe('With non primitives', () => { + describe('Arrays', () => { + it(`returns cached result when called with same parameters`, async () => { + let sumCalled = 0; + function sum(/** @type {number[]} a */ a, /** @type {number[]} a */ b) { + sumCalled += 1; + return [...a, ...b]; + } + const sumMemoized = memoize(sum); + + // Put in cache for args combination + expect(sumMemoized([1], [2])).to.eql([1, 2]); + expect(sumCalled).to.equal(1); + + // Return from cache + expect(sumMemoized([1], [2])).to.eql([1, 2]); + expect(sumCalled).to.equal(1); + + // Put in cache for args combination + expect(sumMemoized([1], [3])).to.eql([1, 3]); + expect(sumCalled).to.equal(2); + }); + + it(`returns cached result per function for same args`, async () => { + let sumCalled = 0; + function sum(/** @type {number[]} a */ a, /** @type {number[]} a */ b) { + sumCalled += 1; + return [...a, ...b]; + } + const sumMemoized = memoize(sum); + let sum2Called = 0; + function sum2(/** @type {number[]} a */ a, /** @type {number[]} a */ b) { + sum2Called += 1; + return [...a, ...b]; + } + const sum2Memoized = memoize(sum2); + + expect(sumMemoized([1], [2])).to.eql([1, 2]); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(0); + + expect(sum2Memoized([1], [2])).to.eql([1, 2]); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + + // Both cached + expect(sumMemoized([1], [2])).to.eql([1, 2]); + expect(sum2Memoized([1], [2])).to.eql([1, 2]); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + }); + }); + + describe('Objects', () => { + it(`returns cached result when called with same parameters`, async () => { + let sumCalled = 0; + function sum(/** @type {object} a */ a, /** @type {object} a */ b) { + sumCalled += 1; + return { ...a, ...b }; + } + const sumMemoized = memoize(sum, { serializeObjects: true }); + + // Put in cache for args combination + expect(sumMemoized({ x: 1 }, { y: 2 })).to.eql({ x: 1, y: 2 }); + expect(sumCalled).to.equal(1); + + // Return from cache + expect(sumMemoized({ x: 1 }, { y: 2 })).to.eql({ x: 1, y: 2 }); + expect(sumCalled).to.equal(1); + + // Put in cache for args combination + expect(sumMemoized({ x: 1 }, { y: 3 })).to.eql({ x: 1, y: 3 }); + expect(sumCalled).to.equal(2); + }); + + it(`returns cached result per function for same args`, async () => { + let sumCalled = 0; + function sum(/** @type {object} a */ a, /** @type {object} a */ b) { + sumCalled += 1; + return { ...a, ...b }; + } + const sumMemoized = memoize(sum, { serializeObjects: true }); + let sum2Called = 0; + function sum2(/** @type {object} a */ a, /** @type {object} a */ b) { + sum2Called += 1; + return { ...a, ...b }; + } + const sum2Memoized = memoize(sum2, { serializeObjects: true }); + + expect(sumMemoized({ x: 1 }, { y: 2 })).to.eql({ x: 1, y: 2 }); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(0); + + expect(sum2Memoized({ x: 1 }, { y: 2 })).to.eql({ x: 1, y: 2 }); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + + // Both cached + expect(sumMemoized({ x: 1 }, { y: 2 })).to.eql({ x: 1, y: 2 }); + expect(sum2Memoized({ x: 1 }, { y: 2 })).to.eql({ x: 1, y: 2 }); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + }); + }); + + describe('When non primitives (references) are returned', () => { + // Solve this by making sure your memoized function uses Object.freeze + it(`will be affected by edited non primitive returns`, async () => { + let sumCalled = 0; + function sum(/** @type {object} a */ a, /** @type {object} a */ b) { + sumCalled += 1; + return { ...a, ...b }; + } + const sumMemoized = memoize(sum, { serializeObjects: true }); + + // Put in cache for args combination + const result = sumMemoized({ x: 1 }, { y: 2 }); + expect(result).to.eql({ x: 1, y: 2 }); + expect(sumCalled).to.equal(1); + + // Return from cache + const resultCached = sumMemoized({ x: 1 }, { y: 2 }); + expect(resultCached).to.equal(result); + expect(resultCached).to.eql({ x: 1, y: 2 }); + expect(sumCalled).to.equal(1); + + // Outside world can edit returned reference + resultCached.x = 3; + // Return from cache + const lastResult = sumMemoized({ x: 1 }, { y: 2 }); + expect(lastResult).to.equal(result); + expect(lastResult).to.not.eql({ x: 1, y: 2 }); + expect(sumCalled).to.equal(1); + }); + }); + }); + + describe('Asynchronous', () => { + it(`returns cached result when called with same parameters`, async () => { + let sumCalled = 0; + async function sum(/** @type {number} a */ a, /** @type {number} a */ b) { + sumCalled += 1; + return a + b; + } + const sumMemoized = memoize(sum); + + // Put in cache for args combination + expect(await sumMemoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + + // Return from cache + expect(await sumMemoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + + // Put in cache for args combination + expect(await sumMemoized(1, 3)).to.equal(4); + expect(sumCalled).to.equal(2); + + // Return from cache + expect(await sumMemoized(1, 3)).to.equal(4); + expect(sumCalled).to.equal(2); + }); + + it(`returns cached result per function for same args`, async () => { + let sumCalled = 0; + async function sum(/** @type {number} a */ a, /** @type {number} a */ b) { + sumCalled += 1; + return a + b; + } + const sumMemoized = memoize(sum); + let sum2Called = 0; + async function sum2(/** @type {number} a */ a, /** @type {number} a */ b) { + sum2Called += 1; + return a + b; + } + const sum2Memoized = memoize(sum2); + + expect(await sumMemoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(0); + + expect(await sum2Memoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + + // Both cached + expect(await sumMemoized(1, 2)).to.equal(3); + expect(await sum2Memoized(1, 2)).to.equal(3); + expect(sumCalled).to.equal(1); + expect(sum2Called).to.equal(1); + }); + }); +});