Even larger `import/no-cycle` performance downgrade in 2.30.0

[jzaefferer]

#2348 was supposed to get fixed in 2.30.0, but for my project the performance of import/no-cycle got even worse.

With 2.29.1 it took 21s:

Rule                                    | Time (ms) | Relative
:---------------------------------------|----------:|--------:
import/no-cycle                         | 21503.212 |    52.9%

With 2.30.0 it took 67s, 3.2 times slower:

Rule                                    | Time (ms) | Relative
:---------------------------------------|----------:|--------:
import/no-cycle                         | 67440.935 |    76.8%

I also tried 'import/no-cycle': [2, { ignoreExternal: false, maxDepth: 3 }],, which was suggested in the previous ticket, but that made no performance difference. It did find more errors though.

Are there other measurements I can share? Like a cpu profile? I'm not sure what's useful here and didn't find good clues in #2348.

[diegodorado]

Same for me: it is worse now. Went from

Rule                                 | Time (ms) | Relative
:------------------------------------|----------:|--------:
import/no-cycle                      |  5091.198 |    31.2%

to

Rule                                 | Time (ms) | Relative
:------------------------------------|----------:|--------:
import/no-cycle                      |  6829.761 |    39.1%

[ljharb]

What happens if you set the disableScc option to true? Does it restore the v2.29 speed?

cc @soryy708

[jzaefferer]

@ljharb yep, it does

[soryy708]

Cool, glad we decided to put disableScc in.

The SCC is a pre-processing we do to speed up no-cycle to enable early-exit (short-circuiting). In large projects with many files to lint, it prunes a lot of unnecessary work, but in small projects the preprocessing can take longer than the time it saves.

[ljharb]

Is there any way we could figure out a heuristic, and dynamically figure out whether it’ll be faster to use scc or not?

[soryy708]

Is there any way we could figure out a heuristic, and dynamically figure out whether it’ll be faster to use scc or not?

I don't have anything in mind. It depends on a lot of things - the user's hardware, cpu, how fast their hdd/ssd is...

If the heuristic will be right then great, the user won't need to configure anything.

But what if the heuristic will be wrong? They'll need to override the heuristic's decision, either to enable or to disable SCC.

[ljharb]

That’s already the case - we just chose a heuristic of “always use scc”. The hope would be do minimize how many users have to enable or disable scc explicitly.

[soryy708]

IMO it's a good heuristic because:

• If your project is small, lint takes seconds, so you don't care about performance as much, don't mind the overhead
• If your project is large, lint takes minutes, so you care about performance, and want more efficiency

Defaults that cater to large codebases (perhaps at the expense of tiny ones) are good, while there is an escape hatch in case the user cares enough.

[ljharb]

I totally agree that it's the right heuristic :-) i'm just wondering if we can fine-tune it a bit for projects in the 20-70s range.

[soryy708]

What happens if you set the disableScc option to true? Does it restore the v2.29 speed?

cc @soryy708

Created an issue to add disableScc to the docs:

• https://github.com/import-js/eslint-plugin-import/issues/3063

[soryy708]

https://github.com/import-js/eslint-plugin-import/pull/3068

[soryy708]

@jzaefferer @diegodorado Please try version 2.31.0, it includes the fix from https://github.com/import-js/eslint-plugin-import/pull/3068. Is performance better with/without disableScc?

[jzaefferer]

With 2.31.0, the performance is much better with the default scc on (no disableScc used)

disableScc true: 

1 import/no-cycle                         | 28912.195 |    72.0%
2 import/no-cycle                         | 29854.613 |    71.6%
3 import/no-cycle                         | 28827.838 |    71.6%

default scc

1 import/no-cycle                         | 12021.782 |    51.5%
2 import/no-cycle                         | 11614.811 |    50.7%
3 import/no-cycle                         | 12250.667 |    50.3%

Very nice! Since I created this ticket, I'll take the liberty to also close it 💃

[TkDodo]

Sadly, it didn’t fix the issue for us. After upgrading, the total lint time went from 1m36s to 2m54s on my machine (linting over multiple packages in a monorepo).

Here are the specific timings for one example package with our settings:

'import/no-cycle': [
    'error',
    {
        maxDepth: 3,
        allowUnsafeDynamicCyclicDependency: true,
    },
],

v2.31.0, disableScc: false (default)

Rule                                    | Time (ms) | Relative
:---------------------------------------|----------:|--------:
import/no-cycle                         | 27556.928 |    70.6%

and with disableScc: true:

import/no-cycle                         | 11824.795 |    51.2%

on v2.30.0, I’m getting the following results:

v2.30.0, disableScc: false (default)

import/no-cycle                         |  4719.848 |    30.0%

and with disableScc: true:

import/no-cycle                         |  2156.667 |    15.5%

So, all settings in v31 perform worse than v30.

[ljharb]

What happens if you remove maxDepth entirely?

[TkDodo]

What happens if you remove maxDepth entirely?

for v30, it becomes a bit faster (with disableScc: true)

import/no-cycle                         |  2144.215 |    15.5%

for v31, still not really better

disableScc: false

import/no-cycle                         | 26919.054 |    70.4%

disableScc: true:

import/no-cycle                         | 27324.862 |    71.0%