Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Significant slowdown for import/no-unused-modules rule after upgrade from v2 to v3 #145

Closed
galkin opened this issue Jul 14, 2022 · 17 comments · Fixed by #154
Closed

Significant slowdown for import/no-unused-modules rule after upgrade from v2 to v3 #145

galkin opened this issue Jul 14, 2022 · 17 comments · Fixed by #154

Comments

@galkin
Copy link

galkin commented Jul 14, 2022

I have medium size TypeScript project.
On v2 TIMING=1 npm run eslint shows

Rule Time (ms) Relative
import/no-unused-modules 5951.935 39.1%
import/namespace 3816.656 25.1%
deprecation/deprecation 2205.676 14.5%
@typescript-eslint/no-floating-promises 1131.970 7.4%
import/order 1024.044 6.7%
@typescript-eslint/no-unused-vars 244.425 1.6%
import/no-unresolved 130.393 0.9%
import/no-named-as-default-member 113.815 0.7%
import/default 87.840 0.6%
import/no-named-as-default 84.351 0.6%

On v3 TIMING=1 npm run eslint shows

Rule Time (ms) Relative
import/no-unused-modules 44214.254 73.7%
import/namespace 10107.349 16.8%
deprecation/deprecation 2321.754 3.9%
@typescript-eslint/no-floating-promises 1189.986 2.0%
import/order 1045.939 1.7%
@typescript-eslint/no-unused-vars 263.443 0.4%
import/no-unresolved 149.957 0.2%
import/no-named-as-default-member 101.459 0.2%
import/no-named-as-default 93.568 0.2%
import/default 87.550 0.1%
@JounQin
Copy link
Collaborator

JounQin commented Jul 14, 2022

cc @ljharb

Do you think anything would affect import/no-unused-modules in a resolver?

We're using enhanced-resolve in v3 to support imports/exports in package.json.

@JounQin
Copy link
Collaborator

JounQin commented Jul 14, 2022

Personally I don't think this should be related to this resolver, did you upgrade eslint-plugin-import at the same time? It could be a performance regression of rule import/no-unused-modules itself?

So a reproduction with only this resolver changed is required then.

@galkin

@ljharb
Copy link
Member

ljharb commented Jul 14, 2022

@JounQin that's unfortunate; i can't speak to its performance though. When resolve is updated to handle exports, then the node resolver will get that functionality as well, and hopefully you'd be able to switch to that.

@JounQin
Copy link
Collaborator

JounQin commented Jul 14, 2022

When resolve is updated to handle exports, then the node resolver will get that functionality as well, and hopefully you'd be able to switch to that.

Of coure.

@JounQin
Copy link
Collaborator

JounQin commented Jul 14, 2022

close due to lack of reproduction, feel free to provide one, then I'll reopen it to dig.

@JounQin JounQin closed this as completed Jul 14, 2022
@bordecal
Copy link

bordecal commented Jul 15, 2022

I am seeing a similar slowdown, with substantial increases in the timings for at least four import/* rules between recent eslint-import-resolver-typescript versions (with rule times in milliseconds as captured using TIMING=1 when running eslint):

Version Total time no-unused-modules no-extraneous-dependencies extensions no-duplicates
2.7.1 1m 43s 10998 8251 3576 <2000
3.1.1 2m 39s 41996 27714 9541 4876
3.2.6 4m 8s 98067 58369 15159 13937

N.B.: The above timings were captured without changing the versions of any dependencies other than eslint-import-resolver-typescript.

@JounQin
Copy link
Collaborator

JounQin commented Jul 15, 2022

Please provide reproduction instead of just posting the timing table, it doesn't help anything.

@bordecal
Copy link

Unfortunately, I can't share my real project, and I don't really have time to create a repro from scratch, so I tried to find a large-ish existing project that is already using the import plugin. The Angular CLI seemed like a potentially good fit, and I was able to see a similar increase in linting time by adding then updating eslint-import-resolver-typescript. This was done over a series of commits that have been pushed to a new branch in a fork of the angular-cli repo:

  1. c33d70c
    a. Prevents eslint caching (for benchmarking purposes)
    b. Adjusts eslint configuration to include rules for which timings have increased in my real code base.
    c. Captures baseline timings in linting/timings.txt.
  2. 937e51d
    a. Adds eslint-import-resolver-typescript 2.7.1 as a dev dependency.
    b. Adjusts eslint configuration to use eslint-import-resolver-typescript as a resolver for the import plugin.
    c. Updates timings file with new timings.
  3. 8cbe105
    a. Updates eslint-import-resolver-typescript from version 2.7.1 to version 3.1.1.
    b. Updates timings file with new timings.

Upgrading from v2.7.1 to v3.1.1 resulted in an increase in a 550% increase in overall linting time, with large increases in execution duration for the same rules mentioned in my earlier comment:

Version Total time no-unused-modules no-extraneous-dependencies extensions no-duplicates
2.7.1 1m 14s 18394 2969 906 3070
3.1.1 6m 52s 161792 59664 11237 70562

Does this provide a sufficient repro? If not, what exactly might you be looking for?

@bordecal
Copy link

Not sure if this might help isolate the root cause, but something I saw in another issue made me wonder about the potential impact of resolver order. I checked the resulting configuration using --print-config, and it listed the typescript resolver before the node resolver:

  "settings": {
    "import/resolver": {
      "typescript": {},
      "node": {
        "extensions": [
          ".ts",
          ".tsx",
          ".js",
          ".jsx"
        ]
      }
    },
    //...

I tried adjusting the eslint configuration to list the node resolver before the typescript resolver. and this does seem to resolve the performance problem completely: bordecal/angular-cli@b36b624

@JounQin
Copy link
Collaborator

JounQin commented Jul 18, 2022

The significant change should be enhaced-resolve for supporting custom exports fields, in v1 it's not supported at all. If you have more genius solution, feel free to PR.

I tried adjusting the eslint configuration to list the node resolver before the typescript resolver.

The node resolver does not support exports at all, neither .d.ts/@types, so the comparison is unfair.

@JounQin JounQin reopened this Jul 18, 2022
@JounQin
Copy link
Collaborator

JounQin commented Jul 19, 2022

Can you try `unsafeCache` provided by [`enhanced-resolve`](https://github.com/webpack/enhanced-resolve#resolver-options)?

@JounQin
Copy link
Collaborator

JounQin commented Jul 19, 2022

It should be fixed by 0a20ff4 (#154)

@JounQin
Copy link
Collaborator

JounQin commented Jul 19, 2022

eslint-import-resolver-typescript@3.3.0 has been released, please give it a try.

@bordecal
Copy link

Version 3.3.0 seems to be making a pretty big difference for the Angular CLI code base, reducing the total linting time from almost 7 minutes to under 2 minutes. Unfortunately, it doesn't seem to be helping with my real code base, which is still running at over 4 minutes (including 90 seconds for no-unused-modules). This has me wondering about a couple of things:

  1. Do you have any inkling at all about what might be causing the long execution times? I'd like to see if I can find a better repro code base than the Angular CLI, but I don't currently have any ideas that would help direct that effort in a meaningful way other than "big and already using eslint with typescript". 😄
  2. Since it seems like using the node resolver as the primary resolver does help quite a lot with respect to performance, do you know if this approach could lead to false negatives with any import plugin rules? If so, might you be able to provide a concrete example?

@JounQin
Copy link
Collaborator

JounQin commented Jul 19, 2022

Do you have any inkling at all about what might be causing the long execution times?

The fact in v3.3.0 is just caching resolver constructed by enhanced-resolve between files, as I linked at #145 (comment), maybe your real code base has multi eslint-import-resolver-typescript options between linting files, so the resolver cache is invalid always. If you want, you can help to PR to enhance the cache algorithm in this resolver.

do you know if this approach could lead to false negatives with any import plugin rules? If so, might you be able to provide a concrete example?

The default node resolver does not support exports filed in package.json because of its dependency resolve and this is the main feature of eslint-import-resolver-typescript@v3.

@BenShelton
Copy link

Seeing similar performance issues, with times climbing from ~40s to ~600s (or around 15x longer) after upgrading to v3.3.0. The main ones being affected are import/no-duplicates, import/order, import/default, import/no-named-as-default and import/no-named-as-default-member.

Also increased was import/export but that was not as badly affected at only 0.5% relative.

We're using an array of 6 tsconfig files under project, could that be compounding the problem? For now we've had to disable this altogether which actually caused more valid errors to be caught than before 🤷🏻

@JounQin
Copy link
Collaborator

JounQin commented Jul 25, 2022

The performance issue is tracked at #158, and PR welcome to improve that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants