gund / eslint-plugin-deprecation

ESLint rule that reports usage of deprecated code
GNU Lesser General Public License v3.0
329 stars 38 forks source link

Performance issues #44

Open thany opened 2 years ago

thany commented 2 years ago

In a rather large project, that unfortunately I'm not at liberty to share, performance of this plugin is a bit dissappointing.

The result of this is not only that it might take a very long time to lint the entire project, but more importantly, in VScode with lint-fixing on save enabled, it will take several seconds for each save action to complete.

I understand it's difficult to get this problem fixed in this particular repository, however, there just might just be a workaround 😎

Inspired on this more generic issue and this preceeding issue, the solution appears to be this piece of VScode configuration:

    "eslint.enable": true,
    "eslint.useESLintClass": true,
    "eslint.codeActionsOnSave.rules": [
        "!deprecation/deprecation",
        "*",
    ],
    "editor.codeActionsOnSave": {
        "source.fixAll": false,
        "source.fixAll.eslint": true
    },

Essentially skipping the deprecation/deprecation rule. This is fine, because the rule only checks code, but doesn't provide fixes, which is exactly the only thing linting on save is good for. The other bits of config are important to get the VScode extension to understand the rules setting in the first place.

Now, I realise that this only fixes it for VScode (well, it does on my machine), so if you enjoy a different editor, you might have to seek out a different solution. However, the above configuration might nudge that search in the right direction. I would not be surprised if eslint plugins for other editors have a similar rules setting to filter certain rules.

And perhaps it's a good idea to list those solutions below, when anyone finds one.

gund commented 2 years ago

Hey! Thanks for sharing your insights on this!

While there maybe something that slows this rule or perhaps a more optimised way of achieving the same linting I wonder maybe you are running some other actions on save that take longer time (sometimes prettier is the one that is slow).

Looking at this piece of config:

"source.fixAll": false,
"source.fixAll.eslint": true

You are basically disabling all of the fixes and keep only eslint fixes which might point to something else being slow...

Could you please confirm if this is still slow without that setting specified?

thany commented 2 years ago

I already did that kind of research πŸ˜‰

VScode is showing a notification in the bottom-right that says Getting code action from "ESLint" so it's quite definitely not another extension that provides fixes on save. This is further confirmed by the fact that I don't even have any other extensions for fixing on save, as far as I'm aware, and the fact that performance was improved when I told it to skip deprecation/deprecation.

Of course, I'm not saying everyone should be adding this config to their editor. It's entirely possible that it heavily depends on the volume and structure of a project, whether or not a performance hit (if any) from deprecation/deprecation is still workable. I was only stating that if you run into ESLint performance problems, this might be an option for you. Or not πŸ˜€

gund commented 2 years ago

Awesome @thany, thanks for confirming this! Indeed a very good find!

I don't have any experience in debugging eslint rules unfortunately so not quite sure if this is actionable at this point so I might move this issue to discussions instead, unless, of course, if somebody else have an idea how to tackle this πŸ™‚

thany commented 2 years ago

so I might move this issue to discussions instead

That's fine. I'm not used to discussions being a thing yet, so I go to issues mainly out of habit. Your call πŸ˜€

bhavitsharma commented 2 years ago

On our fairly large codebase, node OOMs even after 10 gigs of memory.

marekdon commented 2 years ago

Problem with performance is also visible when running timing (TIMING=1 npm run lint):

Rule Time (ms) Relative
deprecation/deprecation 5270.079 77.2%
jsdoc/newline-after-description 279.451 4.1%
no-redeclare 268.256 3.9%
@typescript-eslint/naming-convention 181.790 2.7%
id-match 76.086 1.1%
@typescript-eslint/no-empty-function 63.833 0.9%
object-shorthand 63.655 0.9%
no-alert 39.108 0.6%
one-var 38.894 0.6%
@typescript-eslint/no-loss-of-precision 36.527 0.5%
gund commented 1 year ago

Just a heads up - we've landed a small perf PR in v1.3.3 which may reduce some seconds from your times but don't get too excited as it was a really tiny fix 😁

thany commented 1 year ago

Thanks! I'm no longer on the same dev team that used this plugin, but I'll consider using it with my new teammates πŸ™‚

HolgerJeromin commented 1 year ago

Ref https://github.com/typescript-eslint/typescript-eslint/issues/2620#issuecomment-702808500 for some insights into performance.

jonahallibone commented 2 months ago

Jus to throw in my two cents, my results are pretty poor as well for this rule

Rule                                 |  Time (ms) | Relative
:------------------------------------|-----------:|--------:
deprecation/deprecation              | 171136.883 |    76.9%
import/no-cycle                      |  34966.530 |    15.7%
@typescript-eslint/no-redeclare      |   1755.183 |     0.8%
@typescript-eslint/naming-convention |   1503.848 |     0.7%
react/no-unstable-nested-components  |    827.228 |     0.4%
react/no-array-index-key             |    759.989 |     0.3%
react/destructuring-assignment       |    720.545 |     0.3%
react/jsx-fragments                  |    678.566 |     0.3%
@next/next/no-html-link-for-pages    |    551.337 |     0.2%
react/function-component-definition  |    550.883 |     0.2%
silverwind commented 2 months ago

Same observation from me, this plugin is 3 times slower than already slow import rules:

Rule                                   | Time (ms) | Relative
:--------------------------------------|----------:|--------:
deprecation/deprecation                |  1130.671 |    27.8%
import/named                           |   493.357 |    12.1%
import/no-unused-modules               |   479.098 |    11.8%
@typescript-eslint/no-misused-promises |   259.022 |     6.4%
import/no-cycle                        |   165.034 |     4.1%
@typescript-eslint/no-unused-vars      |   118.863 |     2.9%
@stylistic/js/indent                   |    93.948 |     2.3%
sonarjs/no-ignored-return              |    71.976 |     1.8%
react/display-name                     |    59.778 |     1.5%
import/default                         |    53.713 |     1.3%

https://github.com/EvgenyOrekhov/eslint-config-hardcore/issues/732#issuecomment-1546296914 has some more insights into alternatives to this plugin, but none are particularily fast it appears.

reduckted commented 2 months ago

Does anyone have a link to an open source repository where this rule is really slow? I have some ideas for improvements that I'd like to try out, but don't have an open source repository to try them out on.

reduckted commented 2 months ago

Does anyone have a link to an open source repository where this rule is really slow?

Never mind. I ended up using @typescript-eslint/eslint-plugin to test things out. The rule wasn't particularly slow in that repository, but I was able to prove that all of my ideas for performance improvements would have zero impact. πŸ˜†

For the record though, it looks like almost all of the time is spent in TypeScript's TypeChecker class, so this plugin seems to be doing the best that it can.

silverwind commented 2 months ago

Yes, I assume the issue is mostly a upstream one because all deprecation checker plugins are kind of slow as per https://github.com/EvgenyOrekhov/eslint-config-hardcore/issues/732#issuecomment-1546296914. I guess a few % could be shaved off in the plugin as there is a visible difference to etc/no-deprecated and this plugin.