Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit

Permalink
clang-tidy: added the ability to shard jobs
Browse files Browse the repository at this point in the history
  • Loading branch information
gaaclarke committed Nov 3, 2022
1 parent cdfd9d0 commit be68606
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 25 deletions.
106 changes: 91 additions & 15 deletions tools/clang_tidy/lib/clang_tidy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -112,9 +123,16 @@ class ClangTidy {
final List<dynamic> buildCommandsData = jsonDecode(
options.buildCommandsPath.readAsStringSync(),
) as List<dynamic>;
final List<Command> changedFileBuildCommands = await getLintCommandsForChangedFiles(
final List<List<dynamic>> shardBuildCommandsData = options
.shardCommandsPaths
.map((io.File file) =>
jsonDecode(file.readAsStringSync()) as List<dynamic>)
.toList();
final List<Command> changedFileBuildCommands = await getLintCommandsForFiles(
buildCommandsData,
changedFiles,
filesOfInterest,
shardBuildCommandsData,
options.shardId,
);

if (changedFileBuildCommands.isEmpty) {
Expand Down Expand Up @@ -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)
Expand All @@ -171,23 +189,81 @@ 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++;
}
}

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(
Future<List<Command>> getLintCommandsForFiles(
List<dynamic> buildCommandsData,
List<io.File> changedFiles,
List<io.File> files,
List<List<dynamic>> 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, dynamic>));
final Iterable<List<Command>> shardBuildCommands =
sharedBuildCommandsData.map((List<dynamic> list) => list
.map((dynamic data) =>
Command.fromMap(data as Map<String, dynamic>))
.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((dynamic data) => Command.fromMap(data as Map<String, dynamic>)));
}
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(
Expand Down
56 changes: 50 additions & 6 deletions tools/clang_tidy/lib/src/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -76,6 +80,8 @@ class Options {
fix: options['fix'] as bool,
errSink: errSink,
warningsAsErrors: _platformSpecificWarningsAsErrors(options),
shardCommandsPaths: shardCommandsPaths,
shardId: shardId,
);
}

Expand All @@ -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,
);
}

Expand All @@ -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 '
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
}
12 changes: 8 additions & 4 deletions tools/clang_tidy/test/clang_tidy_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});

Expand All @@ -205,7 +205,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, lessThan(300));
});

Expand All @@ -226,9 +226,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);
Expand All @@ -253,9 +255,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);
Expand Down

0 comments on commit be68606

Please sign in to comment.