Skip to content

perf: Fix caching in config loaders#19042

Merged
nzakas merged 9 commits intomainfrom
issue19025-1
Oct 28, 2024
Merged

perf: Fix caching in config loaders#19042
nzakas merged 9 commits intomainfrom
issue19025-1

Conversation

@mdjermanovic
Copy link
Copy Markdown
Member

@mdjermanovic mdjermanovic commented Oct 22, 2024 •
edited
Loading

Prerequisites checklist

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

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:

Fixes #19025

What changes did you make? (Give an overview)

In async code like this:

eslint/lib/config/config-loader.js

Lines 624 to 637 in 7259627

async #calculateConfigArray(configFilePath, basePath) {
// check for cached version first
if (this.#configArray) {
return this.#configArray;
}
const configs = await calculateConfigArray(configFilePath, basePath, this.#options);
// cache the config array for this instance
this.#configArray = configs;
return configs;
}

it can happen that multiple calls of #calculateConfigArray() have cache miss and all end up calling calculateConfigArray(). In the use case described in #19025 (comment), I believe that for each file being linted ESLint searches for a config file again and calculates config array again.

I updated LegacyConfigLoader and ConfigLoader to cache promises for the config file path and the config array, instead of their values. That should ensure that the config file lookup and calculating config array are run only once per instance. I added a test case that is failing without this change (in the main branch, it performs config lookup and creates a config array twice, once for each file).

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

@eslint-github-bot eslint-github-bot Bot added the chore This change is not user-facing label Oct 22, 2024
@github-actions github-actions Bot added the core Relates to ESLint's core APIs and features label Oct 22, 2024
@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 22, 2024 •
edited
Loading

✅ Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit 8b1481e
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/671cd4e8a1867b0008300169

@mdjermanovic mdjermanovic mentioned this pull request Oct 22, 2024
Closed
1 task
@mdjermanovic
Copy link
Copy Markdown
Member Author

mdjermanovic commented Oct 23, 2024

With the following test:

Details 
// test.mjs

import api from "eslint";

const { ESLint } = api;

const eslint = new ESLint();

const files = [
  ".c8rc",
  ".codeclimate.yml",
  ".github/ISSUE_TEMPLATE/bug-report.yml",
  ".github/ISSUE_TEMPLATE/change.yml",
  ".github/ISSUE_TEMPLATE/config.yml",
  ".github/ISSUE_TEMPLATE/docs.yml",
  ".github/ISSUE_TEMPLATE/new-rule.yml",
  ".github/ISSUE_TEMPLATE/new-syntax.yml",
  ".github/ISSUE_TEMPLATE/rule-change.yml",
  ".github/dependabot.yml",
  ".github/labeler.yml",
  ".github/renovate.json5",
  ".github/workflows/annotate_pr.yaml",
  ".github/workflows/ci.yml",
  ".github/workflows/codeql-analysis.yml",
  ".github/workflows/docs-ci.yml",
  ".github/workflows/pr-labeler.yml",
  ".github/workflows/rebuild-docs-sites.yml",
  ".github/workflows/stale.yml",
  ".github/workflows/types-integration.yml",
  ".github/workflows/update-readme.yml",
  ".markdownlint.yml",
  ".pre-commit-hooks.yaml",
  ".trunk/configs/svgo.config.js",
  ".trunk/trunk.yaml",
  "Makefile.js",
  "bin/eslint.js",
  "conf/default-cli-options.js",
  "conf/ecma-version.js",
  "conf/globals.js",
  "conf/replacements.json",
  "conf/rule-type-list.json",
  "docs/.eleventy.js",
  "docs/.stylelintrc.json",
  "docs/package.json",
  "docs/postcss.config.js",
  "docs/src/_data/config.json",
  "docs/src/_data/eslintVersions.js",
  "docs/src/_data/flags.js",
  "docs/src/_data/further_reading_links.json",
  "docs/src/_data/helpers.js",
  "docs/src/_data/languages.json",
  "docs/src/_data/layout.js",
  "docs/src/_data/links.json",
  "docs/src/_data/navigation.json",
  "docs/src/_data/rule_versions.json",
  "docs/src/_data/rules.json",
  "docs/src/_data/rules_categories.js",
  "docs/src/_data/rules_meta.json",
  "docs/src/_data/site.js",
  "docs/src/_data/sites/en.yml",
  "docs/src/_data/sites/zh-hans.yml",
  "docs/src/_data/versions.json",
  "docs/tools/code-block-utils.js",
  "docs/tools/markdown-it-rule-example.js",
  "docs/tools/prism-eslint-hook.js",
  "docs/tools/validate-links.js",
  "eslint.config.js",
  "knip.jsonc",
  "lib/api.js",
  "lib/cli-engine/cli-engine.js",
  "lib/cli-engine/file-enumerator.js",
  "lib/cli-engine/formatters/formatters-meta.json",
  "lib/cli-engine/formatters/html.js",
  "lib/cli-engine/formatters/json-with-metadata.js",
  "lib/cli-engine/formatters/json.js",
  "lib/cli-engine/formatters/stylish.js",
  "lib/cli-engine/hash.js",
  "lib/cli-engine/index.js",
  "lib/cli-engine/lint-result-cache.js",
  "lib/cli-engine/load-rules.js",
  "lib/cli.js",
  "lib/config/config-loader.js",
  "lib/config/config.js",
  "lib/config/default-config.js",
  "lib/config/flat-config-array.js",
  "lib/config/flat-config-helpers.js",
  "lib/config/flat-config-schema.js",
  "lib/config/rule-validator.js",
  "lib/eslint/eslint-helpers.js",
  "lib/eslint/eslint.js",
  "lib/eslint/index.js",
  "lib/eslint/legacy-eslint.js",
  "lib/languages/js/index.js",
  "lib/languages/js/source-code/index.js",
  "lib/languages/js/source-code/source-code.js",
  "lib/languages/js/source-code/token-store/backward-token-comment-cursor.js",
  "lib/languages/js/source-code/token-store/backward-token-cursor.js",
  "lib/languages/js/source-code/token-store/cursor.js",
  "lib/languages/js/source-code/token-store/cursors.js",
  "lib/languages/js/source-code/token-store/decorative-cursor.js",
  "lib/languages/js/source-code/token-store/filter-cursor.js",
  "lib/languages/js/source-code/token-store/forward-token-comment-cursor.js",
  "lib/languages/js/source-code/token-store/forward-token-cursor.js",
  "lib/languages/js/source-code/token-store/index.js",
  "lib/languages/js/source-code/token-store/limit-cursor.js",
  "lib/languages/js/source-code/token-store/padded-token-cursor.js",
  "lib/languages/js/source-code/token-store/skip-cursor.js",
  "lib/languages/js/source-code/token-store/utils.js",
  "lib/languages/js/validate-language-options.js",
  "lib/linter/apply-disable-directives.js",
  "lib/linter/code-path-analysis/code-path-analyzer.js",
  "lib/linter/code-path-analysis/code-path-segment.js",
  "lib/linter/code-path-analysis/code-path-state.js",
  "lib/linter/code-path-analysis/code-path.js",
  "lib/linter/code-path-analysis/debug-helpers.js",
  "lib/linter/code-path-analysis/fork-context.js",
  "lib/linter/code-path-analysis/id-generator.js",
  "lib/linter/file-context.js",
  "lib/linter/index.js",
  "lib/linter/interpolate.js",
  "lib/linter/linter.js",
  "lib/linter/node-event-generator.js",
  "lib/linter/report-translator.js",
  "lib/linter/rule-fixer.js",
  "lib/linter/rules.js",
  "lib/linter/safe-emitter.js",
  "lib/linter/source-code-fixer.js",
  "lib/linter/timing.js",
  "lib/linter/vfile.js",
  "lib/options.js",
  "lib/rule-tester/index.js",
  "lib/rule-tester/rule-tester.js",
  "lib/rules/accessor-pairs.js",
  "lib/rules/array-bracket-newline.js",
  "lib/rules/array-bracket-spacing.js",
  "lib/rules/array-callback-return.js",
  "lib/rules/array-element-newline.js",
  "lib/rules/arrow-body-style.js",
  "lib/rules/arrow-parens.js",
  "lib/rules/arrow-spacing.js",
  "lib/rules/block-scoped-var.js",
  "lib/rules/block-spacing.js",
  "lib/rules/brace-style.js",
  "lib/rules/callback-return.js",
  "lib/rules/camelcase.js",
  "lib/rules/capitalized-comments.js",
  "lib/rules/class-methods-use-this.js",
  "lib/rules/comma-dangle.js",
  "lib/rules/comma-spacing.js",
  "lib/rules/comma-style.js",
  "lib/rules/complexity.js",
  "lib/rules/computed-property-spacing.js",
  "lib/rules/consistent-return.js",
  "lib/rules/consistent-this.js",
  "lib/rules/constructor-super.js",
  "lib/rules/curly.js",
  "lib/rules/default-case-last.js",
  "lib/rules/default-case.js",
  "lib/rules/default-param-last.js",
  "lib/rules/dot-location.js",
  "lib/rules/dot-notation.js",
  "lib/rules/eol-last.js",
  "lib/rules/eqeqeq.js",
  "lib/rules/for-direction.js",
  "lib/rules/func-call-spacing.js",
  "lib/rules/func-name-matching.js",
  "lib/rules/func-names.js",
  "lib/rules/func-style.js",
  "lib/rules/function-call-argument-newline.js",
  "lib/rules/function-paren-newline.js",
  "lib/rules/generator-star-spacing.js",
  "lib/rules/getter-return.js",
  "lib/rules/global-require.js",
  "lib/rules/grouped-accessor-pairs.js",
  "lib/rules/guard-for-in.js",
  "lib/rules/handle-callback-err.js",
  "lib/rules/id-blacklist.js",
  "lib/rules/id-denylist.js",
  "lib/rules/id-length.js",
  "lib/rules/id-match.js",
  "lib/rules/implicit-arrow-linebreak.js",
  "lib/rules/indent-legacy.js",
  "lib/rules/indent.js",
  "lib/rules/index.js",
  "lib/rules/init-declarations.js",
  "lib/rules/jsx-quotes.js",
  "lib/rules/key-spacing.js",
  "lib/rules/keyword-spacing.js",
  "lib/rules/line-comment-position.js",
  "lib/rules/linebreak-style.js",
  "lib/rules/lines-around-comment.js",
  "lib/rules/lines-around-directive.js",
  "lib/rules/lines-between-class-members.js",
  "lib/rules/logical-assignment-operators.js",
  "lib/rules/max-classes-per-file.js",
  "lib/rules/max-depth.js",
  "lib/rules/max-len.js",
  "lib/rules/max-lines-per-function.js",
  "lib/rules/max-lines.js",
  "lib/rules/max-nested-callbacks.js",
  "lib/rules/max-params.js",
  "lib/rules/max-statements-per-line.js",
  "lib/rules/max-statements.js",
  "lib/rules/multiline-comment-style.js",
  "lib/rules/multiline-ternary.js",
  "lib/rules/new-cap.js",
  "lib/rules/new-parens.js",
  "lib/rules/newline-after-var.js",
  "lib/rules/newline-before-return.js",
  "lib/rules/newline-per-chained-call.js",
  "lib/rules/no-alert.js",
  "lib/rules/no-array-constructor.js",
  "lib/rules/no-async-promise-executor.js",
  "lib/rules/no-await-in-loop.js",
  "lib/rules/no-bitwise.js",
  "lib/rules/no-buffer-constructor.js",
  "lib/rules/no-caller.js",
  "lib/rules/no-case-declarations.js",
  "lib/rules/no-catch-shadow.js",
  "lib/rules/no-class-assign.js",
  "lib/rules/no-compare-neg-zero.js",
  "lib/rules/no-cond-assign.js",
  "lib/rules/no-confusing-arrow.js",
  "lib/rules/no-console.js",
  "lib/rules/no-const-assign.js",
  "lib/rules/no-constant-binary-expression.js",
  "lib/rules/no-constant-condition.js",
  "lib/rules/no-constructor-return.js",
  "lib/rules/no-continue.js",
  "lib/rules/no-control-regex.js",
  "lib/rules/no-debugger.js",
  "lib/rules/no-delete-var.js",
  "lib/rules/no-div-regex.js",
  "lib/rules/no-dupe-args.js",
  "lib/rules/no-dupe-class-members.js",
  "lib/rules/no-dupe-else-if.js",
  "lib/rules/no-dupe-keys.js",
  "lib/rules/no-duplicate-case.js",
  "lib/rules/no-duplicate-imports.js",
  "lib/rules/no-else-return.js",
  "lib/rules/no-empty-character-class.js",
  "lib/rules/no-empty-function.js",
  "lib/rules/no-empty-pattern.js",
  "lib/rules/no-empty-static-block.js",
  "lib/rules/no-empty.js",
  "lib/rules/no-eq-null.js",
  "lib/rules/no-eval.js",
  "lib/rules/no-ex-assign.js",
  "lib/rules/no-extend-native.js",
  "lib/rules/no-extra-bind.js",
  "lib/rules/no-extra-boolean-cast.js",
  "lib/rules/no-extra-label.js",
  "lib/rules/no-extra-parens.js",
  "lib/rules/no-extra-semi.js",
  "lib/rules/no-fallthrough.js",
  "lib/rules/no-floating-decimal.js",
  "lib/rules/no-func-assign.js",
  "lib/rules/no-global-assign.js",
  "lib/rules/no-implicit-coercion.js",
  "lib/rules/no-implicit-globals.js",
  "lib/rules/no-implied-eval.js",
  "lib/rules/no-import-assign.js",
  "lib/rules/no-inline-comments.js",
  "lib/rules/no-inner-declarations.js",
  "lib/rules/no-invalid-regexp.js",
  "lib/rules/no-invalid-this.js",
  "lib/rules/no-irregular-whitespace.js",
  "lib/rules/no-iterator.js",
  "lib/rules/no-label-var.js",
  "lib/rules/no-labels.js",
  "lib/rules/no-lone-blocks.js",
  "lib/rules/no-lonely-if.js",
  "lib/rules/no-loop-func.js",
  "lib/rules/no-loss-of-precision.js",
  "lib/rules/no-magic-numbers.js",
  "lib/rules/no-misleading-character-class.js",
  "lib/rules/no-mixed-operators.js",
  "lib/rules/no-mixed-requires.js",
  "lib/rules/no-mixed-spaces-and-tabs.js",
  "lib/rules/no-multi-assign.js",
  "lib/rules/no-multi-spaces.js",
  "lib/rules/no-multi-str.js",
  "lib/rules/no-multiple-empty-lines.js",
  "lib/rules/no-native-reassign.js",
  "lib/rules/no-negated-condition.js",
  "lib/rules/no-negated-in-lhs.js",
  "lib/rules/no-nested-ternary.js",
  "lib/rules/no-new-func.js",
  "lib/rules/no-new-native-nonconstructor.js",
  "lib/rules/no-new-object.js",
  "lib/rules/no-new-require.js",
  "lib/rules/no-new-symbol.js",
  "lib/rules/no-new-wrappers.js",
  "lib/rules/no-new.js",
  "lib/rules/no-nonoctal-decimal-escape.js",
  "lib/rules/no-obj-calls.js",
  "lib/rules/no-object-constructor.js",
  "lib/rules/no-octal-escape.js",
  "lib/rules/no-octal.js",
  "lib/rules/no-param-reassign.js",
  "lib/rules/no-path-concat.js",
  "lib/rules/no-plusplus.js",
  "lib/rules/no-process-env.js",
  "lib/rules/no-process-exit.js",
  "lib/rules/no-promise-executor-return.js",
  "lib/rules/no-proto.js",
  "lib/rules/no-prototype-builtins.js",
  "lib/rules/no-redeclare.js",
  "lib/rules/no-regex-spaces.js",
  "lib/rules/no-restricted-exports.js",
  "lib/rules/no-restricted-globals.js",
  "lib/rules/no-restricted-imports.js",
  "lib/rules/no-restricted-modules.js",
  "lib/rules/no-restricted-properties.js",
  "lib/rules/no-restricted-syntax.js",
  "lib/rules/no-return-assign.js",
  "lib/rules/no-return-await.js",
  "lib/rules/no-script-url.js",
  "lib/rules/no-self-assign.js",
  "lib/rules/no-self-compare.js",
  "lib/rules/no-sequences.js",
  "lib/rules/no-setter-return.js",
  "lib/rules/no-shadow-restricted-names.js",
  "lib/rules/no-shadow.js",
  "lib/rules/no-spaced-func.js",
  "lib/rules/no-sparse-arrays.js",
  "lib/rules/no-sync.js",
  "lib/rules/no-tabs.js",
  "lib/rules/no-template-curly-in-string.js",
  "lib/rules/no-ternary.js",
  "lib/rules/no-this-before-super.js",
  "lib/rules/no-throw-literal.js",
  "lib/rules/no-trailing-spaces.js",
  "lib/rules/no-undef-init.js",
  "lib/rules/no-undef.js",
  "lib/rules/no-undefined.js",
  "lib/rules/no-underscore-dangle.js",
  "lib/rules/no-unexpected-multiline.js",
  "lib/rules/no-unmodified-loop-condition.js",
  "lib/rules/no-unneeded-ternary.js",
  "lib/rules/no-unreachable-loop.js",
  "lib/rules/no-unreachable.js",
  "lib/rules/no-unsafe-finally.js",
  "lib/rules/no-unsafe-negation.js",
  "lib/rules/no-unsafe-optional-chaining.js",
  "lib/rules/no-unused-expressions.js",
  "lib/rules/no-unused-labels.js",
  "lib/rules/no-unused-private-class-members.js",
  "lib/rules/no-unused-vars.js",
  "lib/rules/no-use-before-define.js",
  "lib/rules/no-useless-assignment.js",
  "lib/rules/no-useless-backreference.js",
  "lib/rules/no-useless-call.js",
  "lib/rules/no-useless-catch.js",
  "lib/rules/no-useless-computed-key.js",
  "lib/rules/no-useless-concat.js",
  "lib/rules/no-useless-constructor.js",
  "lib/rules/no-useless-escape.js",
  "lib/rules/no-useless-rename.js",
  "lib/rules/no-useless-return.js",
  "lib/rules/no-var.js",
  "lib/rules/no-void.js",
  "lib/rules/no-warning-comments.js",
  "lib/rules/no-whitespace-before-property.js",
  "lib/rules/no-with.js",
  "lib/rules/nonblock-statement-body-position.js",
  "lib/rules/object-curly-newline.js",
  "lib/rules/object-curly-spacing.js",
  "lib/rules/object-property-newline.js",
  "lib/rules/object-shorthand.js",
  "lib/rules/one-var-declaration-per-line.js",
  "lib/rules/one-var.js",
  "lib/rules/operator-assignment.js",
  "lib/rules/operator-linebreak.js",
  "lib/rules/padded-blocks.js",
  "lib/rules/padding-line-between-statements.js",
  "lib/rules/prefer-arrow-callback.js",
  "lib/rules/prefer-const.js",
  "lib/rules/prefer-destructuring.js",
  "lib/rules/prefer-exponentiation-operator.js",
  "lib/rules/prefer-named-capture-group.js",
  "lib/rules/prefer-numeric-literals.js",
  "lib/rules/prefer-object-has-own.js",
  "lib/rules/prefer-object-spread.js",
  "lib/rules/prefer-promise-reject-errors.js",
  "lib/rules/prefer-reflect.js",
  "lib/rules/prefer-regex-literals.js",
  "lib/rules/prefer-rest-params.js",
  "lib/rules/prefer-spread.js",
  "lib/rules/prefer-template.js",
  "lib/rules/quote-props.js",
  "lib/rules/quotes.js",
  "lib/rules/radix.js",
  "lib/rules/require-atomic-updates.js",
  "lib/rules/require-await.js",
  "lib/rules/require-unicode-regexp.js",
  "lib/rules/require-yield.js",
  "lib/rules/rest-spread-spacing.js",
  "lib/rules/semi-spacing.js",
  "lib/rules/semi-style.js",
  "lib/rules/semi.js",
  "lib/rules/sort-imports.js",
  "lib/rules/sort-keys.js",
  "lib/rules/sort-vars.js",
  "lib/rules/space-before-blocks.js",
  "lib/rules/space-before-function-paren.js",
  "lib/rules/space-in-parens.js",
  "lib/rules/space-infix-ops.js",
  "lib/rules/space-unary-ops.js",
  "lib/rules/spaced-comment.js",
  "lib/rules/strict.js",
  "lib/rules/switch-colon-spacing.js",
  "lib/rules/symbol-description.js",
  "lib/rules/template-curly-spacing.js",
  "lib/rules/template-tag-spacing.js",
  "lib/rules/unicode-bom.js",
  "lib/rules/use-isnan.js",
  "lib/rules/utils/ast-utils.js",
  "lib/rules/utils/char-source.js",
  "lib/rules/utils/fix-tracker.js",
  "lib/rules/utils/keywords.js",
  "lib/rules/utils/lazy-loading-rule-map.js",
  "lib/rules/utils/regular-expressions.js",
  "lib/rules/utils/unicode/index.js",
  "lib/rules/utils/unicode/is-combining-character.js",
  "lib/rules/utils/unicode/is-emoji-modifier.js",
  "lib/rules/utils/unicode/is-regional-indicator-symbol.js",
  "lib/rules/utils/unicode/is-surrogate-pair.js",
  "lib/rules/valid-typeof.js",
  "lib/rules/vars-on-top.js",
  "lib/rules/wrap-iife.js",
  "lib/rules/wrap-regex.js",
  "lib/rules/yield-star-spacing.js",
  "lib/rules/yoda.js",
  "lib/services/parser-service.js",
  "lib/services/processor-service.js",
  "lib/shared/ajv.js",
  "lib/shared/ast-utils.js",
  "lib/shared/directives.js",
  "lib/shared/flags.js",
  "lib/shared/logging.js",
  "lib/shared/runtime-info.js",
  "lib/shared/serialization.js",
  "lib/shared/severity.js",
  "lib/shared/stats.js",
  "lib/shared/string-utils.js",
  "lib/shared/traverser.js",
  "lib/shared/types.js",
  "lib/universal.js",
  "lib/unsupported-api.js",
  "messages/all-files-ignored.js",
  "messages/all-matched-files-ignored.js",
  "messages/config-file-missing.js",
  "messages/eslintrc-incompat.js",
  "messages/eslintrc-plugins.js",
  "messages/extend-config-missing.js",
  "messages/failed-to-read-json.js",
  "messages/file-not-found.js",
  "messages/invalid-rule-options.js",
  "messages/invalid-rule-severity.js",
  "messages/no-config-found.js",
  "messages/plugin-conflict.js",
  "messages/plugin-invalid.js",
  "messages/plugin-missing.js",
  "messages/print-config-with-directory-path.js",
  "messages/shared.js",
  "messages/whitespace-found.js",
  "package.json",
  "packages/eslint-config-eslint/base.js",
  "packages/eslint-config-eslint/cjs.js",
  "packages/eslint-config-eslint/formatting.js",
  "packages/eslint-config-eslint/index.js",
  "packages/eslint-config-eslint/nodejs.js",
  "packages/eslint-config-eslint/package.json",
  "packages/js/package.json",
  "packages/js/src/configs/eslint-all.js",
  "packages/js/src/configs/eslint-recommended.js",
  "packages/js/src/index.js",
  "tests/_utils/index.js",
  "tests/_utils/test-lazy-loading-rules.js",
  "tests/bin/eslint.js",
  "tests/conf/eslint-all.js",
  "tests/conf/eslint-recommended.js",
  "tests/lib/api.js",
  "tests/lib/cli-engine/cli-engine.js",
  "tests/lib/cli-engine/file-enumerator.js",
  "tests/lib/cli-engine/formatters/html.js",
  "tests/lib/cli-engine/formatters/json-with-metadata.js",
  "tests/lib/cli-engine/formatters/json.js",
  "tests/lib/cli-engine/formatters/stylish.js",
  "tests/lib/cli-engine/lint-result-cache.js",
  "tests/lib/cli-engine/load-rules.js",
  "tests/lib/cli.js",
  "tests/lib/config/flat-config-array.js",
  "tests/lib/config/flat-config-helpers.js",
  "tests/lib/config/flat-config-schema.js",
  "tests/lib/eslint/eslint.config.js",
  "tests/lib/eslint/eslint.js",
  "tests/lib/eslint/legacy-eslint.js",
  "tests/lib/languages/js/source-code/source-code.js",
  "tests/lib/languages/js/source-code/token-store.js",
  "tests/lib/linter/apply-disable-directives.js",
  "tests/lib/linter/code-path-analysis/code-path-analyzer.js",
  "tests/lib/linter/code-path-analysis/code-path.js",
  "tests/lib/linter/interpolate.js",
  "tests/lib/linter/linter.js",
  "tests/lib/linter/node-event-generator.js",
  "tests/lib/linter/report-translator.js",
  "tests/lib/linter/rule-fixer.js",
  "tests/lib/linter/rules.js",
  "tests/lib/linter/safe-emitter.js",
  "tests/lib/linter/source-code-fixer.js",
  "tests/lib/linter/timing.js",
  "tests/lib/linter/vfile.js",
  "tests/lib/options.js",
  "tests/lib/rule-tester/no-test-runners.js",
  "tests/lib/rule-tester/rule-tester.js",
  "tests/lib/rules/accessor-pairs.js",
  "tests/lib/rules/array-bracket-newline.js",
  "tests/lib/rules/array-bracket-spacing.js",
  "tests/lib/rules/array-callback-return.js",
  "tests/lib/rules/array-element-newline.js",
  "tests/lib/rules/arrow-body-style.js",
  "tests/lib/rules/arrow-parens.js",
  "tests/lib/rules/arrow-spacing.js",
  "tests/lib/rules/block-scoped-var.js",
  "tests/lib/rules/block-spacing.js",
  "tests/lib/rules/brace-style.js",
  "tests/lib/rules/callback-return.js",
  "tests/lib/rules/camelcase.js",
  "tests/lib/rules/capitalized-comments.js",
  "tests/lib/rules/class-methods-use-this.js",
  "tests/lib/rules/comma-dangle.js",
  "tests/lib/rules/comma-spacing.js",
  "tests/lib/rules/comma-style.js",
  "tests/lib/rules/complexity.js",
  "tests/lib/rules/computed-property-spacing.js",
  "tests/lib/rules/consistent-return.js",
  "tests/lib/rules/consistent-this.js",
  "tests/lib/rules/constructor-super.js",
  "tests/lib/rules/curly.js",
  "tests/lib/rules/default-case-last.js",
  "tests/lib/rules/default-case.js",
  "tests/lib/rules/default-param-last.js",
  "tests/lib/rules/dot-location.js",
  "tests/lib/rules/dot-notation.js",
  "tests/lib/rules/eol-last.js",
  "tests/lib/rules/eqeqeq.js",
  "tests/lib/rules/for-direction.js",
  "tests/lib/rules/func-call-spacing.js",
  "tests/lib/rules/func-name-matching.js",
  "tests/lib/rules/func-names.js",
  "tests/lib/rules/func-style.js",
  "tests/lib/rules/function-call-argument-newline.js",
  "tests/lib/rules/function-paren-newline.js",
  "tests/lib/rules/generator-star-spacing.js",
  "tests/lib/rules/getter-return.js",
  "tests/lib/rules/global-require.js",
  "tests/lib/rules/grouped-accessor-pairs.js",
  "tests/lib/rules/guard-for-in.js",
  "tests/lib/rules/handle-callback-err.js",
  "tests/lib/rules/id-blacklist.js",
  "tests/lib/rules/id-denylist.js",
  "tests/lib/rules/id-length.js",
  "tests/lib/rules/id-match.js",
  "tests/lib/rules/implicit-arrow-linebreak.js",
  "tests/lib/rules/indent-legacy.js",
  "tests/lib/rules/indent.js",
  "tests/lib/rules/init-declarations.js",
  "tests/lib/rules/jsx-quotes.js",
  "tests/lib/rules/key-spacing.js",
  "tests/lib/rules/keyword-spacing.js",
  "tests/lib/rules/line-comment-position.js",
  "tests/lib/rules/linebreak-style.js",
  "tests/lib/rules/lines-around-comment.js",
  "tests/lib/rules/lines-around-directive.js",
  "tests/lib/rules/lines-between-class-members.js",
  "tests/lib/rules/logical-assignment-operators.js",
  "tests/lib/rules/max-classes-per-file.js",
  "tests/lib/rules/max-depth.js",
  "tests/lib/rules/max-len.js",
  "tests/lib/rules/max-lines-per-function.js",
  "tests/lib/rules/max-lines.js",
  "tests/lib/rules/max-nested-callbacks.js",
  "tests/lib/rules/max-params.js",
  "tests/lib/rules/max-statements-per-line.js",
  "tests/lib/rules/max-statements.js",
  "tests/lib/rules/multiline-comment-style.js",
  "tests/lib/rules/multiline-ternary.js",
  "tests/lib/rules/new-cap.js",
  "tests/lib/rules/new-parens.js",
  "tests/lib/rules/newline-after-var.js",
  "tests/lib/rules/newline-before-return.js",
  "tests/lib/rules/newline-per-chained-call.js",
  "tests/lib/rules/no-alert.js",
  "tests/lib/rules/no-array-constructor.js",
  "tests/lib/rules/no-async-promise-executor.js",
  "tests/lib/rules/no-await-in-loop.js",
  "tests/lib/rules/no-bitwise.js",
  "tests/lib/rules/no-buffer-constructor.js",
  "tests/lib/rules/no-caller.js",
  "tests/lib/rules/no-case-declarations.js",
  "tests/lib/rules/no-catch-shadow.js",
  "tests/lib/rules/no-class-assign.js",
  "tests/lib/rules/no-compare-neg-zero.js",
  "tests/lib/rules/no-cond-assign.js",
  "tests/lib/rules/no-confusing-arrow.js",
  "tests/lib/rules/no-console.js",
  "tests/lib/rules/no-const-assign.js",
  "tests/lib/rules/no-constant-binary-expression.js",
  "tests/lib/rules/no-constant-condition.js",
  "tests/lib/rules/no-constructor-return.js",
  "tests/lib/rules/no-continue.js",
  "tests/lib/rules/no-control-regex.js",
  "tests/lib/rules/no-debugger.js",
  "tests/lib/rules/no-delete-var.js",
  "tests/lib/rules/no-div-regex.js",
  "tests/lib/rules/no-dupe-args.js",
  "tests/lib/rules/no-dupe-class-members.js",
  "tests/lib/rules/no-dupe-else-if.js",
  "tests/lib/rules/no-dupe-keys.js",
  "tests/lib/rules/no-duplicate-case.js",
  "tests/lib/rules/no-duplicate-imports.js",
  "tests/lib/rules/no-else-return.js",
  "tests/lib/rules/no-empty-character-class.js",
  "tests/lib/rules/no-empty-function.js",
  "tests/lib/rules/no-empty-pattern.js",
  "tests/lib/rules/no-empty-static-block.js",
  "tests/lib/rules/no-empty.js",
  "tests/lib/rules/no-eq-null.js",
  "tests/lib/rules/no-eval.js",
  "tests/lib/rules/no-ex-assign.js",
  "tests/lib/rules/no-extend-native.js",
  "tests/lib/rules/no-extra-bind.js",
  "tests/lib/rules/no-extra-boolean-cast.js",
  "tests/lib/rules/no-extra-label.js",
  "tests/lib/rules/no-extra-parens.js",
  "tests/lib/rules/no-extra-semi.js",
  "tests/lib/rules/no-fallthrough.js",
  "tests/lib/rules/no-floating-decimal.js",
  "tests/lib/rules/no-func-assign.js",
  "tests/lib/rules/no-global-assign.js",
  "tests/lib/rules/no-implicit-coercion.js",
  "tests/lib/rules/no-implicit-globals.js",
  "tests/lib/rules/no-implied-eval.js",
  "tests/lib/rules/no-import-assign.js",
  "tests/lib/rules/no-inline-comments.js",
  "tests/lib/rules/no-inner-declarations.js",
  "tests/lib/rules/no-invalid-regexp.js",
  "tests/lib/rules/no-invalid-this.js",
  "tests/lib/rules/no-irregular-whitespace.js",
  "tests/lib/rules/no-iterator.js",
  "tests/lib/rules/no-label-var.js",
  "tests/lib/rules/no-labels.js",
  "tests/lib/rules/no-lone-blocks.js",
  "tests/lib/rules/no-lonely-if.js",
  "tests/lib/rules/no-loop-func.js",
  "tests/lib/rules/no-loss-of-precision.js",
  "tests/lib/rules/no-magic-numbers.js",
  "tests/lib/rules/no-misleading-character-class.js",
  "tests/lib/rules/no-mixed-operators.js",
  "tests/lib/rules/no-mixed-requires.js",
  "tests/lib/rules/no-mixed-spaces-and-tabs.js",
  "tests/lib/rules/no-multi-assign.js",
  "tests/lib/rules/no-multi-spaces.js",
  "tests/lib/rules/no-multi-str.js",
  "tests/lib/rules/no-multiple-empty-lines.js",
  "tests/lib/rules/no-native-reassign.js",
  "tests/lib/rules/no-negated-condition.js",
  "tests/lib/rules/no-negated-in-lhs.js",
  "tests/lib/rules/no-nested-ternary.js",
  "tests/lib/rules/no-new-func.js",
  "tests/lib/rules/no-new-native-nonconstructor.js",
  "tests/lib/rules/no-new-object.js",
  "tests/lib/rules/no-new-require.js",
  "tests/lib/rules/no-new-symbol.js",
  "tests/lib/rules/no-new-wrappers.js",
  "tests/lib/rules/no-new.js",
  "tests/lib/rules/no-nonoctal-decimal-escape.js",
  "tests/lib/rules/no-obj-calls.js",
  "tests/lib/rules/no-object-constructor.js",
  "tests/lib/rules/no-octal-escape.js",
  "tests/lib/rules/no-octal.js",
  "tests/lib/rules/no-param-reassign.js",
  "tests/lib/rules/no-path-concat.js",
  "tests/lib/rules/no-plusplus.js",
  "tests/lib/rules/no-process-env.js",
  "tests/lib/rules/no-process-exit.js",
  "tests/lib/rules/no-promise-executor-return.js",
  "tests/lib/rules/no-proto.js",
  "tests/lib/rules/no-prototype-builtins.js",
  "tests/lib/rules/no-redeclare.js",
  "tests/lib/rules/no-regex-spaces.js",
  "tests/lib/rules/no-restricted-exports.js",
  "tests/lib/rules/no-restricted-globals.js",
  "tests/lib/rules/no-restricted-imports.js",
  "tests/lib/rules/no-restricted-modules.js",
  "tests/lib/rules/no-restricted-properties.js",
  "tests/lib/rules/no-restricted-syntax.js",
  "tests/lib/rules/no-return-assign.js",
  "tests/lib/rules/no-return-await.js",
  "tests/lib/rules/no-script-url.js",
  "tests/lib/rules/no-self-assign.js",
  "tests/lib/rules/no-self-compare.js",
  "tests/lib/rules/no-sequences.js",
  "tests/lib/rules/no-setter-return.js",
  "tests/lib/rules/no-shadow-restricted-names.js",
  "tests/lib/rules/no-shadow.js",
  "tests/lib/rules/no-spaced-func.js",
  "tests/lib/rules/no-sparse-arrays.js",
  "tests/lib/rules/no-sync.js",
  "tests/lib/rules/no-tabs.js",
  "tests/lib/rules/no-template-curly-in-string.js",
  "tests/lib/rules/no-ternary.js",
  "tests/lib/rules/no-this-before-super.js",
  "tests/lib/rules/no-throw-literal.js",
  "tests/lib/rules/no-trailing-spaces.js",
  "tests/lib/rules/no-undef-init.js",
  "tests/lib/rules/no-undef.js",
  "tests/lib/rules/no-undefined.js",
  "tests/lib/rules/no-underscore-dangle.js",
  "tests/lib/rules/no-unexpected-multiline.js",
  "tests/lib/rules/no-unmodified-loop-condition.js",
  "tests/lib/rules/no-unneeded-ternary.js",
  "tests/lib/rules/no-unreachable-loop.js",
  "tests/lib/rules/no-unreachable.js",
  "tests/lib/rules/no-unsafe-finally.js",
  "tests/lib/rules/no-unsafe-negation.js",
  "tests/lib/rules/no-unsafe-optional-chaining.js",
  "tests/lib/rules/no-unused-expressions.js",
  "tests/lib/rules/no-unused-labels.js",
  "tests/lib/rules/no-unused-private-class-members.js",
  "tests/lib/rules/no-unused-vars.js",
  "tests/lib/rules/no-use-before-define.js",
  "tests/lib/rules/no-useless-assignment.js",
  "tests/lib/rules/no-useless-backreference.js",
  "tests/lib/rules/no-useless-call.js",
  "tests/lib/rules/no-useless-catch.js",
  "tests/lib/rules/no-useless-computed-key.js",
  "tests/lib/rules/no-useless-concat.js",
  "tests/lib/rules/no-useless-constructor.js",
  "tests/lib/rules/no-useless-escape.js",
  "tests/lib/rules/no-useless-rename.js",
  "tests/lib/rules/no-useless-return.js",
  "tests/lib/rules/no-var.js",
  "tests/lib/rules/no-void.js",
  "tests/lib/rules/no-warning-comments.js",
  "tests/lib/rules/no-whitespace-before-property.js",
  "tests/lib/rules/no-with.js",
  "tests/lib/rules/nonblock-statement-body-position.js",
  "tests/lib/rules/object-curly-newline.js",
  "tests/lib/rules/object-curly-spacing.js",
  "tests/lib/rules/object-property-newline.js",
  "tests/lib/rules/object-shorthand.js",
  "tests/lib/rules/one-var-declaration-per-line.js",
  "tests/lib/rules/one-var.js",
  "tests/lib/rules/operator-assignment.js",
  "tests/lib/rules/operator-linebreak.js",
  "tests/lib/rules/padded-blocks.js",
  "tests/lib/rules/padding-line-between-statements.js",
  "tests/lib/rules/prefer-arrow-callback.js",
  "tests/lib/rules/prefer-const.js",
  "tests/lib/rules/prefer-destructuring.js",
  "tests/lib/rules/prefer-exponentiation-operator.js",
  "tests/lib/rules/prefer-named-capture-group.js",
  "tests/lib/rules/prefer-numeric-literals.js",
  "tests/lib/rules/prefer-object-has-own.js",
  "tests/lib/rules/prefer-object-spread.js",
  "tests/lib/rules/prefer-promise-reject-errors.js",
  "tests/lib/rules/prefer-reflect.js",
  "tests/lib/rules/prefer-regex-literals.js",
  "tests/lib/rules/prefer-rest-params.js",
  "tests/lib/rules/prefer-spread.js",
  "tests/lib/rules/prefer-template.js",
  "tests/lib/rules/quote-props.js",
  "tests/lib/rules/quotes.js",
  "tests/lib/rules/radix.js",
  "tests/lib/rules/require-atomic-updates.js",
  "tests/lib/rules/require-await.js",
  "tests/lib/rules/require-unicode-regexp.js",
  "tests/lib/rules/require-yield.js",
  "tests/lib/rules/rest-spread-spacing.js",
  "tests/lib/rules/semi-spacing.js",
  "tests/lib/rules/semi-style.js",
  "tests/lib/rules/semi.js",
  "tests/lib/rules/sort-imports.js",
  "tests/lib/rules/sort-keys.js",
  "tests/lib/rules/sort-vars.js",
  "tests/lib/rules/space-before-blocks.js",
  "tests/lib/rules/space-before-function-paren.js",
  "tests/lib/rules/space-in-parens.js",
  "tests/lib/rules/space-infix-ops.js",
  "tests/lib/rules/space-unary-ops.js",
  "tests/lib/rules/spaced-comment.js",
  "tests/lib/rules/strict.js",
  "tests/lib/rules/switch-colon-spacing.js",
  "tests/lib/rules/symbol-description.js",
  "tests/lib/rules/template-curly-spacing.js",
  "tests/lib/rules/template-tag-spacing.js",
  "tests/lib/rules/unicode-bom.js",
  "tests/lib/rules/use-isnan.js",
  "tests/lib/rules/utils/ast-utils.js",
  "tests/lib/rules/utils/char-source.js",
  "tests/lib/rules/utils/fix-tracker.js",
  "tests/lib/rules/valid-typeof.js",
  "tests/lib/rules/vars-on-top.js",
  "tests/lib/rules/wrap-iife.js",
  "tests/lib/rules/wrap-regex.js",
  "tests/lib/rules/yield-star-spacing.js",
  "tests/lib/rules/yoda.js",
  "tests/lib/shared/runtime-info.js",
  "tests/lib/shared/serialization.js",
  "tests/lib/shared/string-utils.js",
  "tests/lib/shared/traverser.js",
  "tests/lib/types/tsconfig.json",
  "tests/lib/types/types.test.ts",
  "tests/lib/universal.js",
  "tests/lib/unsupported-api.js",
  "tests/tools/check-rule-examples.js",
  "tests/tools/code-sample-minimizer.js",
  "tests/tools/config-rule.js",
  "tests/tools/eslint-fuzzer.js",
  "tests/tools/internal-rules/multiline-comment-style.js",
  "tests/tools/internal-rules/no-invalid-meta.js",
  "tools/check-emfile-handling.js",
  "tools/check-rule-examples.js",
  "tools/code-sample-minimizer.js",
  "tools/config-rule.js",
  "tools/eslint-fuzzer.js",
  "tools/fetch-docs-links.js",
  "tools/fuzzer-runner.js",
  "tools/internal-rules/index.js",
  "tools/internal-rules/multiline-comment-style.js",
  "tools/internal-rules/no-invalid-meta.js",
  "tools/internal-testers/event-generator-tester.js",
  "tools/update-eslint-all.js",
  "tools/update-readme.js",
  "wdio.conf.js",
  "webpack.config.js",
];

console.log(`Linting ${files.length} files`);

const start = process.hrtime.bigint();

await eslint.lintFiles(files);

const end = process.hrtime.bigint();

console.log(`Finished in ${Number(end - start) / 1e9}s`);

I'm getting:

Before: Finished in 33.6787389s
After: Finished in 24.7107211s

@mdjermanovic mdjermanovic changed the title perf: Fix caching in LegacyConfigLoader perf: Fix caching in config loaders Oct 23, 2024
@mdjermanovic
Copy link
Copy Markdown
Member Author

mdjermanovic commented Oct 23, 2024 •
edited
Loading

Now I also updated caching in ConfigLoader in the same way as in LegacyConfigLoader. Interesting that the same test as in #19042 (comment), but with new ESLint({ flags: ["unstable_config_lookup_from_file"] }), shows the same performance before and after, and about the same performance as after without unstable_config_lookup_from_file, which confirms observations from #19025 that ESLint works faster with unstable_config_lookup_from_file. Seems that in the unstable_config_lookup_from_file mode, due to a different order of execution, the caching problem wasn't that impactful. Still, I think the caching should be updated in ConfigLoader as well to avoid race conditions.

@mdjermanovic mdjermanovic added the accepted There is consensus among the team that this change meets the criteria for inclusion label Oct 23, 2024
@mdjermanovic
Copy link
Copy Markdown
Member Author

mdjermanovic commented Oct 24, 2024

I added counters in find-up and FlatConfigArray to doublecheck with the test from #19042 (comment) which passes in 830 individual file paths.

Regular mode (no unstable_config_lookup_from_file flag):

Number of findUp() calls Number of new FlatConfigArray() calls
Before 830 830
After 1 1

unstable_config_lookup_from_file mode:

Number of findUp() calls Number of new FlatConfigArray() calls
Before 830 13
After 54 2

@mdjermanovic mdjermanovic marked this pull request as ready for review October 24, 2024 14:35
@mdjermanovic mdjermanovic requested a review from a team as a code owner October 24, 2024 14:35
nzakas
nzakas reviewed Oct 24, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM.

Instead of adding tests for the ESLint class, which requires you to proxyquire things, what do you think about creating unit tests for ConfigLoader and LegacyConfigLoader directly?

@mdjermanovic
Copy link
Copy Markdown
Member Author

mdjermanovic commented Oct 25, 2024

Instead of adding tests for the ESLint class, which requires you to proxyquire things, what do you think about creating unit tests for ConfigLoader and LegacyConfigLoader directly?

Done in c8631a4.

nzakas
nzakas previously approved these changes Oct 25, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM. Will leave open over the weekend to allow for other reviews.

fasttime
fasttime reviewed Oct 26, 2024
View reviewed changes
Comment thread lib/config/config-loader.js Outdated
const configFilePathInfo = this.#configFilePaths.get(absoluteDirPath);

if (typeof configFilePathInfo.then === "function") {
throw new Error(`Config file path for ${fileOrDirPath} has not yet been calculated`);
Copy link
Copy Markdown
Member

@fasttime fasttime Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is's assumed that configFilePathInfo will be pending (and not rejected) here, based on the error message?

Copy link
Copy Markdown
Member Author

@mdjermanovic mdjermanovic Oct 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on when this is called, if configFilePathInfo is a promise, it can be in any state: pending, fulfilled but not processed yet, or rejected. I think the message covers the first two cases. Maybe we should add more text to cover the third case as well?

Copy link
Copy Markdown
Member

@fasttime fasttime Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if the configFilePathInfo can be fulfilled, because the promise in the #configFilePaths map is synchronously replaced with its awaited value when it fulfills before any other handlers run. But it could be rejected or pending I think if ESLint#getRulesMetaForResults is called improperly. It's unlikely that a user will see this error message, but feel free to extend it if you want.

Copy link
Copy Markdown
Member Author

@mdjermanovic mdjermanovic Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated error messages in 8b1481e

snitin315
snitin315 approved these changes Oct 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great fix!

Leaving it open for @fasttime

fasttime
fasttime approved these changes Oct 27, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@fasttime fasttime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Awesome! Leaving open because the weekend isn't over.

@nzakas nzakas merged commit 425202e into main Oct 28, 2024
@nzakas nzakas deleted the issue19025-1 branch October 28, 2024 12:59
@mdjermanovic
Copy link
Copy Markdown
Member Author

mdjermanovic commented Nov 26, 2024

Here are some performance comparisons from ESLint v9.11.1 to v9.15.0, on Node.js v22.11.0.

Notes:

  • "Multiple Individual Files" is the test from perf: Fix caching in config loaders #19042 (comment), just without "lib/config/config-loader.js" file as it didn't exist in v9.11.1. It runs ESLint with a list of individual files.
  • "Multi Files" test runs eslint with glob {lib,tests/lib}/**/*.js.
  • The problem with caching was introduced in v9.12.0 and addresed in v9.14.0.
  • Using Node.js compile cache was introduced in v9.13.0 (c7abaef).

ESLint v9.11.1

Multiple Individual Files test:

Linting 829 files
(node:9752) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files
(Use `node --trace-warnings ...` to show where the warning was created)
Finished in 25.2371744s

npm run test:performance:

> eslint@9.11.1 test:performance
> node Makefile.js perf


Loading:
  Load performance Run #1:  147.3295ms
  Load performance Run #2:  151.4037ms
  Load performance Run #3:  143.925ms
  Load performance Run #4:  148.7198ms
  Load performance Run #5:  153.5939ms

  Load Performance median:  148.7198ms


Single File:
  CPU Speed is 2803 with multiplier 13000000
  Performance Run #1:  2566.0625ms
  Performance Run #2:  2514.8421ms
  Performance Run #3:  2525.1341ms
  Performance Run #4:  2522.0918ms
  Performance Run #5:  2516.7777ms

  Performance budget ok:  2522.0918ms (limit: 4637.887977167321ms)


Multi Files (450 files):
  CPU Speed is 2803 with multiplier 39000000
  Performance Run #1:  6161.1958ms
  Performance Run #2:  6195.7136ms
  Performance Run #3:  6243.2265ms
  Performance Run #4:  6131.552ms
  Performance Run #5:  6125.0619ms

  Performance budget ok:  6161.1958ms (limit: 13913.663931501962ms)

ESLint v9.12.0

Multiple Individual Files test:

Linting 829 files
(node:2184) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files
(Use `node --trace-warnings ...` to show where the warning was created)
Finished in 34.0117622s

npm run test:performance:

> eslint@9.12.0 test:performance
> node Makefile.js perf


Loading:
  Load performance Run #1:  135.7588ms
  Load performance Run #2:  138.0195ms
  Load performance Run #3:  134.1108ms
  Load performance Run #4:  133.0513ms
  Load performance Run #5:  134.5354ms

  Load Performance median:  134.5354ms


Single File:
  CPU Speed is 2803 with multiplier 13000000
  Performance Run #1:  2190.0695ms
  Performance Run #2:  2131.5592ms
  Performance Run #3:  2136.8994000000002ms
  Performance Run #4:  2140.1554ms
  Performance Run #5:  2139.0977ms

  Performance budget ok:  2139.0977ms (limit: 4637.887977167321ms)


Multi Files (450 files):
  CPU Speed is 2803 with multiplier 39000000
  Performance Run #1:  5253.8577ms
  Performance Run #2:  5255.7197ms
  Performance Run #3:  5298.0601ms
  Performance Run #4:  5292.0779999999995ms
  Performance Run #5:  5302.0729ms

  Performance budget ok:  5292.0779999999995ms (limit: 13913.663931501962ms)

ESLint v9.13.0

Multiple Individual Files test:

Linting 829 files
(node:6512) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files
(Use `node --trace-warnings ...` to show where the warning was created)
Finished in 33.902926s

npm run test:performance:

> eslint@9.13.0 test:performance
> node Makefile.js perf


Loading:
  Load performance Run #1:  135.6133ms
  Load performance Run #2:  131.7872ms
  Load performance Run #3:  136.6388ms
  Load performance Run #4:  137.6985ms
  Load performance Run #5:  137.1877ms

  Load Performance median:  136.6388ms


Single File:
  CPU Speed is 2803 with multiplier 13000000
  Performance Run #1:  2420.9562ms
  Performance Run #2:  2126.8546ms
  Performance Run #3:  2124.8257ms
  Performance Run #4:  2106.7238ms
  Performance Run #5:  2109.7183ms

  Performance budget ok:  2124.8257ms (limit: 4637.887977167321ms)


Multi Files (450 files):
  CPU Speed is 2803 with multiplier 39000000
  Performance Run #1:  5250.0039ms
  Performance Run #2:  5256.3579ms
  Performance Run #3:  5293.5773ms
  Performance Run #4:  5310.3409ms
  Performance Run #5:  5325.5768ms

  Performance budget ok:  5293.5773ms (limit: 13913.663931501962ms)

ESLint v9.14.0

Multiple Individual Files test:

Linting 829 files
(node:12932) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files
(Use `node --trace-warnings ...` to show where the warning was created)
Finished in 25.6888137s

npm run test:performance:

> eslint@9.14.0 test:performance
> node Makefile.js perf


Loading:
  Load performance Run #1:  138.8344ms
  Load performance Run #2:  133.9336ms
  Load performance Run #3:  135.7778ms
  Load performance Run #4:  137.2512ms
  Load performance Run #5:  138.4868ms

  Load Performance median:  137.2512ms


Single File:
  CPU Speed is 2803 with multiplier 13000000
  Performance Run #1:  2248.7118ms
  Performance Run #2:  2127.3668ms
  Performance Run #3:  2112.0046ms
  Performance Run #4:  2119.1818ms
  Performance Run #5:  2129.107ms

  Performance budget ok:  2127.3668ms (limit: 4637.887977167321ms)


Multi Files (450 files):
  CPU Speed is 2803 with multiplier 39000000
  Performance Run #1:  5394.3309ms
  Performance Run #2:  5326.4874ms
  Performance Run #3:  5296.7867ms
  Performance Run #4:  5456.2602ms
  Performance Run #5:  5341.5221ms

  Performance budget ok:  5341.5221ms (limit: 13913.663931501962ms)

ESLint v9.15.0

Multiple Individual Files test:

Linting 829 files
(node:9336) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files
(Use `node --trace-warnings ...` to show where the warning was created)
Finished in 27.2308847s

npm run test:performance:

> eslint@9.15.0 test:performance
> node Makefile.js perf


Loading:
  Load performance Run #1:  154.3313ms
  Load performance Run #2:  147.3017ms
  Load performance Run #3:  150.1097ms
  Load performance Run #4:  150.3218ms
  Load performance Run #5:  150.2932ms

  Load Performance median:  150.2932ms


Single File:
  CPU Speed is 2803 with multiplier 13000000
  Performance Run #1:  2249.5559ms
  Performance Run #2:  1967.5928ms
  Performance Run #3:  1974.8404ms
  Performance Run #4:  1980.2089999999998ms
  Performance Run #5:  1972.7755ms

  Performance budget ok:  1974.8404ms (limit: 4637.887977167321ms)


Multi Files (450 files):
  CPU Speed is 2803 with multiplier 39000000
  Performance Run #1:  5461.2538ms
  Performance Run #2:  5452.589ms
  Performance Run #3:  5463.9471ms
  Performance Run #4:  5466.3233ms
  Performance Run #5:  5441.1518ms

  Performance budget ok:  5461.2538ms (limit: 13913.663931501962ms)

@mdjermanovic
Copy link
Copy Markdown
Member Author

mdjermanovic commented Nov 26, 2024

Some observations from the above testing results:

  • Performance problem when linting multiple individual files, introduced in v9.12.0, was fixed in v9.14.0. However, there seems to be a small degradation in v9.15.0. There also seems to be a small degradation in the Load test in v9.15.0.
  • Some changes done in v9.12.0 caused considerable performance improvements in Loading, Single File, and Multi Files tests. Not sure what that was.
  • Surprisingly, the Node.js compile cache change isn't noticeable in these tests.

@nzakas
Copy link
Copy Markdown
Member

nzakas commented Nov 26, 2024

Thanks for digging into this! Would you like to compile that data and observations into a blog post? I think this could be valuable to share with everyone.

@mdjermanovic
Copy link
Copy Markdown
Member Author

mdjermanovic commented Nov 27, 2024

Thanks for digging into this! Would you like to compile that data and observations into a blog post? I think this could be valuable to share with everyone.

Gladly, I'd just like to double-check the validity of my testing first as some results are unexpected.

@fasttime
Copy link
Copy Markdown
Member

fasttime commented Nov 29, 2024

It looks like the Loading test doesn't enable Node.js compile cache, only the Single File and Multi File tests do that.

The compile cache is enabled in bin/eslint.js, but the Loading test loads lib/api.js directly using a wrapper (load-perf). The command string looks like this:

node ./node_modules/load-perf/lib/loader.js ./lib/api.js

I can see it when I stop the debugger here:

eslint/Makefile.js

Lines 1009 to 1011 in d0a5414

const loadPerfData = loadPerf({
checkDependencies: false
});

And then step into the the code of load-perf until this line:

https://github.com/gyandeeps/load-perf/blob/c31e006b46b12f09ee6a9c06bdd794de7ddec892/lib/index.js#L36

If I delete bin/eslint.js and run npm run test:performance, the Loading test passes correctly until the Single File test crashes the run.

I believe this is the reason why the benchmark data show no improvement in the Loading test from v9.12.0 to v9.13.0.

@mdjermanovic
Copy link
Copy Markdown
Member Author

mdjermanovic commented Dec 11, 2024

@fasttime thanks for checking all this! I'm wondering how the Loading test showed improvements in #19012 then.

So, the most relevant test for Node.js compile cache would then be the Single File test?

@fasttime
Copy link
Copy Markdown
Member

fasttime commented Dec 11, 2024

@fasttime thanks for checking all this! I'm wondering how the Loading test showed improvements in #19012 then.

Yeah, that's weird. I don't see a noticeable performance improvement from v9.12.0 to v.9.13.0 in the Loading test on my machine.

So, the most relevant test for Node.js compile cache would then be the Single File test?

It seems so, and in that case the improvement for the Single File test would be still very small. 🤷‍♂️

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

Reviewers

@fasttime fasttime fasttime approved these changes

@snitin315 snitin315 snitin315 approved these changes

@nzakas nzakas nzakas left review comments

Assignees

No one assigned

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion chore This change is not user-facing core Relates to ESLint's core APIs and features

Projects

Triage
Status: Complete

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: Performance Regression in ESLint 9.12.0

4 participants

@mdjermanovic @nzakas @fasttime @snitin315