import/no-duplicates performance slow

[tjx666]

code:

import fs from 'node:fs/promises';
import os from 'node:os';
import path from 'node:path';
import util from 'node:util';

import { findWorkspacePackagesNoCheck } from '@pnpm/find-workspace-packages';
import consola from 'consola';
import c from 'picocolors';

console.log(fs, path, findWorkspacePackagesNoCheck, consola, c, os, util);

Versions:

  System:
    OS: macOS 13.2.1
    CPU: (8) x64 Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz
    Memory: 1.74 GB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.15.0 - ~/Library/Caches/fnm_multishells/87111_1679468379491/bin/node
    npm: 9.6.2 - ~/Library/Caches/fnm_multishells/87111_1679468379491/bin/npm
  npmPackages:
    eslint: ^8.36.0 => 8.36.0 
    eslint-plugin-import": 2.27.5

[R1ON]

I have the same problem :(

Rule                              | Time (ms) | Relative
:---------------------------------|----------:|--------:
import/no-duplicates              | 72224.446 |    63.4%
import/default                    | 31071.649 |    27.3%
indent                            |  2149.804 |     1.9%
no-imports/outside-special-folder |  2124.553 |     1.9%
import/no-unresolved              |  1399.763 |     1.2%
react/no-direct-mutation-state    |   600.695 |     0.5%
import/export                     |   268.659 |     0.2%
import/no-named-as-default-member |   251.551 |     0.2%
react/no-typos                    |   230.053 |     0.2%
react/require-render-return       |   205.671 |     0.2%

[ljharb]

and if you comment out no-duplicates, what happens to the other import rules?

[seiyab]

It might not a problem about a specific rule. For our project, import/extensions is extremely slow consitently.

Rule                                 |  Time (ms) | Relative
:------------------------------------|-----------:|--------:
import/extensions                    | 103423.841 |    64.9%
import/no-extraneous-dependencies    |  19748.530 |    12.4%
import/no-cycle                      |  14308.905 |     9.0%
import/order                         |   2371.498 |     1.5%
import/no-relative-packages          |   1831.405 |     1.1%
@typescript-eslint/naming-convention |   1697.527 |     1.1%
react/no-unstable-nested-components  |   1021.375 |     0.6%
react/function-component-definition  |    915.998 |     0.6%
import/no-unresolved                 |    685.094 |     0.4%
react/prefer-stateless-function      |    638.503 |     0.4%

However, when I turn off import/extensions as https://github.com/import-js/eslint-plugin-import/issues/2746#issuecomment-1654483023 suggests, another rule get extremely slow.

Rule                                 |  Time (ms) | Relative
:------------------------------------|-----------:|--------:
import/no-extraneous-dependencies    | 114140.980 |    77.9%
import/no-cycle                      |  12257.164 |     8.4%
import/order                         |   2212.973 |     1.5%
import/no-relative-packages          |   1627.329 |     1.1%
@typescript-eslint/naming-convention |   1573.758 |     1.1%
react/no-unstable-nested-components  |    963.062 |     0.7%
react/function-component-definition  |    809.087 |     0.6%
import/no-unresolved                 |    678.688 |     0.5%
react/prefer-stateless-function      |    608.823 |     0.4%
react/static-property-placement      |    557.109 |     0.4%

I don't know plugin-import internal but I'm suspicous of something like common setup.

[soryy708]

Related to ExportMap performance?

But no-duplicates doesn't use ExportMap does it?

[ljharb]

true, but it does invoke the resolver, which in turn uses the ModuleCache, so maybe that's the problem.

[zanminkian]

Before commenting out Top 3:

After comment out Top 3:

import/no-relative-packages, import/no-duplicates and import/no-self-import are slow!

[ljharb]

Can you confirm you're using the latest version of the plugin?

[zanminkian]

@ljharb I use the latest version of eslint-plugin-import.

[seiyab]

I'm investigating it. I'm testing by TIMING=1 npx eslint ./src/test/browser/fixtures/esbuild.browser.config.mjs on https://github.com/nodejs/readable-stream.

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
import/no-duplicates |     3.102 |    15.1%
indent               |     2.234 |    10.9%
n/no-deprecated-api  |     1.241 |     6.0%
no-extra-parens      |     0.689 |     3.3%
keyword-spacing      |     0.689 |     3.3%
no-unused-vars       |     0.589 |     2.9%
comma-dangle         |     0.414 |     2.0%
key-spacing          |     0.401 |     1.9%
object-shorthand     |     0.379 |     1.8%
object-curly-newline |     0.301 |     1.5%

requireResolver() and withResolver() is suspicious. Skipping them the rule gets faster.

diff --git a/utils/resolve.js b/utils/resolve.js
index b332d2ec..7669d4ee 100644
--- a/utils/resolve.js
+++ b/utils/resolve.js
@@ -197,6 +197,7 @@ function fullResolve(modulePath, sourceFile, settings) {
   for (const pair of resolvers) {
     const name = pair[0];
     const config = pair[1];
+    continue;
     const resolver = requireResolver(name, sourceFile);
     const resolved = withResolver(resolver, config);

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
indent               |     2.074 |    12.4%
n/no-deprecated-api  |     1.047 |     6.3%
keyword-spacing      |     0.674 |     4.0%
import/no-duplicates |     0.615 |     3.7%
no-unused-vars       |     0.560 |     3.3%
no-extra-parens      |     0.515 |     3.1%
object-shorthand     |     0.343 |     2.0%
key-spacing          |     0.326 |     1.9%
comma-dangle         |     0.322 |     1.9%
space-in-parens      |     0.303 |     1.8%

[seiyab]

【FYI / NITS】 settings['import/core-modules']?.includes() ?? false should be faster than new Set() -> coreSet.has(settings['import/core-modules']). Though Set.prototype.has() costs only O(1), new Set() costs O(n). https://github.com/import-js/eslint-plugin-import/blob/67cc79841fc823ad4af2532af2dc6704e4b3b03a/utils/resolve.js#L155-L156 It shouldn't be bottleneck, though.

[ljharb]

@seiyab so the slowest one takes 3 milliseconds? that seems plenty fast enough.

[seiyab]

@ljharb yes, after posting that I've also considered the measurement didn't reproduce problem like https://github.com/import-js/eslint-plugin-import/issues/2746#issuecomment-1759205571 and https://github.com/import-js/eslint-plugin-import/issues/2746#issuecomment-2425829880. I'll look for another reproducible open project. or will use our private project.

[seiyab]

EDIT: Hide this because this doesn't look to reproduce the original issue (import/no-duplicates), neither. Performance of import/no-cycle (and no-named-as-default) looks independent one.

https://github.com/mui/material-ui

TIMING=1 pnpm run lint

Rule                                 | Time (ms) | Relative
:------------------------------------|----------:|--------:
import/no-cycle                      | 11629.303 |    15.7%
import/no-named-as-default           |  8170.978 |    11.0%
@typescript-eslint/naming-convention |  6511.814 |     8.8%
@next/next/no-html-link-for-pages    |  5490.743 |     7.4%
@typescript-eslint/no-redeclare      |  2707.642 |     3.7%
import/extensions                    |  2278.599 |     3.1%
react/default-props-match-prop-types |  2108.763 |     2.9%
react/no-unstable-nested-components  |  1746.450 |     2.4%
react/no-direct-mutation-state       |  1738.888 |     2.4%
react/prefer-stateless-function      |  1412.814 |     1.9%

user@projects/material-ui [master] % pnpm ls eslint-plugin-import eslint
Legend: production dependency, optional only, dev only

@mui/[email protected] /home/user/projects/material-ui (PRIVATE)

devDependencies:
eslint 8.57.1
eslint-plugin-import 2.31.0

[seiyab]

Using our private repository, I found mysterious & suspicious performance behavior.

with our usual setting ( import/resolver': 'webpack' as default, 'import/resolver': {'typescript': {}} for test):

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
import/no-duplicates | 21406.288 |    94.3%
import/extensions    |  1301.346 |     5.7%

with 'import/resolver': {'typescript': {}} for all files:

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
import/no-duplicates |  3216.390 |    81.0%
import/extensions    |   753.628 |    19.0%

with 'import/resolver': 'typescript' for all files:

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
import/no-duplicates |  8249.529 |    90.5%
import/extensions    |   861.413 |     9.5%

with 'import/resolver': 'webpack' for all files:

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
import/extensions    | 19207.499 |    82.5%
import/no-duplicates |  4085.457 |    17.5%

with 'import/resolver': {'webpack': {}} for all files:

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
import/extensions    | 19307.205 |    82.2%
import/no-duplicates |  4169.535 |    17.8%

• Webpack resolver looks slow
• For TypeScript resolver, object style configuration is faster (why?)

[ljharb]

What about if you add node: {} at the end of the object?

[seiyab]

With 'import/resolver': {'webpack': {}, 'node': {}}

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
import/extensions    | 19836.250 |    82.8%
import/no-duplicates |  4123.201 |    17.2%

With 'import/resolver': {'node': {}} (fails to resolve)

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
import/extensions    |  2864.346 |    63.3%
import/no-duplicates |  1663.351 |    36.7%

[seiyab]

Viewing framegraph, I found that our webpack.config.js costs a lot. new ForkTsCheckerWebpackPlugin() is dominant. Probably function style webpack config is evaluated every resolution. I'll investigate whether we can cache it.

Workaround for who uses webpack resolver + function style webpack config + fork-ts-checker-webpack-plugin: Option 1: Exclude FortkTsCheckerWebpackPlugin in ESLint Option 2: Instantiate FortkTsCheckerWebpackPlugin only once:

const ftcwp = new FortkTsCheckerWebpackPlugin()
// ...
module.exports = (_env, arg) => {
  // ..
  plugins: [
    ftcwp,
    // ...

[ljharb]

I'm confused why you'd be doing anything with typescript in webpack at all (beyond stripping types and building source maps) - bundlers should be building, not validating. Type checking is a linter like eslint, and should run separately as part of that stage of your CI.

[seiyab]

Our main type check runs on another script but we use fork-ts-checker-webpack-plugin to find problem earlier on dev server. Honestly, I'd rather remove it. Just inheriting legacy. But cant' it make sense at all? I'm not sure whether I understand you or not, I'm not an English speaker.

On the other hand, it might be a good time to remove it. I'll discuss with our team, thanks.

I'd also like to ask you whether caching function webpack config result makes sense. Other plugin can cause problem for others. Or it might improve performance even if we omit type checker.

[ljharb]

I’m not sure caching it is possible, since it’s not static, so it could change each time

[seiyab]

What is not static? You mean that multiple webpack configs can exist in single project?

Caching it worked fine for our project. With 'import/resolver': {'webpack': {}}:

Rule                 | Time (ms) | Relative
:--------------------|----------:|--------:
import/extensions    |  2709.963 |    73.1%
import/no-duplicates |   996.104 |    26.9%

My cache implementation:

const _fnCache = [];
function evaluateFunctionConfig(config, env, argv) {
  const cacheKey = { config, args: [env, argv] };
  for (let i = 0; i < _fnCache.length; i++) {
    if (config === _fnCache[i].key.config && isEqual(_fnCache[i].key.args, cacheKey.args)) {
      return _fnCache[i].value;
    }
  }
  const cached = {
    key: cacheKey,
    value: config(env, argv),
  };
  // put in front and pop last item
  if (_fnCache.unshift(cached) > MAX_CACHE) {
    _fnCache.pop();
  }
  return cached.value;
}

[ljharb]

I mean that a single webpack config can generate different results for different inputs, because it's not a static configuration.

Certainly if your own config is deterministic then it can be cached, but there's no way for this plugin to be sure of that.

[seiyab]

Different results for different inputs shouldn't be a matter as we include arguments as cache key. Does deterministic mean same results for same inputs? I can't imagine meaningful config with different results for same inputs. Do I misunderstand?

If you mind compatibility, can I implement the cache feature under cache: true config? Or if you also worry about complexity and maintainability, I will give up to change eslint-import-resolver-webpack and will employ a workaround or publish my own cached resolver.

[ljharb]

If it's opt-in for the webpack resolver, then sure, that sounds great to include.