Skip to content

Commit

Permalink
refactor: replaces lodash/memoize with memoize (#923)
Browse files Browse the repository at this point in the history
## Description

- replaces `lodash/memoize` with `memoize`

## Motivation and Context

We should slowly be starting to replace lodash functions everywhere as
lodash isn't really maintained anymore. Additionally `lodash/memoize`
isn't the fastest memoization function around, it looks like.

The chosen alternative (`memoize`, formerly known as `mem`) isn't the
fastest either, but
- seems to be reliably maintained
- has recent updates
- is decently fast (see benchmarks below)
- is not too large (& doesn't ship unnecessary stuff to npm like tests,
sources, bundles, benchmarks, ...)
- performs decently and reliably
- takes a custom resolver function (which it calls 'cacheKey'), which is
a thing we regrettably need.

Other candidates either have fallen out of maintenance (e.g. 10y without
updates, ...), or miss features we need.

<details>
<summary>Speed benchmarks adopted from `memize` (who adopted from them
in turn from `moize`):</summary>

- run on MacOS 12.7.4 on an 2,6Ghz Quad-Core Intel Core i7 with 16Gb RAM
- See https://github.com/aduth/memize/tree/master/benchmark for details
- The benchmark this ran was (1) adapted to run as ESM (2) adds
lodash/memoize


#### Single parameter

```text
┌────────────────────┬─────────────┬──────────────────────────┬─────────────┐
│ Name               │ Ops / sec   │ Relative margin of error │ Sample size │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ fast-memoize       │ 127,482,919 │ ± 0.28%                  │ 94          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ moize              │ 101,448,279 │ ± 0.16%                  │ 94          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ memoize            │ 89,864,460  │ ± 0.15%                  │ 96          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ memize             │ 75,527,611  │ ± 1.16%                  │ 93          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ lodash             │ 50,176,290  │ ± 0.24%                  │ 95          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ ramda              │ 48,060,225  │ ± 0.54%                  │ 94          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ underscore         │ 30,460,634  │ ± 0.42%                  │ 93          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ memoizee           │ 18,648,393  │ ± 0.51%                  │ 93          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ moize (serialized) │ 16,516,070  │ ± 0.25%                  │ 92          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ memoizerific       │ 9,432,079   │ ± 0.97%                  │ 92          │
├────────────────────┼─────────────┼──────────────────────────┼─────────────┤
│ memoizejs          │ 1,610,735   │ ± 0.28%                  │ 94          │
└────────────────────┴─────────────┴──────────────────────────┴─────────────┘
```
#### Multiple parameters, only primitives
```text
┌────────────────────┬────────────┬──────────────────────────┬─────────────┐
│ Name               │ Ops / sec  │ Relative margin of error │ Sample size │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ moize              │ 32,944,874 │ ± 3.55%                  │ 79          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memize             │ 32,530,910 │ ± 2.19%                  │ 65          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memoizee           │ 11,358,939 │ ± 2.89%                  │ 90          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memoize            │ 9,380,007  │ ± 1.55%                  │ 88          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ moize (serialized) │ 8,536,549  │ ± 0.47%                  │ 90          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memoizerific       │ 5,622,282  │ ± 1.19%                  │ 91          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ lodash             │ 5,393,388  │ ± 0.73%                  │ 92          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memoizejs          │ 1,101,890  │ ± 1.10%                  │ 92          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ fast-memoize       │ 929,638    │ ± 0.22%                  │ 95          │
└────────────────────┴────────────┴──────────────────────────┴─────────────┘
```

#### Multiple parameters, some parameters contain objects

```plaintext
┌────────────────────┬────────────┬──────────────────────────┬─────────────┐
│ Name               │ Ops / sec  │ Relative margin of error │ Sample size │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memize             │ 30,529,321 │ ± 0.97%                  │ 89          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ moize              │ 28,842,916 │ ± 0.77%                  │ 91          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memoizee           │ 10,445,895 │ ± 1.02%                  │ 92          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memoize            │ 9,349,388  │ ± 0.50%                  │ 93          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memoizerific       │ 6,107,483  │ ± 1.29%                  │ 92          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ lodash             │ 5,029,315  │ ± 0.36%                  │ 92          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ memoizejs          │ 808,879    │ ± 2.71%                  │ 94          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ moize (serialized) │ 796,306    │ ± 1.73%                  │ 88          │
├────────────────────┼────────────┼──────────────────────────┼─────────────┤
│ fast-memoize       │ 650,625    │ ± 3.18%                  │ 88          │
└────────────────────┴────────────┴──────────────────────────┴─────────────┘
```


</details>





## How Has This Been Tested?

- [x] green ci

## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation only change
- [x] Refactor (non-breaking change which fixes an issue without
changing functionality)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
  • Loading branch information
sverweij authored Mar 17, 2024
1 parent dcc8c10 commit c9d8b97
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 39 deletions.
26 changes: 26 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@
"is-installed-globally": "1.0.0",
"json5": "2.2.3",
"lodash": "4.17.21",
"memoize": "10.0.0",
"picomatch": "4.0.1",
"prompts": "2.4.2",
"rechoir": "^0.8.0",
Expand Down
2 changes: 1 addition & 1 deletion src/cache/helpers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { createHash } from "node:crypto";
import { readFileSync } from "node:fs";
import { extname } from "node:path";
import memoize from "lodash/memoize.js";
import memoize from "memoize";
// @ts-expect-error ts(2307) - the ts compiler is not privy to the existence of #imports in package.json
import { filenameMatchesPattern } from "#graph-utl/match-facade.mjs";

Expand Down
14 changes: 8 additions & 6 deletions src/extract/parse/to-javascript-ast.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { readFileSync } from "node:fs";
import { Parser as acornParser, parse as acornParse } from "acorn";
import { parse as acornLooseParse } from "acorn-loose";
import acornJsx from "acorn-jsx";
import memoize from "lodash/memoize.js";
import memoize, { memoizeClear } from "memoize";
import transpile from "../transpile/index.mjs";
import getExtension from "#utl/get-extension.mjs";

Expand Down Expand Up @@ -82,13 +82,15 @@ function getAST(pFileName, pTranspileOptions) {
* @param {any} pTranspileOptions - options for the transpiler(s) - typically a tsconfig or a babel config
* @return {acorn.Node} - a (javascript) AST
*/
export const getASTCached = memoize(
getAST,
(pFileName, pTranspileOptions) => `${pFileName}|${pTranspileOptions}`,
);
// taking the transpile options into account of the memoize cache key seems like
// a good idea. However, previously we use `${pTranspileOptions}` which always
// serializes to [object Object], which doesn't help. So for now we're not
// taking the transpile options into account. If we ever need to, it'll be
// a JSON.stringify away (which _will_ be significantly slower)
export const getASTCached = memoize(getAST);

export function clearCache() {
getASTCached.cache.clear();
memoizeClear(getASTCached);
}

export default {
Expand Down
4 changes: 2 additions & 2 deletions src/extract/parse/to-swc-ast.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import tryImport from "semver-try-require";
import memoize from "lodash/memoize.js";
import memoize, { memoizeClear } from "memoize";
import meta from "#meta.cjs";

/** @type {import('@swc/core')} */
Expand Down Expand Up @@ -29,7 +29,7 @@ function getAST(pFileName) {
export const getASTCached = memoize(getAST);

export function clearCache() {
getASTCached.cache.clear();
memoizeClear(getASTCached);
}

export default {
Expand Down
4 changes: 2 additions & 2 deletions src/extract/parse/to-typescript-ast.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { readFileSync } from "node:fs";
import tryImport from "semver-try-require";
import memoize from "lodash/memoize.js";
import memoize, { memoizeClear } from "memoize";
import transpile from "../transpile/index.mjs";
import meta from "#meta.cjs";
import getExtension from "#utl/get-extension.mjs";
Expand Down Expand Up @@ -56,7 +56,7 @@ function getAST(pFileName, pTranspileOptions) {
export const getASTCached = memoize(getAST);

export function clearCache() {
getASTCached.cache.clear();
memoizeClear(getASTCached);
}

export default {
Expand Down
19 changes: 9 additions & 10 deletions src/extract/resolve/external-module-helpers.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { readFileSync } from "node:fs";
import { join } from "node:path";
import memoize from "lodash/memoize.js";
import memoize, { memoizeClear } from "memoize";
import has from "lodash/has.js";
import { resolve } from "./resolve.mjs";
import { isScoped, isRelativeModuleName } from "./module-classifiers.mjs";
Expand Down Expand Up @@ -97,7 +97,7 @@ function bareGetPackageJson(pModuleName, pFileDirectory, pResolveOptions) {
// we need a separate caching context so as not to **** up the regular
// cruise, which might actually want to utilize the exportsFields
// and an array of extensions
"manifest-resolution"
"manifest-resolution",
);
lReturnValue = JSON.parse(readFileSync(lPackageJsonFilename, "utf8"));
} catch (pError) {
Expand All @@ -106,10 +106,9 @@ function bareGetPackageJson(pModuleName, pFileDirectory, pResolveOptions) {
return lReturnValue;
}

export const getPackageJson = memoize(
bareGetPackageJson,
(pModuleName, pBaseDirectory) => `${pBaseDirectory}|${pModuleName}`
);
export const getPackageJson = memoize(bareGetPackageJson, {
cacheKey: (pArguments) => `${pArguments[0]}|${pArguments[1]}`,
});

/**
* Tells whether the pModule as resolved to pBaseDirectory is deprecated
Expand All @@ -122,13 +121,13 @@ export const getPackageJson = memoize(
export function dependencyIsDeprecated(
pModuleName,
pFileDirectory,
pResolveOptions
pResolveOptions,
) {
let lReturnValue = false;
let lPackageJson = getPackageJson(
pModuleName,
pFileDirectory,
pResolveOptions
pResolveOptions,
);

if (Boolean(lPackageJson)) {
Expand All @@ -151,7 +150,7 @@ export function getLicense(pModuleName, pFileDirectory, pResolveOptions) {
let lPackageJson = getPackageJson(
pModuleName,
pFileDirectory,
pResolveOptions
pResolveOptions,
);

if (
Expand All @@ -165,5 +164,5 @@ export function getLicense(pModuleName, pFileDirectory, pResolveOptions) {
}

export function clearCache() {
getPackageJson.cache.clear();
memoizeClear(getPackageJson);
}
24 changes: 12 additions & 12 deletions src/extract/resolve/get-manifest.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { join, dirname, sep } from "node:path";
import { readFileSync } from "node:fs";
import memoize from "lodash/memoize.js";
import memoize, { memoizeClear } from "memoize";
import mergePackages from "./merge-manifests.mjs";

/**
Expand All @@ -23,7 +23,7 @@ const getSingleManifest = memoize((pFileDirectory) => {
// find the closest package.json from pFileDirectory
const lPackageContent = readFileSync(
join(pFileDirectory, "package.json"),
"utf8"
"utf8",
);

try {
Expand All @@ -48,7 +48,7 @@ function maybeReadPackage(pFileDirectory) {
try {
const lPackageContent = readFileSync(
join(pFileDirectory, "package.json"),
"utf8"
"utf8",
);

try {
Expand Down Expand Up @@ -81,9 +81,9 @@ function getIntermediatePaths(pFileDirectory, pBaseDirectory) {
}

// despite the two parameters there's no resolver function provided
// to the _.memoize. This is deliberate - the pBaseDirectory will typically
// be the same for each call in a typical cruise, so the lodash'
// default memoize resolver (the first param) will suffice.
// to memoize. This is deliberate - the pBaseDirectory will typically
// be the same for each call in a typical cruise, so the default
// memoize resolver (the first param) will suffice.
const getCombinedManifests = memoize((pFileDirectory, pBaseDirectory) => {
// The way this is called, this shouldn't happen. If it is, there's
// something gone terribly awry
Expand All @@ -94,16 +94,16 @@ const getCombinedManifests = memoize((pFileDirectory, pBaseDirectory) => {
throw new Error(
`Unexpected Error: Unusual baseDir passed to package reading function: '${pBaseDirectory}'\n` +
`Please file a bug: https://github.com/sverweij/dependency-cruiser/issues/new?template=bug-report.md` +
`&title=Unexpected Error: Unusual baseDir passed to package reading function: '${pBaseDirectory}'`
`&title=Unexpected Error: Unusual baseDir passed to package reading function: '${pBaseDirectory}'`,
);
}

const lReturnValue = getIntermediatePaths(
pFileDirectory,
pBaseDirectory
pBaseDirectory,
).reduce(
(pAll, pCurrent) => mergePackages(pAll, maybeReadPackage(pCurrent)),
{}
{},
);

return Object.keys(lReturnValue).length > 0 ? lReturnValue : null;
Expand All @@ -128,7 +128,7 @@ const getCombinedManifests = memoize((pFileDirectory, pBaseDirectory) => {
export function getManifest(
pFileDirectory,
pBaseDirectory,
pCombinedDependencies = false
pCombinedDependencies = false,
) {
if (pCombinedDependencies) {
return getCombinedManifests(pFileDirectory, pBaseDirectory);
Expand All @@ -138,6 +138,6 @@ export function getManifest(
}

export function clearCache() {
getCombinedManifests.cache.clear();
getSingleManifest.cache.clear();
memoizeClear(getCombinedManifests);
memoizeClear(getSingleManifest);
}
4 changes: 2 additions & 2 deletions src/extract/resolve/resolve-amd.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { accessSync, R_OK } from "node:fs";
import { relative, join } from "node:path";
import memoize from "lodash/memoize.js";
import memoize, { memoizeClear } from "memoize";
import { isBuiltin } from "./is-built-in.mjs";
import pathToPosix from "#utl/path-to-posix.mjs";

Expand Down Expand Up @@ -63,5 +63,5 @@ export function resolveAMD(
}

export function clearCache() {
fileExists.cache.clear();
memoizeClear(fileExists);
}
8 changes: 4 additions & 4 deletions src/report/anon/anonymize-path-element.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import memoize from "lodash/memoize.js";
import memoize, { memoizeClear } from "memoize";
import randomString from "./random-string.mjs";

function replace(pElement, pIndex, pWordList) {
Expand All @@ -13,7 +13,7 @@ function replaceFromWordList(pPathElement, pWordList, pCached) {
.map((pElement, pIndex) =>
pCached
? replaceCached(pElement, pIndex, pWordList)
: replace(pElement, pIndex, pWordList)
: replace(pElement, pIndex, pWordList),
)
.join(".");
}
Expand Down Expand Up @@ -49,7 +49,7 @@ export function anonymizePathElement(
pPathElement,
pWordList = [],
pWhiteListRE = /^$/g,
pCached = true
pCached = true,
) {
return pWhiteListRE.test(pPathElement)
? pPathElement
Expand All @@ -63,5 +63,5 @@ export function anonymizePathElement(
* @returns {void}
*/
export function clearCache() {
replaceCached.cache.clear();
memoizeClear(replaceCached);
}

0 comments on commit c9d8b97

Please sign in to comment.