Skip to content

fix: unify timing report for concurrent linting #20188

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.

Sign up for GitHub

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

Jump to bottom
Merged
mdjermanovic merged 6 commits into from
Oct 23, 2025
Merged

fix: unify timing report for concurrent linting #20188

mdjermanovic merged 6 commits into from
Oct 23, 2025

Conversation

@jaymarvelz
Copy link
Contributor

@jaymarvelz jaymarvelz commented Oct 6, 2025

Prerequisites checklist

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

[ ] Documentation update
[x] 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
[ ] Other, please explain:

What changes did you make? (Give an overview)

When running ESLint with --concurrency, multiple timing reports were generated — one per worker thread — resulting in fragmented timing data.

This change ensures that timing information is collected and merged into a single unified report, matching the behavior seen when linting without concurrency.

  • Add getData() to expose per-rule timing totals from each worker.
  • Add mergeData() to aggregate worker timings in the main thread.
  • Print the timing table only from the main thread to avoid multiple outputs.

Fixes #20063

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

@jaymarvelz jaymarvelz requested a review from a team as a code owner October 6, 2025 09:12
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Oct 6, 2025
@netlify
Copy link

netlify bot commented Oct 6, 2025 •
edited
Loading

✅ Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit a5169b9
🔍 Latest deploy log https://app.netlify.com/projects/docs-eslint/deploys/68f9a5b23367a000089edf08

@eslint-github-bot eslint-github-bot bot added the bug ESLint is working incorrectly label Oct 6, 2025
@eslint-github-bot eslint-github-bot bot mentioned this pull request Oct 6, 2025
Closed
1 task
@github-actions github-actions bot added cli Relates to ESLint's command-line interface core Relates to ESLint's core APIs and features labels Oct 6, 2025
@kecrily kecrily moved this from Needs Triage to Implementing in Triage Oct 6, 2025
@kecrily kecrily added the accepted There is consensus among the team that this change meets the criteria for inclusion label Oct 6, 2025
@mdjermanovic
Copy link
Member

mdjermanovic commented Oct 15, 2025

When I test this locally, with this project, it prints 3 extra headers for the table:

$ TIMING=10 node bin/eslint . --concurrency 2
(node:9364) 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)
Rule | Time (ms) | Relative
:----|----------:|--------:
Rule | Time (ms) | Relative
:----|----------:|--------:
Rule                                | Time (ms) | Relative
:-----------------------------------|----------:|--------:
n/no-extraneous-require             |  1138.998 |     5.5%
jsdoc/valid-types                   |   975.487 |     4.7%
expect-type/expect                  |   845.054 |     4.1%
jsdoc/check-access                  |   822.265 |     3.9%
jsdoc/check-values                  |   758.577 |     3.6%
jsdoc/check-types                   |   702.723 |     3.4%
n/no-unsupported-features/es-syntax |   571.277 |     2.7%
jsdoc/check-tag-names               |   554.260 |     2.7%
jsdoc/check-alignment               |   539.435 |     2.6%
n/no-unpublished-require            |   534.973 |     2.6%
Rule | Time (ms) | Relative
:----|----------:|--------:

Not sure why and how this happens.

mdjermanovic
mdjermanovic reviewed Oct 15, 2025
View reviewed changes
@jaymarvelz
Copy link
Contributor Author

jaymarvelz commented Oct 15, 2025

@mdjermanovic I tested this multiple times locally, but I’m not getting the extra headers

@mdjermanovic
Copy link
Member

mdjermanovic commented Oct 16, 2025

I'm still getting extra headers, but it's interesting that I'm also getting one extra header on the main branch even without concurrency.

$ TIMING=10 npx eslint Makefile.js
(node:12608) 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)
Rule                                | Time (ms) | Relative
:-----------------------------------|----------:|--------:
n/no-extraneous-require             |    43.087 |    17.0%
regexp/confusing-quantifier         |    23.604 |     9.3%
jsdoc/check-access                  |     9.383 |     3.7%
jsdoc/valid-types                   |     7.549 |     3.0%
jsdoc/check-types                   |     6.048 |     2.4%
n/no-unsupported-features/es-syntax |     5.920 |     2.3%
n/no-unpublished-require            |     5.436 |     2.1%
n/no-missing-require                |     5.267 |     2.1%
jsdoc/check-values                  |     5.156 |     2.0%
jsdoc/require-property-description  |     4.477 |     1.8%
Rule | Time (ms) | Relative
:----|----------:|--------:

Anyone else getting the same? It could be that some of the eslint plugins we're using is doing something unusual, like importing Linter.

@fasttime
Copy link
Member

fasttime commented Oct 16, 2025 •
edited
Loading

Anyone else getting the same? It could be that some of the eslint plugins we're using is doing something unusual, like importing Linter.

I'm not getting multiple headers with TIMING=10 npx eslint Makefile.js. Is the initialization code in lib/linter/timing.js running twice for you?

@mdjermanovic
Copy link
Member

mdjermanovic commented Oct 16, 2025

Okay, I got it. eslint-plugin-jsdoc and eslint-plugin-unicorn are importing some eslint modules, and that also triggers loading the timing.js module. Plus, after some testing, I had both plugins installed under packages/eslint-config-eslint/node_modules along with a copy of eslint, and that copy of eslint was printing extra headers. So it's definitely not a problem caused by this PR.

mdjermanovic
mdjermanovic reviewed Oct 16, 2025
View reviewed changes
fasttime
fasttime reviewed Oct 16, 2025
View reviewed changes
fasttime
fasttime reviewed Oct 22, 2025
View reviewed changes
fasttime
fasttime reviewed Oct 22, 2025
View reviewed changes
fasttime
fasttime approved these changes Oct 23, 2025
View reviewed changes
Copy link
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, thanks! Leaving open for @mdjermanovic to review.

@fasttime fasttime moved this from Implementing to Second Review Needed in Triage Oct 23, 2025
mdjermanovic
mdjermanovic approved these changes Oct 23, 2025
View reviewed changes
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit ae07f0b into eslint:main Oct 23, 2025
32 checks passed
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Oct 23, 2025
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

@mdjermanovic mdjermanovic mdjermanovic approved these changes

Assignees

No one assigned

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly cli Relates to ESLint's command-line interface contributor pool 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.

Change Request: unified TIMING report with concurrent linting

4 participants

@jaymarvelz @mdjermanovic @fasttime @kecrily