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.

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

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

Merged
merged 4 commits into from Feb 20, 2022
Merged

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

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dbc0d62
feat: --debug prints time it takes to parse a file
bartlomieju Feb 15, 2022
9e9a715
Update lib/linter/linter.js
bartlomieju Feb 15, 2022
618fea1
add separate logs for parsing and scope analysis
bartlomieju Feb 19, 2022
dac6a14
lint
bartlomieju Feb 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/linter/linter.js
View file
Open in desktop
Expand Up @@ -800,6 +800,7 @@ function parse(text, languageOptions, filePath) {
* problem that ESLint identified just like any other.
*/
try {
debug("Parsing:", filePath);
const parseResult = (typeof parser.parseForESLint === "function")
? parser.parseForESLint(textToParse, parserOptions)
: { ast: parser.parse(textToParse, parserOptions) };
Expand All @@ -808,6 +809,8 @@ function parse(text, languageOptions, filePath) {
const visitorKeys = parseResult.visitorKeys || evk.KEYS;
const scopeManager = parseResult.scopeManager || analyzeScope(ast, languageOptions, visitorKeys);

debug("Parsing successful:", filePath);
Copy link
Member

Choose a reason for hiding this comment

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

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().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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) ,

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @mdjermanovic, I applied your suggestions


return {
success: true,

Expand Down