no-absolute-path is incredibly slow

[feross]

Hey! Great package you have here. We just started using it in StandardJS!

One of our team members, @Flet, was doing some performance testing and noticed that the import/no-absolute-path rule is the slowest rule in our entire set of ESLint rules, by far. Check it out:

$ TIMING=1 standard                                                                                                                                         master
Rule                         | Time (ms) | Relative
:----------------------------|----------:|--------:
import/no-absolute-path      |    27.906 |    24.3%
indent                       |     7.070 |     6.2%
no-extra-parens              |     4.939 |     4.3%
comma-dangle                 |     4.313 |     3.8%
no-unused-vars               |     3.695 |     3.2%
no-regex-spaces              |     2.834 |     2.5%
space-in-parens              |     2.611 |     2.3%
no-useless-return            |     2.361 |     2.1%
no-unmodified-loop-condition |     2.310 |     2.0%
block-spacing                |     2.126 |     1.9%

There might be a good reason for this, but I just wanted to check if this is expected behavior? We're probably going to have to disable the rule for now, since it's not worth the performance cost.

Is there any way to make it faster?

[jseminck]

Hi. I have a possible explanation for your question. Please keep in mind that I am not very familiar with the code base, but I believe my explanation makes sense.

This specific rule uses a common shared helper to determine whether the path is absolute or not. This generic shared helper also resolves every import to determine "the full module filesystem path".

Now strictly speaking, for no-absolute-path rule, this resolving is not needed. Thus it would be possible to re-write this rule and skip the resolving, and the rule will run faster.

However, the output of resolve() is cached. So if you have also other rules that also call resolve() then they will be affected:

• If another rule that already has called resolve() runs before this rule, then that rule would become slow, and this rule would become faster.
• if other rules run after this rule that call resolve() then those rules will run faster because the data is already cached by no-absolute-path.

From my own project, if I only enable no-absolute-path:

[1] Rule                           | Time (ms) | Relative
[1] :------------------------------|----------:|--------:
[1] import/no-absolute-path        | 10621.188 |    40.6%
[1] react/no-string-refs           |  1448.301 |     5.5%

If I enable some other rules, which run before no-absolute-path, then that rule becomes the slowest:

[1] Rule                              | Time (ms) | Relative
[1] :---------------------------------|----------:|--------:
[1] import/namespace                  | 13924.929 |    44.3%
[1] react/no-string-refs              |  1340.541 |     4.3%

So in short, if you only use this rule from eslint-plugin-import then indeed it runs much slower than it should. If you use multiple rules, then this rule is most likely making other rules run faster due to it already caches some data. Note: I did see that you use some other rules as well - I guess none of those rules are performing resolve(). I do not yet know how often this is called amongst the different rules.

It should of course also be possible to refactor no-absolute-path to not perform the resolve() calls at all. I will make a PR with this change, and then the maintainers can review it and see if they want to add it in or not.

[benmosher]

@feross that is exciting!

edit: everything I say below is true, but totally irrelevant for this rule. not sure why it's so slow, I need to look at #843

...but in general @jseminck is right, a bunch of dependency parsing has to happen internally and generally that means one rule looks really slow, but really it's just being evaluated first and the ASTs/export maps for dependencies are cached for the subsequently evaluated rules to use.

The memo-parser exists to help this a little bit: it can share ASTs with ESLint's parse so any given file is parsed only once, instead of twice*. I'm not familiar with Standard's architecture, but if you're able to wrap a configured parser with the memo parser, you could save a lot of time.

*if it's not a dependency or only a dependency (not being linted), it's only parsed once

[benmosher]

after reviewing #843, looks like this rule was cloned from another rule and is indeed doing way more work than is necessary. I would expect @jseminck's PR to help substantially. 🚀

[benmosher]

merged and published as 2.7.0. thanks @jseminck for diagnosing and making the PR!

[feross]

Awesome, just saw this. Thanks for diagnosing, PRing, and merging!