Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce rebuild times when invoking 'et run' #52883

Merged
merged 1 commit into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions tools/engine_tool/lib/src/commands/run_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:engine_build_configs/engine_build_configs.dart';
import 'package:process_runner/process_runner.dart';

import '../build_utils.dart';
import '../label.dart';
johnmccutchan marked this conversation as resolved.
Show resolved Hide resolved
import '../run_utils.dart';
import 'command.dart';
import 'flags.dart';
Expand Down Expand Up @@ -112,14 +113,15 @@ See `flutter run --help` for a listing
return mode;
}

late final Future<RunTarget?> _runTarget =
detectAndSelectRunTarget(environment, _getDeviceId());

Future<String?> _selectTargetConfig() async {
final String configName = argResults![configFlag] as String;
if (configName.isNotEmpty) {
return demangleConfigName(environment, configName);
}
final String deviceId = _getDeviceId();
final RunTarget? target =
await detectAndSelectRunTarget(environment, deviceId);
final RunTarget? target = await _runTarget;
if (target == null) {
return demangleConfigName(environment, 'host_debug');
}
Expand Down Expand Up @@ -158,6 +160,9 @@ See `flutter run --help` for a listing
final List<String> extraGnArgs = <String>[
if (!useRbe) '--no-rbe',
];
final RunTarget? target = await _runTarget;
final List<Label> buildTargetsForShell =
target?.buildTargetsForShell() ?? <Label>[];

// First build the host.
int r = await runBuild(
Expand All @@ -172,12 +177,10 @@ See `flutter run --help` for a listing

// Now build the target if it isn't the same.
if (hostBuild.name != build.name) {
r = await runBuild(
environment,
build,
extraGnArgs: extraGnArgs,
enableRbe: useRbe,
);
r = await runBuild(environment, build,
extraGnArgs: extraGnArgs,
enableRbe: useRbe,
targets: buildTargetsForShell);
if (r != 0) {
return r;
}
Expand Down
85 changes: 59 additions & 26 deletions tools/engine_tool/lib/src/gn.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ interface class Gn {
final Environment _environment;

String get _gnPath => p.join(
_environment.engine.srcDir.path,
'flutter',
'third_party',
'gn',
_environment.platform.isWindows ? 'gn.exe' : 'gn',
);
_environment.engine.srcDir.path,
'flutter',
'third_party',
'gn',
_environment.platform.isWindows ? 'gn.exe' : 'gn',
);

/// Returns a list of build targets that match the given [pattern].
///
Expand All @@ -48,7 +48,8 @@ interface class Gn {
outDir,
pattern.toGnPattern(),
];
final ProcessRunnerResult process = await _environment.processRunner.runProcess(
final ProcessRunnerResult process =
await _environment.processRunner.runProcess(
command,
workingDirectory: _environment.engine.srcDir,
failOk: true,
Expand All @@ -71,21 +72,26 @@ interface class Gn {
);
}

return result.asMap().entries.map((MapEntry<String, Object?> entry) {
final String label = entry.key;
final Object? properties = entry.value;
if (properties is! Map<String, Object?>) {
return null;
}
final BuildTarget? target = BuildTarget._fromJson(
label,
JsonObject(properties),
);
if (target == null) {
return null;
}
return target;
}).whereType<BuildTarget>().toList();
return result
.asMap()
.entries
.map((MapEntry<String, Object?> entry) {
final String label = entry.key;
final Object? properties = entry.value;
if (properties is! Map<String, Object?>) {
return null;
}
final BuildTarget? target = BuildTarget._fromJson(
label,
JsonObject(properties),
);
if (target == null) {
return null;
}
return target;
})
.whereType<BuildTarget>()
.toList();
}
}

Expand All @@ -105,9 +111,9 @@ sealed class BuildTarget {
String type,
bool testOnly,
) = json.map((JsonObject json) => (
json.string('type'),
json.boolean('testonly'),
));
json.string('type'),
json.boolean('testonly'),
));
return switch (type) {
'executable' => ExecutableBuildTarget(
label: Label.parseGn(label),
Expand All @@ -119,6 +125,8 @@ sealed class BuildTarget {
label: Label.parseGn(label),
testOnly: testOnly,
),
'action' =>
ActionBuildTarget(label: Label.parseGn(label), testOnly: testOnly),
_ => null,
};
}
Expand All @@ -142,6 +150,30 @@ sealed class BuildTarget {
String toString();
}

/// A build target that performs some [action][].
///
/// [action]: https://gn.googlesource.com/gn/+/main/docs/reference.md#func_action
final class ActionBuildTarget extends BuildTarget {
/// Construct an action build target.
const ActionBuildTarget({
required super.label,
required super.testOnly,
});

@override
bool operator ==(Object other) {
return other is LibraryBuildTarget &&
label == other.label &&
testOnly == other.testOnly;
}

@override
int get hashCode => Object.hash(label, testOnly);

@override
String toString() => 'ActionBuildTarget($label, testOnly=$testOnly)';
}

/// A build target that produces a [shared library][] or [static library][].
///
/// [shared library]: https://gn.googlesource.com/gn/+/main/docs/reference.md#func_shared_library
Expand Down Expand Up @@ -193,5 +225,6 @@ final class ExecutableBuildTarget extends BuildTarget {
int get hashCode => Object.hash(label, testOnly, executable);

@override
String toString() => 'ExecutableBuildTarget($label, testOnly=$testOnly, executable=$executable)';
String toString() =>
'ExecutableBuildTarget($label, testOnly=$testOnly, executable=$executable)';
}
46 changes: 40 additions & 6 deletions tools/engine_tool/lib/src/run_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'dart:convert';
import 'package:process_runner/process_runner.dart';

import 'environment.dart';
import 'label.dart';
johnmccutchan marked this conversation as resolved.
Show resolved Hide resolved
import 'typed_json.dart';

const String _targetPlatformKey = 'targetPlatform';
Expand All @@ -17,12 +18,14 @@ const String _idKey = 'id';
class RunTarget {
/// Construct a RunTarget from a JSON map.
factory RunTarget.fromJson(Map<String, Object> map) {
return JsonObject(map).map((JsonObject json) => RunTarget._(
json.string(_nameKey),
json.string(_idKey),
json.string(_targetPlatformKey),
), onError: (JsonObject source, JsonMapException e) {
throw FormatException('Failed to parse RunTarget: $e', source.toPrettyString());
return JsonObject(map).map(
(JsonObject json) => RunTarget._(
json.string(_nameKey),
json.string(_idKey),
json.string(_targetPlatformKey),
), onError: (JsonObject source, JsonMapException e) {
throw FormatException(
'Failed to parse RunTarget: $e', source.toPrettyString());
});
}

Expand Down Expand Up @@ -50,6 +53,37 @@ class RunTarget {
throw UnimplementedError('No mapping for $targetPlatform');
}
}

/// Returns the minimum set of build targets needed to build the shell for
/// this target platform.
List<Label> buildTargetsForShell() {
final List<Label> labels = <Label>[];
switch (targetPlatform) {
case 'android-arm64':
case 'android-arm32':
{
labels.add(
Label.parseGn('//flutter/shell/platform/android:android_jar'));
break;
}
// TODO(cbracken): iOS and MacOS share the same target platform but
// have different build targets. For now hard code iOS.
case 'darwin':
{
labels.add(Label.parseGn(
'//flutter/shell/platform/darwin/ios:flutter_framework'));
break;
}
default:
throw UnimplementedError('No mapping for $targetPlatform');
// For the future:
// //flutter/shell/platform/darwin/macos:flutter_framework
// //flutter/shell/platform/linux:flutter_linux_gtk
// //flutter/shell/platform/windows
// //flutter/web_sdk:flutter_web_sdk_archive
}
return labels;
}
}

/// Parse the raw output of `flutter devices --machine`.
Expand Down
14 changes: 14 additions & 0 deletions tools/engine_tool/test/run_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:engine_build_configs/engine_build_configs.dart';
import 'package:engine_repo_tools/engine_repo_tools.dart';
import 'package:engine_tool/src/commands/command_runner.dart';
import 'package:engine_tool/src/environment.dart';
import 'package:engine_tool/src/label.dart';
import 'package:engine_tool/src/logger.dart';
import 'package:engine_tool/src/run_utils.dart';
import 'package:litetest/litetest.dart';
Expand Down Expand Up @@ -111,6 +112,19 @@ void main() {
expect(android.buildConfigFor('debug'), equals('android_debug_arm64'));
});

test('target specific shell build', () async {
final Logger logger = Logger.test((_) {});
final (Environment env, _) = linuxEnv(logger);
final List<RunTarget> targets =
parseDevices(env, fixtures.attachedDevices());
final RunTarget android = targets[0];
expect(android.name, contains('gphone64'));
final List<Label> shellLabels = <Label>[
Label.parseGn('//flutter/shell/platform/android:android_jar')
];
expect(android.buildTargetsForShell(), equals(shellLabels));
});

test('default device', () async {
final Logger logger = Logger.test((_) {});
final (Environment env, _) = linuxEnv(logger);
Expand Down