Reduce rebuild times when invoking 'et run' (#52883)

Android only for now but this avoids rebuilding a bunch of test targets that aren't necessary to do a `flutter run` with a custom engine. I need to fill in some blanks for other platforms before landing.
flutter · May 22, 2024 · 6f73bd9 · 6f73bd9
1 parent 0b495fe
commit 6f73bd9
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 41 deletions.
diff --git a/tools/engine_tool/lib/src/commands/run_command.dart b/tools/engine_tool/lib/src/commands/run_command.dart
@@ -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';
 import '../run_utils.dart';
 import 'command.dart';
 import 'flags.dart';
@@ -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');
     }
@@ -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(
@@ -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;
       }

diff --git a/tools/engine_tool/lib/src/gn.dart b/tools/engine_tool/lib/src/gn.dart
@@ -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].
   ///
@@ -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,
@@ -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();
   }
 }
 
@@ -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),
@@ -119,6 +125,8 @@ sealed class BuildTarget {
           label: Label.parseGn(label),
           testOnly: testOnly,
         ),
+      'action' =>
+        ActionBuildTarget(label: Label.parseGn(label), testOnly: testOnly),
       _ => null,
     };
   }
@@ -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
@@ -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)';
 }
diff --git a/tools/engine_tool/lib/src/run_utils.dart b/tools/engine_tool/lib/src/run_utils.dart
@@ -7,6 +7,7 @@ import 'dart:convert';
 import 'package:process_runner/process_runner.dart';
 
 import 'environment.dart';
+import 'label.dart';
 import 'typed_json.dart';
 
 const String _targetPlatformKey = 'targetPlatform';
@@ -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());
     });
   }
 
@@ -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`.

diff --git a/tools/engine_tool/test/run_command_test.dart b/tools/engine_tool/test/run_command_test.dart
@@ -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';
@@ -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);