clang-tidy: added the ability to shard jobs (#37265)

* clang-tidy: added the ability to shard jobs

* added test

* jenn feedback

* hack ci to run as a shard to measure the time

* tweak

* fix hack

* zach feedback

* zach feedback 2

* removed stray async

* moved to using sets for lookups

* fixed typo in docstring

* Revert "fix hack"

This reverts commit 06a61a6.

Revert "tweak"

This reverts commit e7c58b1.

Revert "hack ci to run as a shard to measure the time"

This reverts commit e458963.

* removed calls to map

* turned the ci hack back on

* Revert "turned the ci hack back on"

This reverts commit 0d53794.

* removed sync*

Author: gaaclarke

tools/clang_tidy/lib/clang_tidy.dart

@@ -29,6 +29,17 @@ class _ComputeJobsResult {
   final bool sawMalformed;
 }
 
+enum _SetStatus {
+  Intersection,
+  Difference,
+}
+
+class _SetStatusCommand {
+  _SetStatusCommand(this.setStatus, this.command);
+  final _SetStatus setStatus;
+  final Command command;
+}
+
 /// A class that runs clang-tidy on all or only the changed files in a git
 /// repo.
 class ClangTidy {
@@ -92,14 +103,14 @@ class ClangTidy {
 
     _outSink.writeln(_linterOutputHeader);
 
-    final List<io.File> changedFiles = await computeChangedFiles();
+    final List<io.File> filesOfInterest = await computeFilesOfInterest();
 
     if (options.verbose) {
       _outSink.writeln('Checking lint in repo at ${options.repoPath.path}.');
       if (options.checksArg.isNotEmpty) {
         _outSink.writeln('Checking for specific checks: ${options.checks}.');
       }
-      final int changedFilesCount = changedFiles.length;
+      final int changedFilesCount = filesOfInterest.length;
       if (options.lintAll) {
         _outSink.writeln('Checking all $changedFilesCount files the repo dir.');
       } else {
@@ -109,12 +120,19 @@ class ClangTidy {
       }
     }
 
-    final List<dynamic> buildCommandsData = jsonDecode(
+    final List<Object?> buildCommandsData = jsonDecode(
       options.buildCommandsPath.readAsStringSync(),
-    ) as List<dynamic>;
-    final List<Command> changedFileBuildCommands = await getLintCommandsForChangedFiles(
+    ) as List<Object?>;
+    final List<List<Object?>> shardBuildCommandsData = options
+        .shardCommandsPaths
+        .map((io.File file) =>
+            jsonDecode(file.readAsStringSync()) as List<Object?>)
+        .toList();
+    final List<Command> changedFileBuildCommands = await getLintCommandsForFiles(
       buildCommandsData,
-      changedFiles,
+      filesOfInterest,
+      shardBuildCommandsData,
+      options.shardId,
     );
 
     if (changedFileBuildCommands.isEmpty) {
@@ -153,7 +171,7 @@ class ClangTidy {
   /// The files with local modifications or all the files if `lintAll` was
   /// specified.
   @visibleForTesting
-  Future<List<io.File>> computeChangedFiles() async {
+  Future<List<io.File>> computeFilesOfInterest() async {
     if (options.lintAll) {
       return options.repoPath
         .listSync(recursive: true)
@@ -171,23 +189,99 @@ class ClangTidy {
     return repo.changedFiles;
   }
 
-  /// Given a build commands json file, and the files with local changes,
-  /// compute the lint commands to run.
+  /// Returns f(n) = value(n * [shardCount] + [id]).
+  Iterable<T> _takeShard<T>(Iterable<T> values, int id, int shardCount) sync* {
+    int count = 0;
+    for (final T val in values) {
+      if (count % shardCount == id) {
+        yield val;
+      }
+      count++;
+    }
+  }
+
+  /// This returns a `_SetStatusCommand` for each [Command] in [items].
+  /// `Intersection` if the Command shows up in [items] and its filePath in all
+  /// [filePathSets], otherwise `Difference`.
+  Iterable<_SetStatusCommand> _calcIntersection(
+      Iterable<Command> items, Iterable<Set<String>> filePathSets) sync* {
+    bool allSetsContain(Command command) {
+      for (final Set<String> filePathSet in filePathSets) {
+        if (!filePathSet.contains(command.filePath)) {
+          return false;
+        }
+      }
+      return true;
+    }
+    for (final Command command in items) {
+      if (allSetsContain(command)) {
+        yield _SetStatusCommand(_SetStatus.Intersection, command);
+      } else {
+        yield _SetStatusCommand(_SetStatus.Difference, command);
+      }
+    }
+  }
+
+  /// Given a build commands json file's contents in [buildCommandsData], and
+  /// the [files] with local changes, compute the lint commands to run.  If
+  /// build commands are supplied in [sharedBuildCommandsData] the intersection
+  /// of those build commands will be calculated and distributed across
+  /// instances via the [shardId].
   @visibleForTesting
-  Future<List<Command>> getLintCommandsForChangedFiles(
-    List<dynamic> buildCommandsData,
-    List<io.File> changedFiles,
-  ) async {
-    final List<Command> buildCommands = <Command>[];
-    for (final dynamic data in buildCommandsData) {
-      final Command command = Command.fromMap(data as Map<String, dynamic>);
-      final LintAction lintAction = await command.lintAction;
-      // Short-circuit the expensive containsAny call for the many third_party files.
-      if (lintAction != LintAction.skipThirdParty && command.containsAny(changedFiles)) {
-        buildCommands.add(command);
+  Future<List<Command>> getLintCommandsForFiles(
+    List<Object?> buildCommandsData,
+    List<io.File> files,
+    List<List<Object?>> sharedBuildCommandsData,
+    int? shardId,
+  ) {
+    final List<Command> totalCommands = <Command>[];
+    if (sharedBuildCommandsData.isNotEmpty) {
+      final List<Command> buildCommands = <Command>[
+        for (Object? data in buildCommandsData)
+          Command.fromMap((data as Map<String, Object?>?)!)
+      ];
+      final List<Set<String>> shardFilePaths = <Set<String>>[
+        for (List<Object?> list in sharedBuildCommandsData)
+          <String>{
+            for (Object? data in list)
+              Command.fromMap((data as Map<String, Object?>?)!).filePath
+          }
+      ];
+      final Iterable<_SetStatusCommand> intersectionResults =
+          _calcIntersection(buildCommands, shardFilePaths);
+      for (final _SetStatusCommand result in intersectionResults) {
+        if (result.setStatus == _SetStatus.Difference) {
+          totalCommands.add(result.command);
+        }
       }
+      final List<Command> intersection = <Command>[
+        for (_SetStatusCommand result in intersectionResults)
+          if (result.setStatus == _SetStatus.Intersection) result.command
+      ];
+      // Make sure to sort results so the sharding scheme is guaranteed to work
+      // since we are not sure if there is a defined order in the json file.
+      intersection
+          .sort((Command x, Command y) => x.filePath.compareTo(y.filePath));
+      totalCommands.addAll(
+          _takeShard(intersection, shardId!, 1 + sharedBuildCommandsData.length));
+    } else {
+      totalCommands.addAll(<Command>[
+        for (Object? data in buildCommandsData)
+          Command.fromMap((data as Map<String, Object?>?)!)
+      ]);
     }
-    return buildCommands;
+    return () async {
+      final List<Command> result = <Command>[];
+      for (final Command command in totalCommands) {
+        final LintAction lintAction = await command.lintAction;
+        // Short-circuit the expensive containsAny call for the many third_party files.
+        if (lintAction != LintAction.skipThirdParty &&
+            command.containsAny(files)) {
+          result.add(command);
+        }
+      }
+      return result;
+    }();
   }
 
   Future<_ComputeJobsResult> _computeJobs(

tools/clang_tidy/lib/src/options.dart

@@ -34,6 +34,8 @@ class Options {
     this.fix = false,
     this.errorMessage,
     this.warningsAsErrors,
+    this.shardId,
+    this.shardCommandsPaths = const <io.File>[],
     StringSink? errSink,
   }) : checks = checksArg.isNotEmpty ? '--checks=$checksArg' : null,
        _errSink = errSink ?? io.stderr;
@@ -64,6 +66,8 @@ class Options {
     ArgResults options, {
     required io.File buildCommandsPath,
     StringSink? errSink,
+    required List<io.File> shardCommandsPaths,
+    int? shardId,
   }) {
     return Options(
       help: options['help'] as bool,
@@ -76,6 +80,8 @@ class Options {
       fix: options['fix'] as bool,
       errSink: errSink,
       warningsAsErrors: _platformSpecificWarningsAsErrors(options),
+      shardCommandsPaths: shardCommandsPaths,
+      shardId: shardId,
     );
   }
 
@@ -87,25 +93,42 @@ class Options {
     final ArgResults argResults = _argParser.parse(arguments);
 
     String? buildCommandsPath = argResults['compile-commands'] as String?;
+
+    String variantToBuildCommandsFilePath(String variant) =>
+      path.join(
+        argResults['src-dir'] as String,
+        'out',
+        variant,
+        'compile_commands.json',
+      );
     // path/to/engine/src/out/variant/compile_commands.json
-    buildCommandsPath ??= path.join(
-      argResults['src-dir'] as String,
-      'out',
-      argResults['target-variant'] as String,
-      'compile_commands.json',
-    );
+    buildCommandsPath ??= variantToBuildCommandsFilePath(argResults['target-variant'] as String);
     final io.File buildCommands = io.File(buildCommandsPath);
+    final List<io.File> shardCommands =
+        (argResults['shard-variants'] as String? ?? '')
+            .split(',')
+            .where((String element) => element.isNotEmpty)
+            .map((String variant) =>
+                io.File(variantToBuildCommandsFilePath(variant)))
+            .toList();
     final String? message = _checkArguments(argResults, buildCommands);
     if (message != null) {
       return Options._error(message, errSink: errSink);
     }
     if (argResults['help'] as bool) {
       return Options._help(errSink: errSink);
     }
+    final String? shardIdString = argResults['shard-id'] as String?;
+    final int? shardId = shardIdString == null ? null : int.parse(shardIdString);
+    if (shardId != null && (shardId >= shardCommands.length || shardId < 0)) {
+      return Options._error('Invalid shard-id value: $shardId.', errSink: errSink);
+    }
     return Options._fromArgResults(
       argResults,
       buildCommandsPath: buildCommands,
       errSink: errSink,
+      shardCommandsPaths: shardCommands,
+      shardId: shardId,
     );
   }
 
@@ -132,6 +155,17 @@ class Options {
       'verbose',
       help: 'Print verbose output.',
     )
+    ..addOption(
+      'shard-id',
+      help: 'When used with the shard-commands option this identifies which shard will execute.',
+      valueHelp: 'A number less than 1 + the number of shard-commands arguments.',
+    )
+    ..addOption(
+      'shard-variants',
+      help: 'Comma separated list of other targets, this invocation '
+            'will only execute a subset of the intersection and the difference of the '
+            'compile commands. Use with `shard-id`.'
+    )
     ..addOption(
       'compile-commands',
       help: 'Use the given path as the source of compile_commands.json. This '
@@ -171,6 +205,12 @@ class Options {
   /// The location of the compile_commands.json file.
   final io.File buildCommandsPath;
 
+  /// The location of shard compile_commands.json files.
+  final List<io.File> shardCommandsPaths;
+
+  /// The identifier of the shard.
+  final int? shardId;
+
   /// The root of the flutter/engine repository.
   final io.Directory repoPath = io.Directory(_engineRoot);
 
@@ -233,6 +273,10 @@ class Options {
       return "ERROR: Build commands path ${buildCommandsPath.absolute.path} doesn't exist.";
     }
 
+    if (argResults.wasParsed('shard-variants') && !argResults.wasParsed('shard-id')) {
+      return 'ERROR: a `shard-id` must be specified with `shard-variants`.';
+    }
+
     return null;
   }
 }