This makes the lint script use multiprocessing to speed it up. #19987

gspencergoog · 2020-07-24T00:36:49Z

Description

I got tired of waiting for it to run, so I added some of the "worker" queue code that I wrote for the assets-for-api-docs generator.

I also tried out putting all the files in one call to clang-tidy with the -p argument, but that was still a lot slower because it runs them all on one core. This runs separate jobs for each file, simultaneously, and then reports the results at the end (associated with each file, of course).

flutter-dashboard · 2020-07-24T00:36:52Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

gaaclarke · 2020-07-24T01:15:19Z

I'll have to give this a bit closer read tomorrow. At first blush I think you should pull the work you did here into a package for pub. I did something similar here: https://pub.dev/packages/pmap It spawns isolates to perform tasks, we should have something similar for subprocesses like you've done.

gaaclarke

Thanks for all the cleanup and the multiprocessing. The plan was to eventually execute clang-tidy once for all the files which has a -j argument. Once I added the FLUTTER_NOLINT option that wasn't possible anymore, so this is great.

I still think the ProcessPool would be a great package but if you don't want to do clean it up for that I think you should at least put it in its own file and add a simple unit test. That will keep the linter code easier to follow and someone could clean it up for a package later.

gaaclarke · 2020-07-24T17:35:22Z

ci/lint.dart

+    return jobDone.future;
+  }
+
+  Future<Map<WorkerJob, ProcessRunnerResult>> startWorkers(


nit: This would be more efficient and robust if it returned Stream<Tuple<WorkerJob, ProcessRunnerResult>>.

Cool, thanks for the suggestion, I'll do that.

gspencergoog · 2020-07-24T18:04:00Z

Yeah, I agree: I've already started the process of releasing it in a separate google/ github repo, since it's not Flutter specific, and I don't want to put it into the packages repo.

gspencergoog · 2020-07-24T19:21:47Z

Actually, there's one consideration: I modified this to not use package/process.dart, since this script doesn't have a pubspec.yaml to add the dependency to, but I actually think that the published version is more generally useful if it does: you can mock it for tests, etc.

Should I make this into a "real" dart app with a pubspec.yaml?

gaaclarke · 2020-07-24T20:16:30Z

Should I make this into a "real" dart app with a pubspec.yaml?

Yea, sounds like that makes the most sense. Probably would have happened eventually anyways.

gspencergoog · 2020-07-27T19:17:50Z

OK, I'm in the process of releasing the process_runner package (have to get some approvals). Once that is done, I'll update this with that dependency and we can move forward. Until then, I'll switch this back to a "draft" PR.

gspencergoog · 2020-07-29T02:16:02Z

OK, the package is released: https://pub.dev/packages/process_runner

I'll incorporate it into the script.

gaaclarke

Nice upgrade across the board. Thanks.

gaaclarke · 2020-07-29T20:53:14Z

ci/bin/lint.dart

+  verbose = options['verbose'] as bool;
+
+  if (options['help'] as bool) {
+    _usage(parser);


Shouldn't execution quit after _usage?

Yes, and it does: _usage exits.

gaaclarke · 2020-07-29T20:56:14Z

I'd use led to fire off a few builds before landing this to make sure it doesn't break CI.

flutter/engine#19987)

* 4521162 Roll Skia from 39d1c1ebb607 to 3a608e5bc9d5 (1 revision) (flutter/engine#20101) * b265bd2 Roll Skia from 3a608e5bc9d5 to 620bfa3fffba (1 revision) (flutter/engine#20102) * 36c5f60 Roll Skia from 620bfa3fffba to 4866d0ad5f3c (1 revision) (flutter/engine#20104) * 06fef5e Enable lazy-async-stacks by-default in all modes (2) (flutter/engine#19270) * d71b11f Roll Fuchsia Linux SDK from ETPOTPepP... to ROYgzKMaF... (flutter/engine#20105) * 42e4ebf Roll Skia from 4866d0ad5f3c to c34efe0da102 (1 revision) (flutter/engine#20108) * 3a65b1a Roll Skia from c34efe0da102 to 8c7ecc1c06f4 (1 revision) (flutter/engine#20109) * ad99f5e Roll Skia from 8c7ecc1c06f4 to 4f587337c306 (1 revision) (flutter/engine#20110) * f0cc38f [web] Set correct defaults for text in canvas (flutter/engine#20067) * 02f9ed9 Roll Fuchsia Mac SDK from 5dM55hp8B... to hzo88TZzN... (flutter/engine#20113) * 34389f5 Roll Skia from 4f587337c306 to 96d6c6f04dcb (4 revisions) (flutter/engine#20114) * 49a40fa Enabled linting on engine.cc (flutter/engine#19981) * b955e15 Manual roll of Dart from 24c7666def...40fd1c456e (flutter/engine#20092) * 145ef60 Remove confusing logs (flutter/engine#20121) * 8fc3926 Roll Skia from 96d6c6f04dcb to 57d859edd3c4 (16 revisions) (flutter/engine#20126) * ec9e17c Roll zlib (flutter/engine#20119) * f288fe5 [web] Enable canvas measurement by default (flutter/engine#19924) * 8464208 Add missing MouseCursorPlugin destroy call (flutter/engine#19968) * 5498add Roll Skia from 57d859edd3c4 to 994ce8cf2300 (1 revision) (flutter/engine#20129) * 9398bc4 Roll Skia from 994ce8cf2300 to 398c654ce7be (2 revisions) (flutter/engine#20133) * d3bc43e Roll Fuchsia Linux SDK from ROYgzKMaF... to d4pESQYnB... (flutter/engine#20132) * 4068918 Manual roll of Dart 40fd1c456e...7e8348f4ce (flutter/engine#20125) * 4c49e0b Manual roll of Dart cb6ed67a73...7e8348f4ce (flutter/engine#20135) * 4a3688e Roll Skia from 398c654ce7be to a4bbc9d8ec4f (1 revision) (flutter/engine#20136) * b3c6fd3 Roll Fuchsia Mac SDK from hzo88TZzN... to 3XwiR_wVO... (flutter/engine#20137) * adb5986 Manual roll of Dart 03e4737f31...cb6ed67a73 (flutter/engine#20138) * 941c442 Add ALERT SoundType enum value (flutter/engine#20139) * 19368ef Fix dartdocs of dart:ui (flutter/engine#20140) * 9dd3d2e Roll Skia from a4bbc9d8ec4f to 94cefeff50d2 (1 revision) (flutter/engine#20141) * 0fdb141 Roll Dart SDK from 03e4737f3115 to 59600f2b46ef (54 revisions) (flutter/engine#20143) * 97d6ee2 Roll Skia from 94cefeff50d2 to 5ba6534884d9 (2 revisions) (flutter/engine#20146) * da3d495 Roll Dart SDK from 59600f2b46ef to 04f4272546af (5 revisions) (flutter/engine#20147) * 8229df8 Roll Skia from 5ba6534884d9 to e393c61a1563 (1 revision) (flutter/engine#20148) * 8e1d48e Migrate some Dart_WeakPersistentHandle uses to Dart_FinalizableHandle (flutter/engine#20107) * 27b61e7 Roll ANGLE to pick up warning fixes (flutter/engine#19105) * 146d504 Roll Skia from e393c61a1563 to 3136789972ea (5 revisions) (flutter/engine#20150) * 841b391 Roll Dart SDK from 04f4272546af to e87cb96bb89c (7 revisions) (flutter/engine#20152) * e9334c9 Roll Skia from 3136789972ea to 5f2b2d6dc691 (5 revisions) (flutter/engine#20153) * f2b02d8 [iOS] Fixes text input plugin crash (flutter/engine#20127) * 3ed5893 Roll Skia from 5f2b2d6dc691 to 8cc118dce813 (2 revisions) (flutter/engine#20154) * 7c5162e Roll Fuchsia Mac SDK from 3XwiR_wVO... to T2xc0OuiK... (flutter/engine#20155) * e23e477 Lint fixes for fml, tools subdirs (flutter/engine#19990) * f620eac Roll Dart SDK from e87cb96bb89c to bd528bfbd69d (8 revisions) (flutter/engine#20158) * e1c9673 Fix targets in build_fuchsia_artifacts (flutter/engine#19794) * c134e16 add information collection for safari bots (flutter/engine#20123) * ee4d50c Revert "Enable lazy-async-stacks by-default in all modes (2) (#19270)" (flutter/engine#20165) * 357b155 Roll Fuchsia Linux SDK from d4pESQYnB... to d_5wDVmBd... (flutter/engine#20161) * a6cd3eb Fix lint errors in lib/ui (flutter/engine#19988) * 280bbfc This makes the lint script use multiprocessing to speed it up. (flutter/engine#19987)

* 4521162 Roll Skia from 39d1c1ebb607 to 3a608e5bc9d5 (1 revision) (flutter/engine#20101) * b265bd2 Roll Skia from 3a608e5bc9d5 to 620bfa3fffba (1 revision) (flutter/engine#20102) * 36c5f60 Roll Skia from 620bfa3fffba to 4866d0ad5f3c (1 revision) (flutter/engine#20104) * 06fef5e Enable lazy-async-stacks by-default in all modes (2) (flutter/engine#19270) * d71b11f Roll Fuchsia Linux SDK from ETPOTPepP... to ROYgzKMaF... (flutter/engine#20105) * 42e4ebf Roll Skia from 4866d0ad5f3c to c34efe0da102 (1 revision) (flutter/engine#20108) * 3a65b1a Roll Skia from c34efe0da102 to 8c7ecc1c06f4 (1 revision) (flutter/engine#20109) * ad99f5e Roll Skia from 8c7ecc1c06f4 to 4f587337c306 (1 revision) (flutter/engine#20110) * f0cc38f [web] Set correct defaults for text in canvas (flutter/engine#20067) * 02f9ed9 Roll Fuchsia Mac SDK from 5dM55hp8B... to hzo88TZzN... (flutter/engine#20113) * 34389f5 Roll Skia from 4f587337c306 to 96d6c6f04dcb (4 revisions) (flutter/engine#20114) * 49a40fa Enabled linting on engine.cc (flutter/engine#19981) * b955e15 Manual roll of Dart from 24c7666def...40fd1c456e (flutter/engine#20092) * 145ef60 Remove confusing logs (flutter/engine#20121) * 8fc3926 Roll Skia from 96d6c6f04dcb to 57d859edd3c4 (16 revisions) (flutter/engine#20126) * ec9e17c Roll zlib (flutter/engine#20119) * f288fe5 [web] Enable canvas measurement by default (flutter/engine#19924) * 8464208 Add missing MouseCursorPlugin destroy call (flutter/engine#19968) * 5498add Roll Skia from 57d859edd3c4 to 994ce8cf2300 (1 revision) (flutter/engine#20129) * 9398bc4 Roll Skia from 994ce8cf2300 to 398c654ce7be (2 revisions) (flutter/engine#20133) * d3bc43e Roll Fuchsia Linux SDK from ROYgzKMaF... to d4pESQYnB... (flutter/engine#20132) * 4068918 Manual roll of Dart 40fd1c456e...7e8348f4ce (flutter/engine#20125) * 4c49e0b Manual roll of Dart cb6ed67a73...7e8348f4ce (flutter/engine#20135) * 4a3688e Roll Skia from 398c654ce7be to a4bbc9d8ec4f (1 revision) (flutter/engine#20136) * b3c6fd3 Roll Fuchsia Mac SDK from hzo88TZzN... to 3XwiR_wVO... (flutter/engine#20137) * adb5986 Manual roll of Dart 03e4737f31...cb6ed67a73 (flutter/engine#20138) * 941c442 Add ALERT SoundType enum value (flutter/engine#20139) * 19368ef Fix dartdocs of dart:ui (flutter/engine#20140) * 9dd3d2e Roll Skia from a4bbc9d8ec4f to 94cefeff50d2 (1 revision) (flutter/engine#20141) * 0fdb141 Roll Dart SDK from 03e4737f3115 to 59600f2b46ef (54 revisions) (flutter/engine#20143) * 97d6ee2 Roll Skia from 94cefeff50d2 to 5ba6534884d9 (2 revisions) (flutter/engine#20146) * da3d495 Roll Dart SDK from 59600f2b46ef to 04f4272546af (5 revisions) (flutter/engine#20147) * 8229df8 Roll Skia from 5ba6534884d9 to e393c61a1563 (1 revision) (flutter/engine#20148) * 8e1d48e Migrate some Dart_WeakPersistentHandle uses to Dart_FinalizableHandle (flutter/engine#20107) * 27b61e7 Roll ANGLE to pick up warning fixes (flutter/engine#19105) * 146d504 Roll Skia from e393c61a1563 to 3136789972ea (5 revisions) (flutter/engine#20150) * 841b391 Roll Dart SDK from 04f4272546af to e87cb96bb89c (7 revisions) (flutter/engine#20152) * e9334c9 Roll Skia from 3136789972ea to 5f2b2d6dc691 (5 revisions) (flutter/engine#20153) * f2b02d8 [iOS] Fixes text input plugin crash (flutter/engine#20127) * 3ed5893 Roll Skia from 5f2b2d6dc691 to 8cc118dce813 (2 revisions) (flutter/engine#20154) * 7c5162e Roll Fuchsia Mac SDK from 3XwiR_wVO... to T2xc0OuiK... (flutter/engine#20155) * e23e477 Lint fixes for fml, tools subdirs (flutter/engine#19990) * f620eac Roll Dart SDK from e87cb96bb89c to bd528bfbd69d (8 revisions) (flutter/engine#20158) * e1c9673 Fix targets in build_fuchsia_artifacts (flutter/engine#19794) * c134e16 add information collection for safari bots (flutter/engine#20123) * ee4d50c Revert "Enable lazy-async-stacks by-default in all modes (2) (flutter#19270)" (flutter/engine#20165) * 357b155 Roll Fuchsia Linux SDK from d4pESQYnB... to d_5wDVmBd... (flutter/engine#20161) * a6cd3eb Fix lint errors in lib/ui (flutter/engine#19988) * 280bbfc This makes the lint script use multiprocessing to speed it up. (flutter/engine#19987)

googlebot added the cla: yes label Jul 24, 2020

gspencergoog requested a review from gaaclarke July 24, 2020 00:36

gspencergoog force-pushed the lint_multiprocess branch from bd90608 to 94a271c Compare July 24, 2020 00:45

gspencergoog force-pushed the lint_multiprocess branch 2 times, most recently from cc1bfec to 2c14b3a Compare July 24, 2020 01:28

gaaclarke suggested changes Jul 24, 2020

View reviewed changes

gspencergoog marked this pull request as draft July 27, 2020 19:17

gspencergoog added the Work in progress (WIP) Not ready (yet) for review! label Jul 27, 2020

gspencergoog force-pushed the lint_multiprocess branch 3 times, most recently from 0ef3158 to 7edd218 Compare July 28, 2020 19:11

gspencergoog force-pushed the lint_multiprocess branch from 7edd218 to ef5c596 Compare July 29, 2020 02:24

gaaclarke approved these changes Jul 29, 2020

View reviewed changes

gspencergoog added 3 commits July 30, 2020 12:44

Make lint script use multiprocessing

038aeeb

Revert null safety, add command line flags.

b67b6fe

Switch to use Directory and File primitives

6f26771

gspencergoog force-pushed the lint_multiprocess branch 2 times, most recently from 09e3464 to 6daaad6 Compare July 30, 2020 20:42

gspencergoog marked this pull request as ready for review July 30, 2020 21:23

gspencergoog removed the Work in progress (WIP) Not ready (yet) for review! label Jul 30, 2020

auto-assign bot requested a review from iskakaushik July 30, 2020 21:27

gspencergoog force-pushed the lint_multiprocess branch from 6daaad6 to f47fc24 Compare July 30, 2020 21:28

Convert to using process_runner package

d403a80

gspencergoog force-pushed the lint_multiprocess branch from f47fc24 to d403a80 Compare July 30, 2020 21:33

gspencergoog merged commit 280bbfc into flutter:master Jul 31, 2020

gspencergoog deleted the lint_multiprocess branch July 31, 2020 03:23

engine-flutter-autoroll mentioned this pull request Jul 31, 2020

Roll Engine from a9910e409c02 to 280bbfc763cf (49 revisions) flutter/flutter#62647

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 31, 2020

280bbfc This makes the lint script use multiprocessing to speed it up. (

bdbe649

flutter/engine#19987)

This makes the lint script use multiprocessing to speed it up. #19987

This makes the lint script use multiprocessing to speed it up. #19987

Uh oh!

Conversation

gspencergoog commented Jul 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

flutter-dashboard bot commented Jul 24, 2020

Uh oh!

gaaclarke commented Jul 24, 2020

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jul 24, 2020

Choose a reason for hiding this comment

Uh oh!

gspencergoog Jul 24, 2020

Choose a reason for hiding this comment

Uh oh!

gspencergoog commented Jul 24, 2020

Uh oh!

gspencergoog commented Jul 24, 2020

Uh oh!

gaaclarke commented Jul 24, 2020

Uh oh!

gspencergoog commented Jul 27, 2020

Uh oh!

gspencergoog commented Jul 29, 2020

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jul 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gspencergoog Jul 30, 2020

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Jul 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gspencergoog commented Jul 24, 2020 •

edited

Loading

gaaclarke Jul 29, 2020 •

edited

Loading