feat: `--debug` prints time it takes to parse a file by bartlomieju · Pull Request #15609 · eslint/eslint

bartlomieju · 2022-02-15T02:35:08Z

Prerequisites checklist

I have read the contributing guidelines.

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
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

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

Closes #15585

I think this is enough debug output, if parsing takes less <1ms then its cost is negligible anyway.

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

mdjermanovic · 2022-02-15T14:54:13Z

        const scopeManager = parseResult.scopeManager || analyzeScope(ast, languageOptions, visitorKeys);

+        debug("Parsing successful:", filePath);


Espree (default parser) doesn't provide parseResult.scopeManager, so this would include time spent in a separate scope analysis (by eslint-scope), which might be a significant % of the total time between these two debug logs.

I don't know if this makes more sense for your analysis, just want to check if the intent is to calculate parsing time + scope analysis time, or it might be better to split them with more debug logs?

For context, some parsers, like @typescript-eslint/parser, provide their own scope analysis (as parseResult.scopeManager) so in that case there would be no difference since both parsing and scope analysis are done in parser.parseForESLint().

That's a good point, I think it might be useful to have time taken for both steps logged separately. Could you hint how this could be solved generically that would handle built-in parser as well as @typescript-eslint/parser?

how this could be solved generically that would handle built-in parser as well as @typescript-eslint/parser

We can add one pair of debug logs around const parseResult = ..., and another pair around const scopeManager = ....

When the built-in parser is used, that would log parse time and scope time separately.

When @typescript-eslint/parser is used, parse time and scope time would be both included in the first log (the second would be ~0 time) and it doesn't seem there's anything we can do to split them here. The only solution might be to ask @typescript-eslint/parser to add logs around this line. Then, it would be possible to see parse + scope time (logged by ESLint) and scope time (logged by @typescript-eslint/parser) ,

The only solution might be to ask @typescript-eslint/parser to add logs around this line. Then, it would be possible to see parse + scope time (logged by ESLint) and scope time (logged by @typescript-eslint/parser) ,

In this case, since both packages are using the same debug library, you could set environment variable DEBUG=eslint:linter,typescript-eslint:parser:parser and run eslint without --debug to see logs from both modules.

Thanks @mdjermanovic, I applied your suggestions

mdjermanovic

LGTM, thanks!

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.9.0` -> `8.10.0`](https://renovatebot.com/diffs/npm/eslint/8.9.0/8.10.0) | --- ### Release Notes <details> <summary>eslint/eslint</summary> ### [`v8.10.0`](https://github.com/eslint/eslint/releases/v8.10.0) [Compare Source](eslint/eslint@v8.9.0...v8.10.0) #### Features - [`6e2c325`](eslint/eslint@6e2c325) feat: Add `ignoreOnInitialization` option to no-shadow rule ([#14963](eslint/eslint#14963)) (Soufiane Boutahlil) - [`115cae5`](eslint/eslint@115cae5) feat: `--debug` prints time it takes to parse a file ([#15609](eslint/eslint#15609)) (Bartek Iwańczuk) - [`345e70d`](eslint/eslint@345e70d) feat: Add `onlyOneSimpleParam` option to no-confusing-arrow rule ([#15566](eslint/eslint#15566)) (Gautam Arora) #### Bug Fixes - [`cdc5802`](eslint/eslint@cdc5802) fix: Avoid `__dirname` for built-in configs ([#15616](eslint/eslint#15616)) (DoZerg) - [`ee7c5d1`](eslint/eslint@ee7c5d1) fix: false positive in `camelcase` with combined properties ([#15581](eslint/eslint#15581)) (Nitin Kumar) #### Documentation - [`1005bd5`](eslint/eslint@1005bd5) docs: update CLA information ([#15630](eslint/eslint#15630)) (Nitin Kumar) - [`5d65c3b`](eslint/eslint@5d65c3b) docs: Fix typo in `no-irregular-whitespace` ([#15634](eslint/eslint#15634)) (Ryota Sekiya) - [`b93af98`](eslint/eslint@b93af98) docs: add links between rules about whitespace around block curly braces ([#15625](eslint/eslint#15625)) (Milos Djermanovic) - [`ebc0460`](eslint/eslint@ebc0460) docs: update babel links ([#15624](eslint/eslint#15624)) (Milos Djermanovic) #### Chores - [`7cec74e`](eslint/eslint@7cec74e) chore: upgrade [@eslint/eslintrc](https://github.com/eslint/eslintrc)[@1](https://github.com/1).2.0 ([#15648](eslint/eslint#15648)) (Milos Djermanovic) - [`11c8580`](eslint/eslint@11c8580) chore: read `ESLINT_MOCHA_TIMEOUT` env var in Makefile.js ([#15626](eslint/eslint#15626)) (Piggy) - [`bfaa548`](eslint/eslint@bfaa548) test: add integration tests with built-in configs ([#15612](eslint/eslint#15612)) (Milos Djermanovic) - [`39a2fb3`](eslint/eslint@39a2fb3) perf: fix lazy loading of core rules ([#15606](eslint/eslint#15606)) (Milos Djermanovic) - [`3fc9196`](eslint/eslint@3fc9196) chore: include `tests/conf` in test runs ([#15610](eslint/eslint#15610)) (Milos Djermanovic) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1190 Reviewed-by: 6543 <6543@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

* feat: --debug prints time it takes to parse a file * Update lib/linter/linter.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * add separate logs for parsing and scope analysis * lint Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

…15609)" This reverts commit ef85718.

feat: --debug prints time it takes to parse a file

dbc0d62

eslint-github-bot Bot added triage An ESLint team member will look at this issue soon feature This change adds a new feature to ESLint labels Feb 15, 2022

mdjermanovic reviewed Feb 15, 2022

View reviewed changes

Comment thread lib/linter/linter.js Outdated

Update lib/linter/linter.js

9e9a715

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

mdjermanovic changed the title ~~feat: --debug prints time it takes to parse a file~~ feat: --debug prints time it takes to parse a file Feb 15, 2022

mdjermanovic reviewed Feb 15, 2022

View reviewed changes

bartlomieju added 2 commits February 19, 2022 17:42

add separate logs for parsing and scope analysis

618fea1

lint

dac6a14

mdjermanovic approved these changes Feb 20, 2022

View reviewed changes

mdjermanovic merged commit 115cae5 into eslint:main Feb 20, 2022

bartlomieju deleted the debug_log_parse branch February 20, 2022 15:00

sync-by-unito Bot mentioned this pull request Mar 1, 2022

build(deps-dev): bump eslint from 7.32.0 to 8.10.0 numbersprotocol/capture-cam#1334

Closed

sync-by-unito Bot mentioned this pull request Mar 30, 2022

build(deps-dev): bump eslint from 7.32.0 to 8.12.0 numbersprotocol/capture-cam#1446

Closed

srijan-deepsource added a commit to DeepSourceCorp/eslint that referenced this pull request May 30, 2022

Revert "feat: --debug prints time it takes to parse a file (eslint#…

39daf42

…15609)" This reverts commit ef85718.

eslint-github-bot Bot locked and limited conversation to collaborators Aug 20, 2022

eslint-github-bot Bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: `--debug` prints time it takes to parse a file#15609

feat: `--debug` prints time it takes to parse a file#15609
mdjermanovic merged 4 commits intoeslint:mainfrom
bartlomieju:debug_log_parse

bartlomieju commented Feb 15, 2022

Uh oh!

Uh oh!

mdjermanovic Feb 15, 2022

Uh oh!

bartlomieju Feb 15, 2022

Uh oh!

mdjermanovic Feb 15, 2022

Uh oh!

mdjermanovic Feb 15, 2022

Uh oh!

bartlomieju Feb 19, 2022

Uh oh!

mdjermanovic left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const scopeManager = parseResult.scopeManager \|\| analyzeScope(ast, languageOptions, visitorKeys);

		debug("Parsing successful:", filePath);

Uh oh!

Conversation

bartlomieju commented Feb 15, 2022

Prerequisites checklist

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

What changes did you make? (Give an overview)

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

Uh oh!

Uh oh!

mdjermanovic Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

bartlomieju Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

mdjermanovic Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

mdjermanovic Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

bartlomieju Feb 19, 2022

Choose a reason for hiding this comment

Uh oh!

mdjermanovic left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants