clang-tidy: added the ability to shard jobs

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,86 @@ class ClangTidy {
     return repo.changedFiles;
   }
 
+  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 all [sets],
+  /// otherwise `Difference`.
+  Iterable<_SetStatusCommand> _calcIntersection(
+      Iterable<Command> items, Iterable<List<Command>> sets) sync* {
+    bool allSetsContain(Command command) {
+      for (final List<Command> set in sets) {
+        final Iterable<String> filePaths = set.map((Command e) => e.filePath);
+        if (!filePaths.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, and the files with local changes,
   /// compute the lint commands to run.
   @visibleForTesting
-  Future<List<Command>> getLintCommandsForChangedFiles(
-    List<dynamic> buildCommandsData,
-    List<io.File> changedFiles,
+  Future<List<Command>> getLintCommandsForFiles(
+    List<Object?> buildCommandsData,
+    List<io.File> files,
+    List<List<Object?>> sharedBuildCommandsData,
+    int? shardId,
   ) 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);
+    final List<Command> totalCommands = <Command>[];
+    if (sharedBuildCommandsData.isNotEmpty) {
+      final Iterable<Command> buildCommands = buildCommandsData
+          .map((dynamic data) => Command.fromMap(data as Map<String, Object?>));
+      final Iterable<List<Command>> shardBuildCommands =
+          sharedBuildCommandsData.map((List<Object?> list) => list
+              .map((Object? data) =>
+                  Command.fromMap((data as Map<String, Object?>?)!))
+              .toList());
+      final Iterable<_SetStatusCommand> intersectionResults =
+          _calcIntersection(buildCommands, shardBuildCommands);
+      totalCommands.addAll(intersectionResults
+          .where((_SetStatusCommand element) =>
+              element.setStatus == _SetStatus.Difference)
+          .map((_SetStatusCommand e) => e.command));
+      final List<Command> intersection = intersectionResults
+          .where((_SetStatusCommand element) =>
+              element.setStatus == _SetStatus.Intersection)
+          .map((_SetStatusCommand e) => e.command)
+          .toList();
+      // Make sure to sort results, 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 + shardBuildCommands.length));
+    } else {
+      totalCommands.addAll(buildCommandsData.map((Object? data) => Command.fromMap((data as Map<String, Object?>?)!)));
+    }
+    Stream<Command> filterCommands() async* {
+      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)) {
+          yield command;
+        }
       }
     }
-    return buildCommands;
+    return filterCommands().toList();
   }
 
   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;
   }
 }

tools/clang_tidy/test/clang_tidy_test.dart

@@ -193,7 +193,7 @@ Future<int> main(List<String> args) async {
       outSink: outBuffer,
       errSink: errBuffer,
     );
-    final List<io.File> fileList = await clangTidy.computeChangedFiles();
+    final List<io.File> fileList = await clangTidy.computeFilesOfInterest();
     expect(fileList.length, greaterThan(1000));
   });
 
@@ -205,10 +205,77 @@ Future<int> main(List<String> args) async {
       outSink: outBuffer,
       errSink: errBuffer,
     );
-    final List<io.File> fileList = await clangTidy.computeChangedFiles();
+    final List<io.File> fileList = await clangTidy.computeFilesOfInterest();
     expect(fileList.length, lessThan(300));
   });
 
+  test('Sharding', () async {
+    final StringBuffer outBuffer = StringBuffer();
+    final StringBuffer errBuffer = StringBuffer();
+    final ClangTidy clangTidy = ClangTidy(
+      buildCommandsPath: io.File(buildCommands),
+      lintAll: true,
+      outSink: outBuffer,
+      errSink: errBuffer,
+    );
+    Map<String, dynamic> makeBuildCommandEntry(String filePath) => <String, dynamic>{
+          'directory': '/unused',
+          'command': '../../buildtools/mac-x64/clang/bin/clang $filePath',
+          'file': filePath,
+        };
+    final List<String> filePaths = () sync* {
+      for (int i = 0; i < 10; ++i) {
+        yield '/path/to/a/source_file_$i.cc';
+      }
+    }().toList();
+    final List<dynamic> buildCommandsData =
+        filePaths.map((String e) => makeBuildCommandEntry(e)).toList();
+    final List<dynamic> shardBuildCommandsData =
+      filePaths.sublist(6).map((String e) => makeBuildCommandEntry(e)).toList();
+
+    {
+      final List<Command> commands = await clangTidy.getLintCommandsForFiles(
+        buildCommandsData,
+        filePaths.map((String e) => io.File(e)).toList(),
+        <List<dynamic>>[shardBuildCommandsData],
+        0,
+      );
+      final Iterable<String> commandFilePaths = commands.map((Command e) => e.filePath);
+      expect(commands.length, equals(8));
+      expect(commandFilePaths.contains('/path/to/a/source_file_0.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_1.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_2.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_3.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_4.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_5.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_6.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_7.cc'), false);
+      expect(commandFilePaths.contains('/path/to/a/source_file_8.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_9.cc'), false);
+    }
+    {
+      final List<Command> commands = await clangTidy.getLintCommandsForFiles(
+        buildCommandsData,
+        filePaths.map((String e) => io.File(e)).toList(),
+        <List<dynamic>>[shardBuildCommandsData],
+        1,
+      );
+
+      final Iterable<String> commandFilePaths = commands.map((Command e) => e.filePath);
+      expect(commands.length, equals(8));
+      expect(commandFilePaths.contains('/path/to/a/source_file_0.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_1.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_2.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_3.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_4.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_5.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_6.cc'), false);
+      expect(commandFilePaths.contains('/path/to/a/source_file_7.cc'), true);
+      expect(commandFilePaths.contains('/path/to/a/source_file_8.cc'), false);
+      expect(commandFilePaths.contains('/path/to/a/source_file_9.cc'), true);
+    }
+  });
+
   test('No Commands are produced when no files changed', () async {
     final StringBuffer outBuffer = StringBuffer();
     final StringBuffer errBuffer = StringBuffer();
@@ -226,9 +293,11 @@ Future<int> main(List<String> args) async {
         'file': filePath,
       },
     ];
-    final List<Command> commands = await clangTidy.getLintCommandsForChangedFiles(
+    final List<Command> commands = await clangTidy.getLintCommandsForFiles(
       buildCommandsData,
       <io.File>[],
+      <List<dynamic>>[],
+      null,
     );
 
     expect(commands, isEmpty);
@@ -253,9 +322,11 @@ Future<int> main(List<String> args) async {
         'file': filePath,
       },
     ];
-    final List<Command> commands = await clangTidy.getLintCommandsForChangedFiles(
+    final List<Command> commands = await clangTidy.getLintCommandsForFiles(
       buildCommandsData,
       <io.File>[io.File(filePath)],
+      <List<dynamic>>[],
+      null,
     );
 
     expect(commands, isNotEmpty);