diff --git a/engine/src/flutter/tools/engine_tool/test/utils.dart b/engine/src/flutter/tools/engine_tool/test/utils.dart index db5d2c95da..f9f94bac7e 100644 --- a/engine/src/flutter/tools/engine_tool/test/utils.dart +++ b/engine/src/flutter/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 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 command, List 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 command); /// Returns a [Matcher] that fails the test if no process has a matching command. diff --git a/engine/src/flutter/tools/gn b/engine/src/flutter/tools/gn index ac3cd93588..75dd36d0c3 100755 --- a/engine/src/flutter/tools/gn +++ b/engine/src/flutter/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: diff --git a/engine/src/flutter/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart b/engine/src/flutter/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart index df702191f8..2006857165 100644 --- a/engine/src/flutter/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart +++ b/engine/src/flutter/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? 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 get environment => { + if (remoteDisabled) + 'RBE_remote_disabled': '1' + else ...{ + 'RBE_exec_strategy': execStrategy.toString(), + if (execStrategy == RbeExecStrategy.racing) ...{ + '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 [], this.extraNinjaArgs = const [], @@ -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. + [], // combined, + pid: result.pid, // pid, ); } String okMessage = bootstrapResult.stdout.trim(); @@ -515,9 +609,12 @@ 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; @@ -525,6 +622,7 @@ final class BuildRunner extends Runner { final io.Process process = await processRunner.processManager.start( command, workingDirectory: engineSrcDir.path, + environment: rbeConfig.environment, ); final List stderrOutput = []; final List stdoutOutput = []; diff --git a/engine/src/flutter/tools/pkg/engine_build_configs/test/build_config_runner_test.dart b/engine/src/flutter/tools/pkg/engine_build_configs/test/build_config_runner_test.dart index b2e28d2072..226be900e4 100644 --- a/engine/src/flutter/tools/pkg/engine_build_configs/test/build_config_runner_test.dart +++ b/engine/src/flutter/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: ['--rbe'], + dryRun: true, + ); + final List events = []; + 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: ['--rbe'], + dryRun: true, + ); + final List events = []; + 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: ['--rbe'], + dryRun: true, + ); + final List events = []; + 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(