Avoid clearing Prettier cache when not using prettierrc

[lencioni]

At Airbnb we've noticed that the Prettier ESLint rule is extraordinarily
slow. When running ESLint with TIMING=1, Prettier is two orders of
magnitude slower than our other slowest rules (which come from
eslint-plugin-import mostly).

I spent some time investigating this today. I ran the Chrome DevTools
profiler while running ESLint on a directory with 480 files in it.
Looking at the flame charts, I noticed that this plugin ended up calling
Prettier's pluginSearchDirs.map.pluginSearchDir for every file that was
being linted, even though it was being memoized. Through some logging, I
determined that the Prettier cache was being cleared between every file,
and narrowed it down to this line, which was added here:

#55 (comment)

It looks like this cache busting was added to make it so long-running
ESLint processes (e.g. for vscode-eslint) would be able to pick up
changes to prettierrc files without having to reload the process.
Thankfully, we don't use a prettierrc file at Airbnb right now, in favor
of putting our Prettier config inline with our ESLint config. So the
quick performance fix for us is to simply skip the cache busting when
this option is not enabled.

In my profiling, this reduces the time spent in ESLint's verifyAndFix
when running on the same 480 files from 34 seconds to 26 seconds, for a
total savings of 8 seconds.

For folks who are using prettierrc files, this is still pretty crappy,
so it would be great to find a better way to do this. Unfortunately my
knowledge of the inner workings of vscode-eslint and ESLint are not
enough to know if there might be a better way to do this--e.g. maybe
there's some ESLint option that we can respect here?

Before:

After:

[BPScott]

Ooof that's a chunky speedup. Great investigating!

I've just tried this out with my go-to codebase that's a bit bigger (~700 files for eslint) and am seeing an 8 second speedup too - time to run just the prettier rule goes from ~16s to ~8s. Uncached total runtime goes from ~72s to ~64s. Our of interest what's the speedup for just the prettier rule on your codebase?

This is a big speedup so given most people are going to be spending time running lints rather than editing configs I'm rather tempted to say we straight up remove the clearCacheConfig call entirely and document that people are gonna need to reload their vscode windows if they edit their config.

VSCode eslint already requires you to reload the window to cachebust other plugin configs such as
changes to graphql schemas when using eslint-plugin-graphql so I don't think we should be a special case given how much we lose by trying to handle that.

Yo @dbaeumer, sorry for the wild ping, but it looks like vscode-eslint handles "if the .eslintrc changes then bust caches and reload eslint" here. It's not really vscode-eslint's problem but would you consider accepting adding ..prettierr{c.js,c.yaml,c.yml,c,c.json} (etc) to that dirty check for the sake of trying to do the right thing automagically in our case?

[lencioni]

Even if the vscode plugin does this, there area bunch of others that would also need to have this special case, like sublime, atom, and eslint_d.

One option would be to add an async thing that clears the cache later, but I suspect that won't be ideal now that eslint has gone async. I think this would be ideal if node had requestIdleCallback (nodejs/node#2543), but we could probably mimic something pretty decent without too much overhead using recursive setTimeouts.

I think ideally eslint would have an API for rules to do some cleanup work after all the rules finished. Obviously, that won't be backwards compatible though.

In any case, my preference would be to get this change published as is ASAP, since it is a strict improvement, and then follow up with deeper changes. Then we can decide if we want to break people's workflows or try to make it just work.

I'd be happy to submit a separate PR that does the setTimeouts shenanigans if you like that idea.

[lencioni]

Our of interest what's the speedup for just the prettier rule on your codebase?

ESLint takes too long to run locally on our entire codebase right now. In CI, we use jest-runner-eslint to speed things up a lot. We also have a large monorepo and do some things to only run ESLint on parts of the monorepo that might be affected by any given change. With all of this in place, and running on some pretty beefy boxes, we see max run times around 14 minutes lately. I'd be happy to report back with new numbers after this change is published, but I'm expecting that this will drop that time considerably.

[dbaeumer]

@BPScott yes, I would accept suc a PR since there is a huge likelihood that if a prettier config file is present and eslint is running that the corresponding plugin is used.

[lencioni]

Our of interest what's the speedup for just the prettier rule on your codebase?

Thanks for the quick publish on this! I've updated our plugin and early results look like it saves us 1-2 minutes in CI, which is somewhere around 10-15% of the total job runtime.

This is now the TIMING=1 output for one of our Jest workers:

[2020-06-15T14:55:38Z] Rule                                     |  Time (ms) | Relative
[2020-06-15T14:55:38Z] :----------------------------------------|-----------:|--------:
[2020-06-15T14:55:38Z] import/no-named-as-default               | 112620.047 |    27.2%
[2020-06-15T14:55:38Z] prettier/prettier                        |  82839.768 |    20.0%
[2020-06-15T14:55:38Z] import/no-self-import                    |  23323.196 |     5.6%
[2020-06-15T14:55:38Z] import/default                           |  20537.711 |     5.0%
[2020-06-15T14:55:38Z] import/no-extraneous-dependencies        |  16908.916 |     4.1%
[2020-06-15T14:55:38Z] rulesdir/no-hyperloop-restricted-imports |  10973.224 |     2.7%
[2020-06-15T14:55:38Z] import/named                             |  10957.346 |     2.6%
[2020-06-15T14:55:38Z] react/void-dom-elements-no-children      |   7367.993 |     1.8%
[2020-06-15T14:55:38Z] react/no-unknown-property                |   6615.837 |     1.6%
[2020-06-15T14:55:38Z] react/destructuring-assignment           |   6272.271 |     1.5%

[BPScott]

Even if the vscode plugin does this, there area bunch of others that would also need to have this special case, like sublime, atom, and eslint_d.

Yepyep, and I'm trying to say that all of those also need some mechanism to solve e.g. cachebusting after graphql schema changes if consumers are using eslint-plugin-graphql. So I think the responsibility of specifying and handling that mechanism lies with the tools that are creating long-running eslint processes rather than the plugins themselves.