Skip to content

Commit

Permalink
[utils] [perf] Performance of fullResolve
Browse files Browse the repository at this point in the history
While looking at a larger code base https://gitlab.com/gitlab-org/gitlab,
I've came to realize that `fullResolve` takes a lot of CPU cycles,
particularly the `hashObject` calls inside it.

I applied the following patch locally to see how often this is called and
how many unique hashes were produced:

```diff
diff --git a/utils/resolve.js b/utils/resolve.js
index 4a35c6a..3c28324 100644
--- a/utils/resolve.js
+++ b/utils/resolve.js
@@ -83,13 +83,28 @@ function relative(modulePath, sourceFile, settings) {
   return fullResolve(modulePath, sourceFile, settings).path;
 }

+let prevSettings = null;
+let nonEqualSettings = 0;
+let totalCalls = 0;
+let uniqueHashes = new Set();
 function fullResolve(modulePath, sourceFile, settings) {
   // check if this is a bonus core module
   const coreSet = new Set(settings['import/core-modules']);
   if (coreSet.has(modulePath)) return { found: true, path: null };

   const sourceDir = path.dirname(sourceFile);
-  const cacheKey = sourceDir + hashObject(settings).digest('hex') + modulePath;
+
+  totalCalls+=1;
+  const hash = hashObject(settings).digest('hex');
+
+  if(prevSettings !== settings){
+    uniqueHashes.add(hash);
+    prevSettings = settings;
+    nonEqualSettings+=1;
+    console.log(`fullResolve | Total calls:${totalCalls} | Non-Equal settings:${nonEqualSettings} | Unique hashes:${uniqueHashes.size} | dir:${sourceDir}`)
+  }
+
+  const cacheKey = sourceDir + hash + modulePath;

   const cacheSettings = ModuleCache.getSettings(settings);
```

For our code base, `fullResolve` is called more than 570 thousand times.
The simple in-equality `!==` code path is taken 1090 times.
Actually only _four_ unique hashes are produced, meaning we only have
four unique settings across our code base.

I assume that a full object equality comparison might not be needed,
and a simple object comparison with `!==` already would reduce the amount
of `hashObject` calls by 570x. This is what is implemented in this commit.

Time spend in `fullResolve` was reduced by ~38%:
- Before: 17% (19.10s) of our total execution time
- After: 11% (11.86s) of our total execution time

The effect might even be more pronounced on machines that are slower when
calculating `sha256` hashes or that have less memory, as the `hashObject`
method tends to create loads of small strings which need to be garbage
collected.
  • Loading branch information
leipert committed Apr 8, 2023
1 parent 6be042b commit 712ab68
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
5 changes: 5 additions & 0 deletions utils/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

## Unreleased

### Fixed
- Improve performance of `fullResolve` for large projects ([#2755], thanks [@leipert])

## v2.7.4 - 2022-08-11

### Fixed
Expand Down Expand Up @@ -123,6 +126,7 @@ Yanked due to critical issue with cache key resulting from #839.
### Fixed
- `unambiguous.test()` regex is now properly in multiline mode

[#2755]: https://github.com/import-js/eslint-plugin-import/pull/2755
[#2523]: https://github.com/import-js/eslint-plugin-import/pull/2523
[#2431]: https://github.com/import-js/eslint-plugin-import/pull/2431
[#2350]: https://github.com/import-js/eslint-plugin-import/issues/2350
Expand Down Expand Up @@ -160,6 +164,7 @@ Yanked due to critical issue with cache key resulting from #839.
[@iamnapo]: https://github.com/iamnapo
[@JounQin]: https://github.com/JounQin
[@kaiyoma]: https://github.com/kaiyoma
[@leipert]: https://github.com/leipert
[@manuth]: https://github.com/manuth
[@maxkomarychev]: https://github.com/maxkomarychev
[@mgwalker]: https://github.com/mgwalker
Expand Down
10 changes: 9 additions & 1 deletion utils/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,21 @@ function relative(modulePath, sourceFile, settings) {
return fullResolve(modulePath, sourceFile, settings).path;
}

let prevSettings = null;
let memoizedHash = '';
function fullResolve(modulePath, sourceFile, settings) {
// check if this is a bonus core module
const coreSet = new Set(settings['import/core-modules']);
if (coreSet.has(modulePath)) return { found: true, path: null };

const sourceDir = path.dirname(sourceFile);
const cacheKey = sourceDir + hashObject(settings).digest('hex') + modulePath;

if (prevSettings !== settings){
memoizedHash = hashObject(settings).digest('hex');
prevSettings = settings;
}

const cacheKey = sourceDir + memoizedHash + modulePath;

const cacheSettings = ModuleCache.getSettings(settings);

Expand Down

0 comments on commit 712ab68

Please sign in to comment.