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

no-cycle performance in 2.30 is even worse than 2.27.5 #3060

Open
AnnatarHe opened this issue Sep 14, 2024 · 16 comments
Open

no-cycle performance in 2.30 is even worse than 2.27.5 #3060

AnnatarHe opened this issue Sep 14, 2024 · 16 comments

Comments

@AnnatarHe
Copy link

AnnatarHe commented Sep 14, 2024

Hey, Is there any benchmark for the new SCC algorithm?

In a project I am maintaining, 2x more time is used compared with 2.27.5.
it has 544 files project(in monorepo).

Version 2.27.5 2.30.0
Time ~40s ~80s

2.30.0

Rule                                            | Time (ms) | Relative
:-----------------------------------------------|----------:|--------:
import/no-cycle                                 | 58328.676 |    79.2%

2.27.5

Rule                                            | Time (ms) | Relative
:-----------------------------------------------|----------:|--------:
import/no-cycle                                 | 15741.654 |    45.8%
@ljharb
Copy link
Member

ljharb commented Sep 14, 2024

It depends on the size of the codebase and a number of other factors - try setting the disableScc option to true?

@AnnatarHe
Copy link
Author

AnnatarHe commented Sep 15, 2024

@ljharb
if set disableScc as true, we are back to 2.27.5. the time is around 40s 😅

this project has around 500 files and 156k lines of code.

Rule                                            | Time (ms) | Relative
:-----------------------------------------------|----------:|--------:
import/no-cycle                                 |  5751.410 |    20.6%
===============================================================================
 Language            Files        Lines         Code     Comments       Blanks
===============================================================================
 JavaScript              5           86           69            6           11
 TypeScript            550       169737       156696         6472         6569
===============================================================================
 Total                 555       169823       156765         6478         6580
===============================================================================

@AnnatarHe
Copy link
Author

oh, I just found another similar issue. #3047

@ljharb
Copy link
Member

ljharb commented Sep 15, 2024

cc @soryy708

@AnnatarHe
Copy link
Author

btw, in another project that has more than 7800 files. a 16 cores 32 GB memory server cannot finish in 1 hour, and the node(lint) process took almost 8G memory

@soryy708
Copy link
Contributor

I had a project where SCC clearly made a significant good impact:

#2998 (comment)

With 7k files, and almost 400kloc,

So what is it about your project's configuration that makes it so slow and resource-intensive?

@AnnatarHe
Copy link
Author

AnnatarHe commented Sep 16, 2024

I posted a result when setting disableScc to true, so I think it's clear that SCC made the performance issue.

And this comment might be the reason: un-ts#113 (comment)

Our project is pretty huge, with over 2 million lines of code... it's quite hard to put it here.

eslint version is 8.39.0

@soryy708
Copy link
Contributor

soryy708 commented Sep 16, 2024

The linked comment is in eslint-plugin-import-x which is a community fork. If you want them to make a change to their library, please bring it up on their repository.

I doubt that the impact of their proposed replacing of @rtsao/scc with @newdash/graphlib will make much of a difference on performance. Running Tarjan-SCC pre-processing, whether with that library or with another library, will add some work to do so require a performance hit for small projects.

As for your 2mloc project, is that the same that has only 7800 files?
Are you sure the issue is with the choice of SCC library, as opposed to some other step like parsing or ExportMap construction?
Would love to see a profiler output (e.g. flamegraph). How much time are we spending in Tarjan-SCC and how much in other steps?

@soryy708
Copy link
Contributor

It depends on the size of the codebase and a number of other factors - try setting the disableScc option to true?

Created an issue to add disableScc to the docs:

@AnnatarHe
Copy link
Author

our project is a mono repo, including many projects. the whole repo is over 2mloc. and one of the largest one is over 7800 files. I'm using a smaller project to test(544 files, the profiling file generated from this project)

here is the profiling file. and using https://www.speedscope.app/ (please let me know once you downloaded, I'll delete it): isolate-0x51a8150-1444136-v8.log.json

image

@soryy708
Copy link
Contributor

Why are we spending so much time in SCC? It's a linear algorithm. Are we running it many times?
You said mono repo, so I suspect maybe it has something to do with the way ExportMap handles externals?
The projects I tested on didn't have many, so maybe there's a pathology when a monorepo has many eslint/ts configs?

@soryy708
Copy link
Contributor

#3068

@soryy708
Copy link
Contributor

soryy708 commented Oct 7, 2024

@AnnatarHe Please try version 2.31.0, it includes the fix from #3068.
Is performance better with/without disableScc?

@AnnatarHe
Copy link
Author

@soryy708 no significant difference. so it looks like the new version brings the performance back to previous version

disableScc: true

Rule                                            |  Time (ms) | Relative
:-----------------------------------------------|-----------:|--------:
import/no-cycle                                 | 110489.719 |    90.2%
prettier/prettier                               |   9351.288 |     7.6%
import/namespace                                |    834.045 |     0.7%
unused-imports/no-unused-imports                |    531.654 |     0.4%


Executed in  137.36 secs    fish           external
   usr time  163.09 secs  306.00 micros  163.09 secs
   sys time   11.42 secs  173.00 micros   11.42 secs

disableScc: false

Rule                                            |  Time (ms) | Relative
:-----------------------------------------------|-----------:|--------:
import/no-cycle                                 | 116099.013 |    90.8%
prettier/prettier                               |   8842.955 |     6.9%
import/namespace                                |    924.044 |     0.7%
unused-imports/no-unused-imports                |    561.242 |     0.4%

Executed in  143.02 secs    fish           external
   usr time  171.97 secs  327.00 micros  171.97 secs
   sys time   11.48 secs  184.00 micros   11.48 secs

@soryy708
Copy link
Contributor

soryy708 commented Oct 7, 2024

@AnnatarHe pardon I think I don't follow, which one of the multiple projects mentioned was this recent benchmark in?
Also if we're talking specifically about no-cycle, lets isolate the variables to just that, by disabling all other lint rules, so that we get more repeatable results.

When you say

the new version brings the performance back to previous version

which version do you mean?
did performance return to the same as you had in 2.27.5?

@zernie
Copy link

zernie commented Oct 7, 2024

disable-scss brought no-cycle's percentage from 85% down to 28%. Thanks!

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

No branches or pull requests

4 participants