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

Performance regression in 8.0.0 #268

Closed
papandreou opened this issue Aug 19, 2020 · 8 comments · Fixed by #280
Closed

Performance regression in 8.0.0 #268

papandreou opened this issue Aug 19, 2020 · 8 comments · Fixed by #280

Comments

@papandreou
Copy link
Contributor

When I upgraded from 7.0.1 to 8.0.0, linting my project started taking 2.5x as long as before.

git bisect says:

6fe21e6e96f479d63055cda50d341b1142f7b8ec is the first bad commit
commit 6fe21e6e96f479d63055cda50d341b1142f7b8ec
Author: Mathias Schreck <[email protected]>
Date:   Mon Aug 3 13:38:11 2020 +0200

    Improve data structure of custom names

 README.md                              |  24 +----
 lib/rules/no-exclusive-tests.js        |   2 +-
 lib/rules/no-skipped-tests.js          |   2 +-
 lib/util/ast.js                        |  14 +--
 lib/util/names.js                      | 164 +++++++++++++++++++++------------
 lib/util/settings.js                   |  24 +----
 test/rules/max-top-level-suites.js     |   8 +-
 test/rules/no-async-describe.js        |   2 +-
 test/rules/no-exclusive-tests.js       |  22 ++---
 test/rules/no-hooks-for-single-case.js |   8 +-
 test/rules/no-identical-title.js       |   8 +-
 test/rules/no-nested-tests.js          |   8 +-
 test/rules/no-setup-in-describe.js     |  26 ++----
 test/rules/no-sibling-hooks.js         |   8 +-
 test/rules/no-skipped-tests.js         |  23 +++--
 test/rules/no-top-level-hooks.js       |   4 +-
 test/rules/prefer-arrow-callback.js    |   1 -
 test/util/namesSpec.js                 | 151 ++++++++++++++----------------
 18 files changed, 244 insertions(+), 255 deletions(-)
@danielrentz
Copy link

danielrentz commented Aug 20, 2020

I have noticed the same issue. In our project we have 550 test files. The time needed linting them has dramatically increased:

ESLint without mocha plugin: 01m04s
ESLint with mocha plugin 7.0.1: 02m22s
ESLint with mocha plugin 8.0.0: 13m16s

@lo1tuma
Copy link
Owner

lo1tuma commented Aug 20, 2020

Thanks for reporting this. My initial assumption is that the refactoring of astUtils is causing this, because now they are wrapped in a function an called for every rule and every linted file.
I was planning to rewrite astUtils anyway to address a couple of other issues, so I hope that I can improve the performance along the way.
Anyway the first step should be implementing a performance regression test, that will fail when the performance decreases.

@marneborn
Copy link

marneborn commented Sep 28, 2020

Just to add some details from my runs:
[email protected]

Rule                              | Time (ms) | Relative
:---------------------------------|----------:|--------:
mocha/no-nested-tests             | 33516.285 |    19.0%
mocha/no-identical-title          | 30261.398 |    17.2%
mocha/no-exclusive-tests          | 21381.243 |    12.1%
mocha/no-mocha-arrows             | 19715.057 |    11.2%
mocha/max-top-level-suites        | 12042.615 |     6.8%
mocha/no-top-level-hooks          | 11268.808 |     6.4%
mocha/no-pending-tests            | 11251.553 |     6.4%
mocha/no-global-tests             | 11175.641 |     6.3%
indent                            |  7998.558 |     4.5%
import/no-extraneous-dependencies |  1599.963 |     0.9%
✨  Done in 193.51s.

[email protected]

Rule                              | Time (ms) | Relative
:---------------------------------|----------:|--------:
indent                            |  6336.102 |    30.6%
mocha/no-mocha-arrows             |  2957.594 |    14.3%
import/no-extraneous-dependencies |   906.180 |     4.4%
mocha/no-nested-tests             |   585.623 |     2.8%
mocha/no-identical-title          |   460.823 |     2.2%
no-empty-function                 |   398.166 |     1.9%
no-whitespace-before-property     |   306.908 |     1.5%
mocha/max-top-level-suites        |   283.220 |     1.4%
import/no-unresolved              |   274.460 |     1.3%
key-spacing                       |   259.182 |     1.3%
✨  Done in 31.66s.


@AriPerkkio
Copy link

I'm also seeing these performance issues. I'm testing stability of some well-known ESLint community plugins and eslint-plugin-mocha is really slow compared to any other ESLint plugin. Here are some results of running the plugin against ~1800 repositories:

  • @typescript-eslint: 2h 2m 49s
  • eslint core with javascript: 2h 42m 26s
  • eslint core with typescript: 2h 57m 35s
  • eslint-plugin-cypress: 1h 18m 19s
  • eslint-plugin-import: 1h 50m 9s
  • eslint-plugin-jest-dom: 1h 20m 8s
  • eslint-plugin-jest: 1h 34m 28s
  • eslint-plugin-jsx-a11y: 1h 16m 22s
  • eslint-plugin-mocha: 6h 1m 5s
  • eslint-plugin-node: 1h 27m 14s
  • eslint-plugin-react-hooks: 1h 11m 38s
  • eslint-plugin-react-redux: 1h 9m 58s
  • eslint-plugin-react: 2h 59m 27s
  • eslint-plugin-sonarjs: 1h 20m 8s
  • eslint-plugin-testing-library: 1h 14m 55s
  • eslint-plugin-unicorn: 2h 13m 56s

CI runs of eslint-plugin-mocha timeout after 6 hours. At this point ~1600 repositories have been processed. https://github.com/AriPerkkio/eslint-remote-tester/actions/runs/415815724

[INFO/STATUS] Repositories (1635/1834)
"extends": ["plugin:mocha/recommended"]

@serut
Copy link

serut commented Mar 24, 2021

Any updates on this issue ?

@lo1tuma
Copy link
Owner

lo1tuma commented Mar 24, 2021

@serut as mentioned in the release notes, with version 8.1.0 the rules no-exclusive-tests and no-pending-tests have been improved regarding performance.

@jgeurts
Copy link

jgeurts commented Mar 24, 2021

@lo1tuma do you have any benchmarks for how those rules in 8.1 perform compared to how they perform in 7.x? I missed those, if they were available

@serut
Copy link

serut commented Mar 24, 2021

7.0.0

> TIMING=1 eslint --fix web_modules src --ext .js,.jsx

Rule                                | Time (ms) | Relative
:-----------------------------------|----------:|--------:
import/no-cycle                     | 11627.627 |    15.9%
indent                              |  6203.095 |     8.5%
import/no-self-import               |  3687.194 |     5.0%
react/no-deprecated                 |  2513.409 |     3.4%
react/state-in-constructor          |  2436.697 |     3.3%
react/no-string-refs                |  1970.265 |     2.7%
react/sort-comp                     |  1845.995 |     2.5%
react/no-direct-mutation-state      |  1833.409 |     2.5%
react/void-dom-elements-no-children |  1833.332 |     2.5%
react/no-typos                      |  1518.538 |     2.1%

real    1m52,146s
user    2m17,300s
sys     0m1,445s

8.0.0

Rule                       | Time (ms) | Relative
:--------------------------|----------:|--------:
mocha/no-nested-tests      | 14804.749 |    10.8%
import/no-cycle            | 11263.642 |     8.2%
mocha/no-identical-title   | 10832.939 |     7.9%
mocha/no-exclusive-tests   |  9639.595 |     7.0%
mocha/no-skipped-tests     |  9052.927 |     6.6%
indent                     |  5792.707 |     4.2%
mocha/no-global-tests      |  4821.304 |     3.5%
mocha/no-top-level-hooks   |  4802.673 |     3.5%
mocha/max-top-level-suites |  4783.477 |     3.5%
mocha/no-sibling-hooks     |  4762.750 |     3.5%

real    2m55,496s
user    3m22,943s
sys     0m1,952s

8.1.0

Rule                       | Time (ms) | Relative
:--------------------------|----------:|--------:
mocha/no-nested-tests      | 14666.830 |    11.7%
import/no-cycle            | 11143.362 |     8.9%
mocha/no-identical-title   | 10739.450 |     8.6%
mocha/no-skipped-tests     |  9044.689 |     7.2%
indent                     |  5525.793 |     4.4%
mocha/max-top-level-suites |  5187.068 |     4.2%
mocha/no-top-level-hooks   |  4792.140 |     3.8%
mocha/no-global-tests      |  4781.966 |     3.8%
mocha/no-sibling-hooks     |  4773.527 |     3.8%
import/no-self-import      |  3479.034 |     2.8%

real    2m43,085s
user    3m9,872s
sys     0m1,894s

ErisDS added a commit to TryGhost/eslint-plugin-ghost that referenced this issue May 7, 2021
refs: lo1tuma/eslint-plugin-mocha#268

- eslint-plugin-mocha v8 has a huge performance issue
- it makes linting out tests take more than a minute!
- reverting to v7 fixes this, and as far as I can tell does not remove any critical features or fixes that we rely upon
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants