[et] Better RBE defaults (flutter#54059)

This PR adopts some RBE configuration from the way that chromium uses RBE

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/refs/heads/main/reclient_helper.py

These changes should bias both local and CI builds more towards using the worker pool, which we recently expanded, and should help limit the bandwidth used, which is a bottleneck for build times on a slow connection.

Author: zanderso

tools/engine_tool/test/utils.dart

@@ -35,6 +35,10 @@ class CannedProcess {
   FakeProcess get fakeProcess {
     return FakeProcess(exitCode: _exitCode, stdout: _stdout, stderr: _stderr);
   }
+
+  io.ProcessResult get processResult {
+    return io.ProcessResult(0, _exitCode, _stdout, _stderr);
+  }
 }
 
 /// ExecutedProcess includes the command and the result.
@@ -79,7 +83,19 @@ class TestEnvironment {
         });
         return processResult;
       }, onRun: (List<String> command) {
-        throw UnimplementedError('onRun');
+        final io.ProcessResult result = _getCannedProcessResult(
+          command, cannedProcesses,
+        );
+        processHistory.add(ExecutedProcess(
+          command,
+          FakeProcess(
+            exitCode: result.exitCode,
+            stdout: result.stdout as String,
+            stderr: result.stderr as String,
+          ),
+          result.exitCode,
+        ));
+        return result;
       })),
       logger: logger,
       verbose: verbose,
@@ -184,6 +200,17 @@ FakeProcess _getCannedResult(
   return FakeProcess();
 }
 
+io.ProcessResult _getCannedProcessResult(
+    List<String> command, List<CannedProcess> cannedProcesses) {
+  for (final CannedProcess cp in cannedProcesses) {
+    final bool matched = cp.commandMatcher(command);
+    if (matched) {
+      return cp.processResult;
+    }
+  }
+  return io.ProcessResult(0, 0, '', '');
+}
+
 typedef CommandMatcher = bool Function(List<String> command);
 
 /// Returns a [Matcher] that fails the test if no process has a matching command.

tools/gn

@@ -226,6 +226,15 @@ def setup_rbe(args):
   # care of starting and stopping the compiler proxy.
   running_on_luci = os.environ.get('LUCI_CONTEXT') is not None
 
+  # The default is 'racing', which eagerly attempts to use the local machine
+  # to run build actions. This is not a performance improvement in CI where the
+  # 'local machine' is nearly always a VM anyway, and is not any more powerful
+  # than the 'remote' RBE workers. Only  attempt a 'local' build if the remote
+  # worker times-out or otherwise fails. See also docs on RbeExecStrategy in the
+  # engine_build_config package under tools/pkg.
+  if running_on_luci:
+    rbe_gn_args['rbe_exec_strategy'] = 'remote_local_fallback'
+
   if args.rbe_server_address:
     rbe_gn_args['rbe_server_address'] = args.rbe_server_address
   if args.rbe_exec_strategy:

tools/pkg/engine_build_configs/lib/src/build_config_runner.dart

@@ -5,7 +5,7 @@
 import 'dart:async';
 import 'dart:convert';
 import 'dart:ffi' as ffi;
-import 'dart:io' as io show Directory, File, Process;
+import 'dart:io' as io show Directory, File, Process, ProcessResult;
 import 'dart:math';
 
 import 'package:path/path.dart' as p;
@@ -14,9 +14,9 @@ import 'package:process_runner/process_runner.dart';
 
 import 'build_config.dart';
 
-/// The base clase for events generated by a command.
+/// The base class for events generated by a command.
 sealed class RunnerEvent {
-  RunnerEvent(this.name, this.command, this.timestamp);
+  RunnerEvent(this.name, this.command, this.timestamp, [this.environment]);
 
   /// The name of the task or command.
   final String name;
@@ -26,11 +26,14 @@ sealed class RunnerEvent {
 
   /// When the event happened.
   final DateTime timestamp;
+
+  /// Additional environment variables set during the command, if any.
+  final Map<String, String>? environment;
 }
 
 /// A [RunnerEvent] representing the start of a command.
 final class RunnerStart extends RunnerEvent {
-  RunnerStart(super.name, super.command, super.timestamp);
+  RunnerStart(super.name, super.command, super.timestamp, [super.environment]);
 
   @override
   String toString() {
@@ -120,6 +123,85 @@ final class RunnerError extends RunnerEvent {
   }
 }
 
+/// The strategy that RBE uses for local vs. remote actions.
+enum RbeExecStrategy {
+  /// On a cache miss, all actions will be executed locally.
+  local,
+
+  /// On a cache miss, actions will run both remotely and locally (capacity
+  /// allowing), with the action completing when the faster of the two finishes.
+  racing,
+
+  /// On a cache miss, all actions will be executed remotely.
+  remote,
+
+  /// On a cache miss, actions will be executed locally if the latency of the
+  /// remote action completing is too high.
+  remoteLocalFallback;
+
+  @override
+  String toString() => switch (this) {
+    RbeExecStrategy.local => 'local',
+    RbeExecStrategy.racing => 'racing',
+    RbeExecStrategy.remote => 'remote',
+    RbeExecStrategy.remoteLocalFallback => 'remote_local_fallback',
+  };
+}
+
+/// Configuration options that affect how RBE works.
+class RbeConfig {
+  const RbeConfig({
+    this.remoteDisabled = false,
+    this.execStrategy = RbeExecStrategy.racing,
+    this.racingBias = 0.95,
+    this.localResourceFraction = 0.2,
+  });
+
+  /// Whether all remote queries/actions are disabled.
+  ///
+  /// When this is true, not even the remote cache will be queried. All actions
+  /// will be performed locally.
+  final bool remoteDisabled;
+
+  /// The RBE execution strategy.
+  final RbeExecStrategy execStrategy;
+
+  /// When the RBE execution strategy is 'racing', how much to bias towards
+  /// local vs. remote.
+  ///
+  /// Values should be in the range of [0, 1]. Closer to 1 implies biasing
+  /// towards remote.
+  final double racingBias;
+
+  /// When the RBE execution strategy is 'racing', how much of the local
+  /// machine to use.
+  final double localResourceFraction;
+
+  /// Environment variables to set when running RBE related subprocesses.
+  /// Defaults mirroring:
+  /// https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/refs/heads/main/reclient_helper.py
+  Map<String, String> get environment => <String, String>{
+    if (remoteDisabled)
+      'RBE_remote_disabled': '1'
+    else ...<String, String>{
+      'RBE_exec_strategy': execStrategy.toString(),
+      if (execStrategy == RbeExecStrategy.racing) ...<String, String>{
+        'RBE_racing_bias': racingBias.toString(),
+        'RBE_local_resource_fraction': localResourceFraction.toString(),
+      },
+    },
+    // Reduce the cas concurrency. Lower value doesn't impact
+    // performance when on high-speed connection, but does show improvements
+    // on easily congested networks.
+    'RBE_cas_concurrency': '100',
+    // Enable the deps cache. Mac needs a larger deps cache as it
+    // seems to have larger dependency sets per action. A larger deps cache
+    // on other platforms doesn't necessarily help but also won't hurt.
+    'RBE_enable_deps_cache': '1',
+    'RBE_deps_cache_max_mb': '1024',
+  };
+}
+
 /// The type of a callback that handles [RunnerEvent]s while a [Runner]
 /// is executing its `run()` method.
 typedef RunnerEventHandler = void Function(RunnerEvent);
@@ -190,6 +272,7 @@ final class BuildRunner extends Runner {
     ffi.Abi? abi,
     required io.Directory engineSrcDir,
     required this.build,
+    this.rbeConfig = const RbeConfig(),
     this.concurrency = 0,
     this.extraGnArgs = const <String>[],
     this.extraNinjaArgs = const <String>[],
@@ -210,6 +293,9 @@ final class BuildRunner extends Runner {
   /// The [Build] to run.
   final Build build;
 
+  /// Configuration options for RBE.
+  final RbeConfig rbeConfig;
+
   /// The maximum number of concurrent jobs.
   ///
   /// This currently only applies to the ninja build, passed as the -j
@@ -450,14 +536,22 @@ final class BuildRunner extends Runner {
       '${build.name}: RBE ${shutdown ? 'shutdown' : 'startup'}',
       bootstrapCommand,
       DateTime.now(),
+      rbeConfig.environment,
     ));
     final ProcessRunnerResult bootstrapResult;
     if (dryRun) {
       bootstrapResult = _dryRunResult;
     } else {
-      bootstrapResult = await processRunner.runProcess(
+      final io.ProcessResult result = await processRunner.processManager.run(
         bootstrapCommand,
-        failOk: true,
+        environment: rbeConfig.environment,
+      );
+      bootstrapResult = ProcessRunnerResult(
+        result.exitCode,
+        (result.stdout as String).codeUnits, // stdout.
+        (result.stderr as String).codeUnits, // stderr.
+        <int>[], // combined,
+        pid: result.pid, // pid,
       );
     }
     String okMessage = bootstrapResult.stdout.trim();
@@ -515,16 +609,20 @@ final class BuildRunner extends Runner {
         ...extraNinjaArgs,
         ...build.ninja.targets,
       ];
-      eventHandler(
-        RunnerStart('${build.name}: ninja', command, DateTime.now()),
-      );
+      eventHandler(RunnerStart(
+        '${build.name}: ninja',
+        command,
+        DateTime.now(),
+        rbeConfig.environment,
+      ));
       final ProcessRunnerResult processResult;
       if (dryRun) {
         processResult = _dryRunResult;
       } else {
         final io.Process process = await processRunner.processManager.start(
           command,
           workingDirectory: engineSrcDir.path,
+          environment: rbeConfig.environment,
         );
         final List<int> stderrOutput = <int>[];
         final List<int> stdoutOutput = <int>[];

tools/pkg/engine_build_configs/test/build_config_runner_test.dart

@@ -283,6 +283,135 @@ void main() {
     expect(events[5].name, equals('$buildName: ninja'));
   });
 
+  test('GlobalBuildRunner sets default RBE env vars in an RBE build', () async {
+    final Build targetBuild = buildConfig.builds[0];
+    final BuildRunner buildRunner = BuildRunner(
+      platform: FakePlatform(
+        operatingSystem: Platform.linux,
+        numberOfProcessors: 32,
+      ),
+      processRunner: ProcessRunner(
+        processManager: _fakeProcessManager(),
+      ),
+      abi: ffi.Abi.linuxX64,
+      engineSrcDir: engine.srcDir,
+      build: targetBuild,
+      concurrency: 500,
+      extraGnArgs: <String>['--rbe'],
+      dryRun: true,
+    );
+    final List<RunnerEvent> events = <RunnerEvent>[];
+    void handler(RunnerEvent event) => events.add(event);
+    final bool runResult = await buildRunner.run(handler);
+
+    final String buildName = targetBuild.name;
+
+    expect(runResult, isTrue);
+
+    // Check that the events for the Ninja command are correct.
+    expect(events[4] is RunnerStart, isTrue);
+    expect(events[4].name, equals('$buildName: ninja'));
+    expect(events[4].environment, isNotNull);
+    expect(events[4].environment!.containsKey('RBE_exec_strategy'), isTrue);
+    expect(
+      events[4].environment!['RBE_exec_strategy'],
+      equals(RbeExecStrategy.racing.toString()),
+    );
+    expect(events[4].environment!.containsKey('RBE_racing_bias'), isTrue);
+    expect(events[4].environment!['RBE_racing_bias'], equals('0.95'));
+    expect(
+      events[4].environment!.containsKey('RBE_local_resource_fraction'),
+      isTrue,
+    );
+    expect(
+      events[4].environment!['RBE_local_resource_fraction'],
+      equals('0.2'),
+    );
+  });
+
+  test('GlobalBuildRunner sets RBE_disable_remote when remote builds are disabled', () async {
+    final Build targetBuild = buildConfig.builds[0];
+    final BuildRunner buildRunner = BuildRunner(
+      platform: FakePlatform(
+        operatingSystem: Platform.linux,
+        numberOfProcessors: 32,
+      ),
+      processRunner: ProcessRunner(
+        processManager: _fakeProcessManager(),
+      ),
+      abi: ffi.Abi.linuxX64,
+      engineSrcDir: engine.srcDir,
+      build: targetBuild,
+      concurrency: 500,
+      rbeConfig: const RbeConfig(remoteDisabled: true),
+      extraGnArgs: <String>['--rbe'],
+      dryRun: true,
+    );
+    final List<RunnerEvent> events = <RunnerEvent>[];
+    void handler(RunnerEvent event) => events.add(event);
+    final bool runResult = await buildRunner.run(handler);
+
+    final String buildName = targetBuild.name;
+
+    expect(runResult, isTrue);
+
+    // Check that the events for the Ninja command are correct.
+    expect(events[4] is RunnerStart, isTrue);
+    expect(events[4].name, equals('$buildName: ninja'));
+    expect(events[4].environment, isNotNull);
+    expect(events[4].environment!.containsKey('RBE_remote_disabled'), isTrue);
+    expect(events[4].environment!['RBE_remote_disabled'], equals('1'));
+    expect(events[4].environment!.containsKey('RBE_exec_strategy'), isFalse);
+    expect(events[4].environment!.containsKey('RBE_racing_bias'), isFalse);
+    expect(
+      events[4].environment!.containsKey('RBE_local_resource_fraction'),
+      isFalse,
+    );
+  });
+
+  test('GlobalBuildRunner sets RBE_exec_strategy when a non-default value is passed in the RbeConfig', () async {
+    final Build targetBuild = buildConfig.builds[0];
+    final BuildRunner buildRunner = BuildRunner(
+      platform: FakePlatform(
+        operatingSystem: Platform.linux,
+        numberOfProcessors: 32,
+      ),
+      processRunner: ProcessRunner(
+        processManager: _fakeProcessManager(),
+      ),
+      abi: ffi.Abi.linuxX64,
+      engineSrcDir: engine.srcDir,
+      build: targetBuild,
+      concurrency: 500,
+      rbeConfig: const RbeConfig(execStrategy: RbeExecStrategy.local),
+      extraGnArgs: <String>['--rbe'],
+      dryRun: true,
+    );
+    final List<RunnerEvent> events = <RunnerEvent>[];
+    void handler(RunnerEvent event) => events.add(event);
+    final bool runResult = await buildRunner.run(handler);
+
+    final String buildName = targetBuild.name;
+
+    expect(runResult, isTrue);
+
+    // Check that the events for the Ninja command are correct.
+    expect(events[4] is RunnerStart, isTrue);
+    expect(events[4].name, equals('$buildName: ninja'));
+    expect(events[4].environment, isNotNull);
+    expect(events[4].environment!.containsKey('RBE_remote_disabled'), isFalse);
+    expect(events[4].environment!.containsKey('RBE_exec_strategy'), isTrue);
+    expect(
+      events[4].environment!['RBE_exec_strategy'],
+      equals(RbeExecStrategy.local.toString()),
+    );
+    expect(events[4].environment!.containsKey('RBE_racing_bias'), isFalse);
+    expect(
+      events[4].environment!.containsKey('RBE_local_resource_fraction'),
+      isFalse,
+    );
+  });
+
   test('GlobalBuildRunner passes the specified -j when explicitly provided in a non-RBE build', () async {
     final Build targetBuild = buildConfig.builds[0];
     final BuildRunner buildRunner = BuildRunner(