mach lint takes 30 seconds on my machine to check a single file
Categories
(Developer Infrastructure :: Lint and Formatting, defect, P3)
Tracking
(Not tracked)
People
(Reporter: gregtatum, Unassigned)
References
(Depends on 1 open bug)
Details
(Whiteboard: [dev-prod-2020])
Attachments
(1 file)
~/dev/platform-gecko2 at 08:50:39
➤ mach lint --fix tools/profiler/tests/xpcshell/test_feature_mainthreadio.js
error: problem with lint setup, skipping android-api-lint, android-checkstyle, android-javadoc, android-lint, android-test
✖ 0 problems (0 errors, 0 warnings)
~/dev/platform-gecko2 at 08:51:06
Most of the time seems to be spent in this failed setup for android tools, which I get every time, even though I'm only checking javascript files. 30 seconds seems a bit excessive for a command I run frequently while developing, and is usually only operating on a few files that I have touched.
I thought perhaps I could fix this issue with installing those linters correctly, but there is no additional information on resolving this issue.
Running verbosely and with warnings also does not reveal more information.
➤ mach lint -v -W --fix tools/profiler/tests/xpcshell/test_feature_mainthreadio.js
error: problem with lint setup, skipping android-api-lint, android-checkstyle, android-javadoc, android-lint, android-test
✖ 0 problems (0 errors, 0 warnings)
|
||
Comment 1•5 years ago
|
||
Yeah, that's definitely not normal. The android setup issue is bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1571684. Though that bug is just about making the error disappear (as far as I know it shouldn't be slowing anything down).
I suspect that something else is going on. Is it just as slow if you run ./mach lint -l license tools/profiler/tests/xpcshell/test_feature_mainthreadio.js? If that's fast it implies one of the setup functions for one of the other lint implementations is taking a long time. We can try to narrow down which one. I should add some better logging to the setup step when verbose mode is enabled..
|
Reporter | |
Comment 2•5 years ago
|
||
That command executes quickly.
|
|
||
Comment 3•5 years ago
|
||
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
|
|
||
Comment 4•5 years ago
|
||
Attached patch adds a bit of logging around the setup functions, so when you run ./mach lint -v you'll see which linter is taking awhile to setup. Once we have that info we can dig into it some more.
|
|
||
Updated•5 years ago
|
|
|
Reporter | |
Comment 5•5 years ago
|
||
I tried working around the new black issue...
➤ ~/.mozbuild/_virtualenvs/mach/bin/pip install black
➤ mach lint -v tools/profiler/tests/xpcshell
15:40:39.627 mozlint (57021) | setup for l10n finished in 0.05 seconds
15:40:39.832 mozlint (57021) | setup for eslint finished in 0.2 seconds
15:40:39.834 mozlint (57021) | setup for android-javadoc finished in 0.0 seconds
15:40:39.835 mozlint (57021) | setup for android-api-lint finished in 0.0 seconds
15:40:45.990 mozlint (57021) | setup for pylint finished in 6.15 seconds
15:40:45.994 mozlint (57021) | setup for py3 finished in 0.0 seconds
15:40:45.995 mozlint (57021) | setup for android-test finished in 0.0 seconds
15:40:45.996 mozlint (57021) | setup for py2 finished in 0.0 seconds
15:40:46.810 mozlint (57021) | setup for codespell finished in 0.81 seconds
15:40:49.107 mozlint (57021) | setup for black finished in 2.3 seconds
15:40:49.108 mozlint (57021) | setup for android-checkstyle finished in 0.0 seconds
15:40:49.110 mozlint (57021) | setup for android-lint finished in 0.0 seconds
15:40:50.597 mozlint (57021) | setup for rst finished in 1.49 seconds
15:40:52.150 mozlint (57021) | setup for flake8 finished in 1.55 seconds
error: problem with lint setup, skipping android-api-lint, android-checkstyle, android-javadoc, android-lint, android-test
Traceback (most recent call last):
File "/Users/greg/dev/platform-gecko2/python/mozlint/mozlint/roller.py", line 64, in _run_worker
res = func(paths, config, **lintargs) or []
File "/Users/greg/dev/platform-gecko2/python/mozlint/mozlint/types.py", line 57, in __call__
return self._lint(paths, config, **lintargs)
File "/Users/greg/dev/platform-gecko2/python/mozlint/mozlint/types.py", line 148, in _lint
return func(files, config, **lintargs)
File "/Users/greg/dev/platform-gecko2/tools/lint/python/black.py", line 131, in lint
return run_black(config, files, fix=fix, log=lintargs["log"])
File "/Users/greg/dev/platform-gecko2/tools/lint/python/black.py", line 118, in run_black
log.debug("Black version {}".format(get_black_version(binary)))
File "/Users/greg/dev/platform-gecko2/tools/lint/python/black.py", line 51, in get_black_version
return re.match(r"black, version (.*)$", output)[1]
TypeError: 'NoneType' object is not subscriptable
A failure occurred in the black linter.
✖ 1 problem (0 errors, 0 warnings, 1 failure)
Glean could not be found, so telemetry will not be reported. You may need to run |mach bootstrap|.
|
|
||
Comment 6•5 years ago
|
||
Thanks for bearing through this.
So you said the slowness was before the error about Android lints, which I guess means we can at least continue debugging here.. black notwithstanding. So looks like pylint is the slowest, and a few others are over a second (which I'd also consider to be too slow). Looks like the total setup time is around 15s (which combined with other overhead and the actual linting itself), could definitely approach 30s.
Couple things we can do:
- Look into the slower setup functions and try to speed them up, here is the one for pylint:
https://searchfox.org/mozilla-central/source/tools/lint/python/pylint.py#50 (actually I'm surprised that's taking 6s.. I wonder if there's something up with pip.. for comparison I see it taking just under 1s). - Run setup functions in parallel (never bothered doing this initially since they were supposed to be super fast and the number of linters was much smaller back then).
|
||
Comment 8•5 years ago
|
||
| bugherder | ||
|
||
Comment 9•5 years ago
|
||
The severity field is not set for this bug.
:ahal, could you have a look please?
For more information, please visit auto_nag documentation.
|
|
||
Updated•5 years ago
|
|
|
||
Comment 10•5 years ago
|
||
Unfortunately I won't have time to get to this before impending leave.
|
|
||
Comment 11•4 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:ahal, maybe it's time to close this bug?
|
|
||
Comment 12•4 years ago
|
||
This is still valid, though I probably won't be able to prioritize it anytime soon.
|
|
||
Comment 13•4 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:ahal, maybe it's time to close this bug?
|
|
||
Updated•4 years ago
|
|
||
Updated•3 years ago
|
|
|
Reporter | |
Comment 15•3 years ago
|
||
This is still taking quite awhile, and is a regular productivity hit for me. Here are the current results, which take 15s.
➤ time ./mach lint --fix toolkit/components/translations/content/translations.mjs --verbose
09:55:55.208 mozlint (41080) | setup for l10n finished in 0.08 seconds
09:55:55.574 mozlint (41080) | setup for eslint finished in 0.37 seconds
09:55:55.577 mozlint (41080) | setup for android-javadoc finished in 0.0 seconds
09:55:55.578 mozlint (41080) | setup for clang-format finished in 0.0 seconds
09:55:55.579 mozlint (41080) | setup for android-api-lint finished in 0.0 seconds
09:55:57.387 mozlint (41080) | setup for pylint finished in 1.81 seconds
09:55:57.389 mozlint (41080) | setup for android-test finished in 0.0 seconds
09:55:58.735 mozlint (41080) | setup for codespell finished in 1.34 seconds
09:55:58.738 mozlint (41080) | Looking for black at /Users/greg/dev/gecko/obj-ff-release/_virtualenvs/lint/bin/black
09:55:58.924 mozlint (41080) | Black is present with expected version 21.11b1
09:55:58.924 mozlint (41080) | setup for black finished in 0.19 seconds
09:56:00.553 mozlint (41080) | setup for isort finished in 1.63 seconds
09:56:00.555 mozlint (41080) | setup for android-checkstyle finished in 0.0 seconds
09:56:00.556 mozlint (41080) | setup for android-format finished in 0.0 seconds
09:56:00.557 mozlint (41080) | setup for android-lint finished in 0.0 seconds
09:56:01.919 mozlint (41080) | setup for rst finished in 1.36 seconds
09:56:03.477 mozlint (41080) | setup for flake8 finished in 1.56 seconds
error: problem with lint setup, skipping android-api-lint, android-checkstyle, android-format, android-javadoc, android-lint, android-test
✖ 0 problems (0 errors, 0 warnings, 0 fixed)
real 0m15.430s
user 0m19.955s
sys 0m2.997s
|
|
Reporter | |
Comment 16•3 years ago
|
||
Also, for comparison here is fixing the outgoing changes, which I run on every patch I write, which is hitting the similar performance issues.
➤ time ./mach lint -wo --fix --verbose
09:59:34.504 mozlint (41373) | setup for l10n finished in 0.07 seconds
09:59:34.838 mozlint (41373) | setup for eslint finished in 0.33 seconds
09:59:34.840 mozlint (41373) | setup for android-javadoc finished in 0.0 seconds
09:59:34.842 mozlint (41373) | setup for clang-format finished in 0.0 seconds
09:59:34.843 mozlint (41373) | setup for android-api-lint finished in 0.0 seconds
09:59:36.572 mozlint (41373) | setup for pylint finished in 1.73 seconds
09:59:36.574 mozlint (41373) | setup for android-test finished in 0.0 seconds
09:59:37.886 mozlint (41373) | setup for codespell finished in 1.31 seconds
09:59:37.888 mozlint (41373) | Looking for black at /Users/greg/dev/gecko/obj-ff-release/_virtualenvs/lint/bin/black
09:59:38.65 mozlint (41373) | Black is present with expected version 21.11b1
09:59:38.66 mozlint (41373) | setup for black finished in 0.18 seconds
09:59:39.686 mozlint (41373) | setup for isort finished in 1.62 seconds
09:59:39.688 mozlint (41373) | setup for android-checkstyle finished in 0.0 seconds
09:59:39.689 mozlint (41373) | setup for android-format finished in 0.0 seconds
09:59:39.690 mozlint (41373) | setup for android-lint finished in 0.0 seconds
09:59:40.971 mozlint (41373) | setup for rst finished in 1.28 seconds
09:59:42.459 mozlint (41373) | setup for flake8 finished in 1.49 seconds
error: problem with lint setup, skipping android-api-lint, android-checkstyle, android-format, android-javadoc, android-lint, android-test
/Users/greg/dev/gecko/browser/locales-preview/translations.ftl
9:29 error Strings should use the corresponding terms instead of hard-coded brand names (Firefox) CO01 (fluent-lint)
/Users/greg/dev/gecko/toolkit/components/translations/content/bergamot-translator.js
1670:4 error Parsing error: Unexpected token (1670:4) (eslint)
✖ 2 problems (2 errors, 2 warnings, 0 fixed)
real 0m20.387s
user 1m47.606s
sys 0m8.073s
|
|
Reporter | |
Comment 17•3 years ago
|
||
A 3x speedup is to just run the single linter on a file:
➤ time ./mach lint --linter eslint toolkit/components/translations/content/translations.mjs --verbose
08:26:16.644 mozlint (55777) | setup for eslint finished in 0.35 seconds
✖ 0 problems (0 errors, 0 warnings, 0 fixed)
real 0m4.737s
user 0m5.343s
sys 0m0.807s
I aliased this to
alias meslint="./mach lint --linter eslint"
It at least helps with with fast iterations of running a single linter on a known issue.
|
||
Comment 18•3 years ago
|
||
(In reply to Greg Tatum [:gregtatum] from comment #17)
I aliased this to
You can also use ./mach eslint. There are some other linters that'll check mjs/js files, but code review bot will run those as well.
|
|
Reporter | |
Comment 19•3 years ago
|
||
Running multiple linters on a file makes sense that it could take some time, and will take a certain amount of time to just run different linters, which is probably out of our control. Looking at the timing above in comment 15, it's taking 8.3 seconds to setup the linters and 7.2 seconds to run them. I would assume that much of that 8.3 seconds could be optimized away, unless I'm missing something. I would assume the setup has a lot of redundant code that only needs to be initialized once (although it's been awhile since I've read through the linter implementation).
Description
•