Skip to content

Commit

Permalink
fix: cache pending/ignore config lookups for faster perf/less memory
Browse files Browse the repository at this point in the history
This commit improves the lookups of `ignore` and `pending` configs by reducing the amount of
iterations required over those config properties.

tl;dr:

- cached the config lookups for `pending` and `ignore` to reduce the amount of times we have to
  re-iterate the `pending` or `ignore` arrays provided by the config.
- cache string lookups for normalized moduleId paths since

This was done in three steps:

First Step

The normalized `moduleId` was cached. Running ember-template-lint on a big codebase (668 template
files) took 12.94 seconds to run. I used this command in the app's codebase directory to profile:

```
node --prof ../ember-template-lint/bin/ember-template-lint.js .
```

Next, I generated a graph with the [flamebearer][flamebearer] npm package:

```
node --prof-process --preprocess -j isolate*.log | flamebearer
```

<img width="1188" alt="Screen_Shot_2020-08-13_at_9_25_14_AM" src="https://user-images.githubusercontent.com/1275021/90181853-f29c8200-dd65-11ea-8163-c2e3f94a5e00.png">

Profiling showed that ~19% of the CPU time was spent inside of the `statusForModule` function. Upon
further investigation, I noticed that the [normalized moduleId based on the current working
directory was being generated in a loop][3], creating a new string for potentially every item in the
`pending` or `ignore` config. Our app has no `ignore` config, but does have a lot of `pending`
entries.

Moving the `fullPathModuleId` to a variable outside the loop took the runtime from 12.94 seconds to
11.04 seconds, about ~2 seconds of savings on its own!

An important thing to keep in mind for the next two steps: `statusForModule` is called _at least
once per rule_ to determine if the rule should be ignored or allowed to fail (or removed from the
pending file if now passing). This means that any functions ran or strings created by
`statusForModule` are, in the worst case scenario, calculated
`numberOfFiles * pendingRuleConfigItems * ignoreConfigItems` times if the loop doesn't find the
config for the file early, as it has to search the entire array of `pending` or `ignore` items for
every file.

Rather than rejoining the paths, they are instead generated once and stored in a cache object. This
reduces the number of strings generated and time generating the same strings over and over. Caching
the result of `process.cwd` (since it seems unlikely to change once the program is started)
potentially has a nice performance side effect for users on Node 10 (supported until April 2021) as
[process.cwd was not cached until Node 12.2](nodejs/node#27224).

Second Step

Lookups for `ignore` and `pending` were separated out into different caches. This was done because
while [`statusForModule` will check for a function to run][1] instead of a string, in practice this
is only true for `ignore` rules. The reason `ignore` rules need the function check is that they are
always a function due to being converted from [strings to functions via the micromatch module][2].
Caching the lookup of `ignore` rules reduced the need to run the functions for every rule/file.

Third Step

Last, but not least: `pending` rules always seem to be a list of `object`s or `string`s (as
generated by `--print-pending`) and having a list that potentially has a function in it doesn't seem
likely given that the recommendation from ember-template-lint is to copy/paste the output of
`--print-pending`.

After re-profiling, the runtime went from 12.94 seconds to 9.04 seconds on my machine. The
`get-config` file no longer showed up in the profile.

<img width="1116" alt="Screen Shot 2020-08-13 at 12 07 40 PM" src="https://user-images.githubusercontent.com/1275021/90181902-0647e880-dd66-11ea-9cd4-cc2778198139.png">

[1]:
  https://github.com/ember-template-lint/ember-template-lint/blob/5937b63bed30380b4bce0f96b19061658a176840/lib/get-config.js#L376-L377
[2]:
  https://github.com/ember-template-lint/ember-template-lint/blob/5937b63bed30380b4bce0f96b19061658a176840/lib/get-config.js#L280
[3]:
  https://github.com/ember-template-lint/ember-template-lint/blob/5937b63bed30380b4bce0f96b19061658a176840/lib/get-config.js#L374
[flamebearer]: https://github.com/mapbox/flamebearer
  • Loading branch information
fivetanley committed Aug 20, 2020
1 parent 1c72839 commit 50f0a87
Showing 1 changed file with 62 additions and 22 deletions.
84 changes: 62 additions & 22 deletions lib/get-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,47 +364,87 @@ function _determineConfigForSeverity(config) {
}
}

function statusForModule(type, config, options) {
let moduleId = options.moduleId;
let list = config[type];
let configPath = options.configPath || '';
if (!list) {
return false;
class ModuleStatusCache {
constructor(config, configPath) {
this.config = config;
this.configPath = configPath || '';
this.cache = {
pending: {},
ignore: {},
};
this.processCWD = process.cwd();
}

for (const item of list) {
let fullPathModuleId = path.resolve(process.cwd(), moduleId);
lookupPending(moduleId) {
if (!moduleId || !this.config.pending) {
return false;
}
if (!this.cache.pendingLookup) {
this.cache.pendingLookup = this._extractPendingCache();
}
if (!(moduleId in this.cache.pending)) {
const fullPathModuleId = path.resolve(this.processCWD, moduleId);
this.cache.pending[moduleId] = this.cache.pendingLookup[fullPathModuleId];
}
return this.cache.pending[moduleId];
}

if (typeof item === 'function' && item(moduleId)) {
return true;
} else if (typeof item === 'string') {
let fullPathItem = path.resolve(process.cwd(), path.dirname(configPath), item);
if (fullPathModuleId === fullPathItem) {
return true;
}
} else if (item.moduleId) {
let fullPathItem = path.resolve(process.cwd(), path.dirname(configPath), item.moduleId);
if (fullPathModuleId === fullPathItem) {
return item;
lookupIgnore(moduleId) {
if (!(moduleId in this.cache.ignore)) {
const ignores = this.config['ignore'] || [];
this.cache.ignore[moduleId] = ignores.find((match) => match(moduleId));
}
return Boolean(this.cache.ignore[moduleId]);
}

_extractPendingCache() {
const list = this.config.pending;
const byFullModuleId = {};

if (!list) {
return byFullModuleId;
}

for (const item of list) {
if (typeof item === 'string') {
const fullPath = this.resolveFullModuleId(item);
byFullModuleId[fullPath] = true;
} else if (item.moduleId) {
const fullPath = this.resolveFullModuleId(item.moduleId);
byFullModuleId[fullPath] = item;
}
}

return byFullModuleId;
}

return false;
resolveFullModuleId(moduleId) {
if (!this._baseDirBasedOnConfigPath) {
this._baseDirBasedOnConfigPath = path.resolve(this.processCWD, path.dirname(this.configPath));
}
return path.resolve(this._baseDirBasedOnConfigPath, moduleId);
}
}

let configModuleCacheMap = new WeakMap();

/**
* Returns the config in conjunction with overrides configuration.
* @param {*} config
* @param {*} filePath
*/
function getConfigForFile(config, options) {
if (!configModuleCacheMap.has(config)) {
configModuleCacheMap.set(config, new ModuleStatusCache(config, options.configPath));
}
let moduleStatusCache = configModuleCacheMap.get(config);
let filePath = options.filePath;
let configuredRules = config.rules;
let overrides = config.overrides;

let fileConfig = Object.assign({}, config, {
pendingStatus: statusForModule('pending', config, options),
shouldIgnore: statusForModule('ignore', config, options),
pendingStatus: moduleStatusCache.lookupPending(options.moduleId),
shouldIgnore: moduleStatusCache.lookupIgnore(options.moduleId),
});

if (filePath && overrides) {
Expand Down

0 comments on commit 50f0a87

Please sign in to comment.