Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information