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

Chore: lazy loading for rules #11705

Merged
merged 2 commits into from
May 11, 2019
Merged

Chore: lazy loading for rules #11705

merged 2 commits into from
May 11, 2019

Conversation

mysticatea
Copy link
Member

What is the purpose of this pull request? (put an "X" next to item)

[X] Other, please explain: improve performance.

What changes did you make? (Give an overview)

This PR improves performance by avoiding to load unused rules.
But this PR keeps static linking for bundlers can recognize rules.

  • lib/util/lazy-loading-rule-map.js ... The map of built-in rules. This map will load each rule at the first access.
  • lib/cli.js ... This file will iterate all rules at the first access of rulesMeta.
  • lib/testers/rule-tester.js ... This file no longer iterates all rules.
  • conf/eslint-recommended.js ... This file no longer iterates all rules.
  • Makefile.js ... The test verifies if conf/eslint-recommended.js is correct.

Impacts:

eslint lib/linter.js --no-eslintrc --config conf/eslint-recommended.js --parser-options ecmaVersion:2018
# 0.73s → 0.56s (23% speed up)

eslint lib/linter.js
# 1.25s → 1.20s (4% speed up)

mocha "tests/{bin,lib,tools}/**/*.js" --reporter progress
# 52s → 49s (5% speed up)

Especially, it has a lerge impact if it used fewer rules such as eslint:recommended.


npm run perf:

# BEFORE -----------------------------------------------------------------------
Loading:
  Load performance Run #1:  421.4627ms
  Load performance Run #2:  449.755299ms
  Load performance Run #4:  460.1405ms
  Load performance Run #5:  453.075ms
  Load Performance median:  453.075ms


Single File:
  CPU Speed is 3192 with multiplier 13000000
  Performance Run #1:  3998.2129ms
  Performance Run #2:  3855.885199ms
  Performance Run #3:  3704.6185ms
  Performance Run #4:  3810.938001ms
  Performance Run #5:  3731.5751ms

  Performance budget ok:  3810.938001ms (limit: 4072.6817042606517ms)


Multi Files (0 files):
  CPU Speed is 3192 with multiplier 39000000
  Performance Run #1:  11271.758299ms
  Performance Run #2:  10577.971301ms
  Performance Run #3:  10542.4316ms
  Performance Run #4:  10947.606701ms
  Performance Run #5:  10122.6511ms

  Performance budget ok:  10577.971301ms (limit: 12218.045112781954ms)

# AFTER ------------------------------------------------------------------------
Loading:
  Load performance Run #1:  220.293599ms
  Load performance Run #2:  215.602099ms
  Load performance Run #3:  210.9296ms
  Load performance Run #4:  222.679ms
  Load performance Run #5:  217.569701ms

  Load Performance median:  217.569701ms


Single File:
  CPU Speed is 3192 with multiplier 13000000
  Performance Run #1:  3839.186ms
  Performance Run #2:  3770.319299ms
  Performance Run #3:  3685.686001ms
  Performance Run #4:  3779.8463ms
  Performance Run #5:  3650.4698ms

  Performance budget ok:  3770.319299ms (limit: 4072.6817042606517ms)


Multi Files (0 files):
  CPU Speed is 3192 with multiplier 39000000
  Performance Run #1:  10705.549801ms
  Performance Run #2:  10394.445ms
  Performance Run #3:  10542.7665ms
  Performance Run #4:  10355.708099ms
  Performance Run #5:  10419.626301ms

  Performance budget ok:  10419.626301ms (limit: 12218.045112781954ms)
  • Load performance 52% speed up.
  • Single File 4% speed up.
  • Multi Files 1% speed up.

Is there anything you'd like reviewers to focus on?

Can we go to this direction?

@mysticatea mysticatea added core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion chore This change is not user-facing labels May 10, 2019
@not-an-aardvark not-an-aardvark merged commit f42d0af into master May 11, 2019
@mysticatea mysticatea deleted the lazy-loading branch May 11, 2019 03:06
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Nov 8, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Nov 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion chore This change is not user-facing core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants