Skip to content

Commit 0ae35c0

Browse files
leipertljharb
authored andcommitted
[perf] ExportMap: Improve ExportMap.for performance on larger codebases
While looking at a larger code base https://gitlab.com/gitlab-org/gitlab, `ExportMap.for` seems to take a significant amount of time. More than half of it is spend on calculating hashes with `hashObject`. Digging a little deeper, it seems like we are calling it around 500 thousand times. Each iteration calculates the hash of a context object. This context object is created inside of the `childContext` function and consists of four parts: - `settings` -> an Object itself - `parserOptions` -> an Object itself - `parserPath` -> a String - `path` -> a String. Interestingly `settings`, `parserOptions` and `parserPath` rarely do change for us, so calculating their hashes on every iteration seems unnecessary. `hashObject` recursively calculates the hashes of each key / value pair. So instead of doing: ```js cacheKey = hashObject({settings, parserOptions, parserPath, path}) ``` We could also do: ```js cacheKey = parserPath + hashObject(parserOptions) + hashObject(settings) + path ``` This would be just as stable as before, although resulting in longer cache keys. `parserPath` and `path` would not need to be hashed, because they are strings and a single character change in them would result in a different cache key. Furthermore we can memoize the hashes of `parserOptions` and `settings`, in case they didn't change compared. The equality is checked with a simple `JSON.stringify`. We move this `cacheKey` calculation to `childContext`, adding the cache key to the `context` object. This way, we can fall back to the old calculation inside of `ExportMap.for`, as it is a public interface which consumers might be using. In our code base the results speak for itself: - 51.59s spent in `ExportMap.for`, 0ms spent in `childContext`. - 16.89s is spent in node:crypto/hash `update` (overall) - 41.02s spent in `ExportMap.for, 1.91s spent in `childContext`. - Almost no time spent in `hashObject`, actually all calls in our flame graph come from other code paths - 7.86s is spent in node:crypto/hash `update` (overall) So on this machine, and project, we are cutting the execution time of `ExportMap.for` in half. On machines, which are hashing slower, the effect might be more pronounced. Similarly machines with less memory, as the `hashObject` function creates a lot of tiny strings. One side-effect here could be, that the memoization is in-efficient if the `settings` or `parserOptions` change often. (I cannot think of such a scenario, but I am not that versed in the use cases of this plugin.) But even then, the overhead should mainly be the `JSON.stringify`.
1 parent dc596a2 commit 0ae35c0

File tree

2 files changed

+22
-2
lines changed

2 files changed

+22
-2
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
99
### Fixed
1010
- [`no-duplicates`]: remove duplicate identifiers in duplicate imports ([#2577], thanks [@joe-matsec])
1111
- [`consistent-type-specifier-style`]: fix accidental removal of comma in certain cases ([#2754], thanks [@bradzacher])
12+
- [Perf] `ExportMap`: Improve `ExportMap.for` performance on larger codebases ([#2756], thanks [@leipert])
1213

1314
### Changed
1415
- [Docs] [`no-duplicates`]: fix example schema ([#2684], thanks [@simmo])
@@ -1066,6 +1067,7 @@ for info on changes for earlier releases.
10661067

10671068
[`memo-parser`]: ./memo-parser/README.md
10681069

1070+
[#2756]: https://github.com/import-js/eslint-plugin-import/pull/2756
10691071
[#2754]: https://github.com/import-js/eslint-plugin-import/pull/2754
10701072
[#2748]: https://github.com/import-js/eslint-plugin-import/pull/2748
10711073
[#2699]: https://github.com/import-js/eslint-plugin-import/pull/2699
@@ -1737,6 +1739,7 @@ for info on changes for earlier releases.
17371739
[@kylemh]: https://github.com/kylemh
17381740
[@laysent]: https://github.com/laysent
17391741
[@le0nik]: https://github.com/le0nik
1742+
[@leipert]: https://github.com/leipert
17401743
[@lemonmade]: https://github.com/lemonmade
17411744
[@lencioni]: https://github.com/lencioni
17421745
[@leonardodino]: https://github.com/leonardodino

src/ExportMap.js

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ ExportMap.get = function (source, context) {
305305
ExportMap.for = function (context) {
306306
const { path } = context;
307307

308-
const cacheKey = hashObject(context).digest('hex');
308+
const cacheKey = context.cacheKey || hashObject(context).digest('hex');
309309
let exportMap = exportCache.get(cacheKey);
310310

311311
// return cached ignore
@@ -559,7 +559,7 @@ ExportMap.parse = function (path, content, context) {
559559
if (tsConfigInfo.tsConfigPath !== undefined) {
560560
// Projects not using TypeScript won't have `typescript` installed.
561561
if (!ts) { ts = require('typescript'); } // eslint-disable-line import/no-extraneous-dependencies
562-
562+
563563
const configFile = ts.readConfigFile(tsConfigInfo.tsConfigPath, ts.sys.readFile);
564564
return ts.parseJsonConfigFileContent(
565565
configFile.config,
@@ -781,12 +781,29 @@ export function recursivePatternCapture(pattern, callback) {
781781
}
782782
}
783783

784+
let parserOptionsHash = '';
785+
let prevParserOptions = '';
786+
let settingsHash = '';
787+
let prevSettings = '';
784788
/**
785789
* don't hold full context object in memory, just grab what we need.
790+
* also calculate a cacheKey, where parts of the cacheKey hash are memoized
786791
*/
787792
function childContext(path, context) {
788793
const { settings, parserOptions, parserPath } = context;
794+
795+
if (JSON.stringify(settings) !== prevSettings) {
796+
settingsHash = hashObject({ settings }).digest('hex');
797+
prevSettings = JSON.stringify(settings);
798+
}
799+
800+
if (JSON.stringify(parserOptions) !== prevParserOptions) {
801+
parserOptionsHash = hashObject({ parserOptions }).digest('hex');
802+
prevParserOptions = JSON.stringify(parserOptions);
803+
}
804+
789805
return {
806+
cacheKey: String(parserPath) + parserOptionsHash + settingsHash + String(path),
790807
settings,
791808
parserOptions,
792809
parserPath,

0 commit comments

Comments
 (0)