Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 33d8520

Browse files
committedAug 13, 2020
fix: cache pending/ignore config lookups for faster perf/less memory
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
1 parent 8a7de7d commit 33d8520

File tree

1 file changed

+62
-22
lines changed

1 file changed

+62
-22
lines changed
 

‎lib/get-config.js

+62-22
Original file line numberDiff line numberDiff line change
@@ -362,47 +362,87 @@ function _determineConfigForSeverity(config) {
362362
}
363363
}
364364

365-
function statusForModule(type, config, options) {
366-
let moduleId = options.moduleId;
367-
let list = config[type];
368-
let configPath = options.configPath || '';
369-
if (!list) {
370-
return false;
365+
class ModuleStatusCache {
366+
constructor(config, configPath) {
367+
this.config = config;
368+
this.configPath = configPath || '';
369+
this.cache = {
370+
pending: {},
371+
ignore: {},
372+
};
373+
this.processCWD = process.cwd();
371374
}
372375

373-
for (const item of list) {
374-
let fullPathModuleId = path.resolve(process.cwd(), moduleId);
376+
lookupPending(moduleId) {
377+
if (!moduleId || !this.config.pending) {
378+
return false;
379+
}
380+
if (!this.cache.pendingLookup) {
381+
this.cache.pendingLookup = this._extractPendingCache();
382+
}
383+
if (!(moduleId in this.cache.pending)) {
384+
const fullPathModuleId = path.resolve(this.processCWD, moduleId);
385+
this.cache.pending[moduleId] = this.cache.pendingLookup[fullPathModuleId];
386+
}
387+
return this.cache.pending[moduleId];
388+
}
375389

376-
if (typeof item === 'function' && item(moduleId)) {
377-
return true;
378-
} else if (typeof item === 'string') {
379-
let fullPathItem = path.resolve(process.cwd(), path.dirname(configPath), item);
380-
if (fullPathModuleId === fullPathItem) {
381-
return true;
382-
}
383-
} else if (item.moduleId) {
384-
let fullPathItem = path.resolve(process.cwd(), path.dirname(configPath), item.moduleId);
385-
if (fullPathModuleId === fullPathItem) {
386-
return item;
390+
lookupIgnore(moduleId) {
391+
if (!(moduleId in this.cache.ignore)) {
392+
const ignores = this.config['ignore'] || [];
393+
this.cache.ignore[moduleId] = ignores.find((match) => match(moduleId));
394+
}
395+
return Boolean(this.cache.ignore[moduleId]);
396+
}
397+
398+
_extractPendingCache() {
399+
const list = this.config.pending;
400+
const byFullModuleId = {};
401+
402+
if (!list) {
403+
return byFullModuleId;
404+
}
405+
406+
for (const item of list) {
407+
if (typeof item === 'string') {
408+
const fullPath = this.resolveFullModuleId(item);
409+
byFullModuleId[fullPath] = true;
410+
} else if (item.moduleId) {
411+
const fullPath = this.resolveFullModuleId(item.moduleId);
412+
byFullModuleId[fullPath] = item;
387413
}
388414
}
415+
416+
return byFullModuleId;
389417
}
390418

391-
return false;
419+
resolveFullModuleId(moduleId) {
420+
if (!this._baseDirBasedOnConfigPath) {
421+
this._baseDirBasedOnConfigPath = path.resolve(this.processCWD, path.dirname(this.configPath));
422+
}
423+
return path.resolve(this._baseDirBasedOnConfigPath, moduleId);
424+
}
392425
}
426+
427+
let configModuleCacheMap = new WeakMap();
428+
393429
/**
394430
* Returns the config in conjunction with overrides configuration.
395431
* @param {*} config
396432
* @param {*} filePath
397433
*/
398434
function getConfigForFile(config, options) {
435+
if (!configModuleCacheMap.has(config)) {
436+
configModuleCacheMap.set(config, new ModuleStatusCache(config, options.configPath));
437+
}
438+
let moduleStatusCache = configModuleCacheMap.get(config);
399439
let filePath = options.filePath;
400440
let configuredRules = config.rules;
401441
let overrides = config.overrides;
402442

403443
let fileConfig = Object.assign({}, config, {
404-
pendingStatus: statusForModule('pending', config, options),
405-
shouldIgnore: statusForModule('ignore', config, options),
444+
pendingStatus: moduleStatusCache.lookupPending(options.moduleId),
445+
shouldIgnore: moduleStatusCache.lookupIgnore(options.moduleId),
406446
});
407447

408448
if (filePath && overrides) {

0 commit comments

Comments
 (0)
Please sign in to comment.