diff --git a/dev/devicelab/lib/tasks/perf_tests.dart b/dev/devicelab/lib/tasks/perf_tests.dart index eebe64b2db..7630fed8a5 100644 --- a/dev/devicelab/lib/tasks/perf_tests.dart +++ b/dev/devicelab/lib/tasks/perf_tests.dart @@ -1351,31 +1351,20 @@ class DevToolsMemoryTest { _device = await devices.workingDevice; await _device.unlock(); - await _launchApp(); - if (_observatoryUri == null) { - return TaskResult.failure('Observatory URI not found.'); - } - - await _launchDevTools(); - await flutter( 'drive', options: [ - '--use-existing-app', _observatoryUri, '-d', _device.deviceId, '--screenshot', hostAgent.dumpDirectory.path, '--profile', + '--profile-memory', _kJsonFileName, + '--no-publish-port', + '-v', driverTest, ], ); - _devToolsProcess.kill(); - await _devToolsProcess.exitCode; - - _runProcess.kill(); - await _runProcess.exitCode; - final Map data = json.decode( file('$project/$_kJsonFileName').readAsStringSync(), ) as Map; @@ -1391,10 +1380,6 @@ class DevToolsMemoryTest { } } - await flutter('install', options: [ - '--uninstall-only', - ]); - return TaskResult.success( {'maxRss': maxRss, 'maxAdbTotal': maxAdbTotal}, benchmarkScoreKeys: ['maxRss', 'maxAdbTotal'], @@ -1402,90 +1387,7 @@ class DevToolsMemoryTest { }); } - Future _launchApp() async { - print('launching $project$driverTest on device...'); - final String flutterPath = path.join(flutterDirectory.path, 'bin', 'flutter'); - _runProcess = await startProcess( - flutterPath, - [ - 'run', - '--verbose', - '--profile', - '--no-publish-port', - '-d', _device.deviceId, - driverTest, - ], - ); - - // Listen for Observatory URI and forward stdout/stderr - final Completer observatoryUri = Completer(); - _runProcess.stdout - .transform(utf8.decoder) - .transform(const LineSplitter()) - .listen((String line) { - print('run stdout: $line'); - final RegExpMatch match = RegExp(r'An Observatory debugger and profiler on .+ is available at: ((http|//)[a-zA-Z0-9:/=_\-\.\[\]]+)').firstMatch(line); - if (match != null && !observatoryUri.isCompleted) { - observatoryUri.complete(match[1]); - _observatoryUri = match[1]; - } - }, onDone: () { - if (!observatoryUri.isCompleted) { - observatoryUri.complete(); - } - }); - _forwardStream(_runProcess.stderr, 'run stderr'); - - _observatoryUri = await observatoryUri.future; - } - - Future _launchDevTools() async { - // To mitigate https://github.com/flutter/flutter/issues/82142 - await exec(pubBin, [ - 'global', - 'deactivate', - 'devtools', - ], canFail: true); - // The version of devtools is pinned. If we pub global activate devtools and an - // upstream devtools release breaks our CI, it will manifest on an unrelated - // commit, making it more difficult to determine the cause. - // - // Also, for release branches, all external test dependencies need to be pinned. - await exec(pubBin, [ - 'global', - 'activate', - 'devtools', - '2.2.3', - // Try to debug https://github.com/flutter/flutter/issues/82142 - '--verbose', - ]); - _devToolsProcess = await startProcess( - pubBin, - [ - 'global', - 'run', - 'devtools', - '--vm-uri', _observatoryUri, - '--profile-memory', _kJsonFileName, - ], - ); - _forwardStream(_devToolsProcess.stdout, 'devtools stdout'); - _forwardStream(_devToolsProcess.stderr, 'devtools stderr'); - } - - void _forwardStream(Stream> stream, String label) { - stream - .transform(utf8.decoder) - .transform(const LineSplitter()) - .listen((String line) { - print('$label: $line'); - }); - } - Device _device; - String _observatoryUri; - Process _runProcess; - Process _devToolsProcess; static const String _kJsonFileName = 'devtools_memory.json'; } diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index ef598721e1..8faf27b2c9 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -21,6 +21,7 @@ import '../dart/package_map.dart'; import '../device.dart'; import '../drive/drive_service.dart'; import '../globals.dart' as globals; +import '../resident_runner.dart'; import '../runner/flutter_command.dart' show FlutterCommandResult, FlutterOptions; import '../web/web_device.dart'; import 'run.dart'; @@ -143,7 +144,10 @@ class DriveCommand extends RunCommandBase { help: 'Attempts to write an SkSL file when the drive process is finished ' 'to the provided file, overwriting it if necessary.') ..addMultiOption('test-arguments', help: 'Additional arguments to pass to the ' - 'Dart VM running The test script.'); + 'Dart VM running The test script.') + ..addOption('profile-memory', help: 'Launch devtools and profile application memory, writing ' + 'The output data to the file path provided to this argument as JSON.', + valueHelp: 'profile_memory.json'); } // `pub` must always be run due to the test script running from source, @@ -215,6 +219,7 @@ class DriveCommand extends RunCommandBase { logger: _logger, processUtils: globals.processUtils, dartSdkPath: globals.artifacts.getHostArtifact(HostArtifact.engineDartBinary).path, + devtoolsLauncher: DevtoolsLauncher.instance, ); final PackageConfig packageConfig = await loadPackageConfigWithLogging( _fileSystem.file('.packages'), @@ -271,6 +276,7 @@ class DriveCommand extends RunCommandBase { ? int.tryParse(stringArg('driver-port')) : null, androidEmulator: boolArg('android-emulator'), + profileMemory: stringArg('profile-memory'), ); if (testResult != 0 && screenshot != null) { await takeScreenshot(device, screenshot, _fileSystem, _logger, _fsUtils); diff --git a/packages/flutter_tools/lib/src/devtools_launcher.dart b/packages/flutter_tools/lib/src/devtools_launcher.dart index dcdbcddac7..31057ec006 100644 --- a/packages/flutter_tools/lib/src/devtools_launcher.dart +++ b/packages/flutter_tools/lib/src/devtools_launcher.dart @@ -41,6 +41,7 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { final Platform _platform; final PersistentToolState _persistentToolState; final io.HttpClient _httpClient; + final Completer _processStartCompleter = Completer(); io.Process _devToolsProcess; @@ -49,7 +50,10 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { static const String _pubHostedUrlKey = 'PUB_HOSTED_URL'; @override - Future launch(Uri vmServiceUri) async { + Future get processStart => _processStartCompleter.future; + + @override + Future launch(Uri vmServiceUri, {List additionalArguments}) async { // Place this entire method in a try/catch that swallows exceptions because // this method is guaranteed not to return a Future that throws. try { @@ -116,7 +120,9 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { 'devtools', '--no-launch-browser', if (vmServiceUri != null) '--vm-uri=$vmServiceUri', + ...?additionalArguments, ]); + _processStartCompleter.complete(); final Completer completer = Completer(); _devToolsProcess.stdout .transform(utf8.decoder) diff --git a/packages/flutter_tools/lib/src/drive/drive_service.dart b/packages/flutter_tools/lib/src/drive/drive_service.dart index 8b74d10bb9..e74dcba533 100644 --- a/packages/flutter_tools/lib/src/drive/drive_service.dart +++ b/packages/flutter_tools/lib/src/drive/drive_service.dart @@ -4,6 +4,8 @@ // @dart = 2.8 +import 'dart:async'; + import 'package:dds/dds.dart' as dds; import 'package:file/file.dart'; import 'package:meta/meta.dart'; @@ -16,6 +18,7 @@ import '../base/logger.dart'; import '../base/process.dart'; import '../build_info.dart'; import '../device.dart'; +import '../resident_runner.dart'; import '../sksl_writer.dart'; import '../vmservice.dart'; import 'web_driver_service.dart'; @@ -26,15 +29,18 @@ class FlutterDriverFactory { @required Logger logger, @required ProcessUtils processUtils, @required String dartSdkPath, + @required DevtoolsLauncher devtoolsLauncher, }) : _applicationPackageFactory = applicationPackageFactory, _logger = logger, _processUtils = processUtils, - _dartSdkPath = dartSdkPath; + _dartSdkPath = dartSdkPath, + _devtoolsLauncher = devtoolsLauncher; final ApplicationPackageFactory _applicationPackageFactory; final Logger _logger; final ProcessUtils _processUtils; final String _dartSdkPath; + final DevtoolsLauncher _devtoolsLauncher; /// Create a driver service for running `flutter drive`. DriverService createDriverService(bool web) { @@ -49,6 +55,7 @@ class FlutterDriverFactory { processUtils: _processUtils, dartSdkPath: _dartSdkPath, applicationPackageFactory: _applicationPackageFactory, + devtoolsLauncher: _devtoolsLauncher, ); } } @@ -78,6 +85,9 @@ abstract class DriverService { /// Start the test file with the provided [arguments] and [environment], returning /// the test process exit code. + /// + /// if [profileMemory] is provided, it will be treated as a file path to write a + /// devtools memory profile. Future startTest( String testFile, List arguments, @@ -89,6 +99,7 @@ abstract class DriverService { bool androidEmulator, int driverPort, List browserDimension, + String profileMemory, }); /// Stop the running application and uninstall it from the device. @@ -110,12 +121,14 @@ class FlutterDriverService extends DriverService { @required Logger logger, @required ProcessUtils processUtils, @required String dartSdkPath, + @required DevtoolsLauncher devtoolsLauncher, @visibleForTesting VMServiceConnector vmServiceConnector = connectToVmService, }) : _applicationPackageFactory = applicationPackageFactory, _logger = logger, _processUtils = processUtils, _dartSdkPath = dartSdkPath, - _vmServiceConnector = vmServiceConnector; + _vmServiceConnector = vmServiceConnector, + _devtoolsLauncher = devtoolsLauncher; static const int _kLaunchAttempts = 3; @@ -124,6 +137,7 @@ class FlutterDriverService extends DriverService { final ProcessUtils _processUtils; final String _dartSdkPath; final VMServiceConnector _vmServiceConnector; + final DevtoolsLauncher _devtoolsLauncher; Device _device; ApplicationPackage _applicationPackage; @@ -242,14 +256,30 @@ class FlutterDriverService extends DriverService { bool androidEmulator, int driverPort, List browserDimension, + String profileMemory, }) async { - return _processUtils.stream([ - _dartSdkPath, - ...[...arguments, testFile, '-rexpanded'], - ], environment: { - 'VM_SERVICE_URL': _vmServiceUri, - ...environment, - }); + if (profileMemory != null) { + unawaited(_devtoolsLauncher.launch( + Uri.parse(_vmServiceUri), + additionalArguments: ['--profile-memory=$profileMemory'], + )); + // When profiling memory the original launch future will never complete. + await _devtoolsLauncher.processStart; + } + try { + final int result = await _processUtils.stream([ + _dartSdkPath, + ...[...arguments, testFile, '-rexpanded'], + ], environment: { + 'VM_SERVICE_URL': _vmServiceUri, + ...environment, + }); + return result; + } finally { + if (profileMemory != null) { + await _devtoolsLauncher.close(); + } + } } @override diff --git a/packages/flutter_tools/lib/src/drive/web_driver_service.dart b/packages/flutter_tools/lib/src/drive/web_driver_service.dart index 8ef198420c..aa16f87f12 100644 --- a/packages/flutter_tools/lib/src/drive/web_driver_service.dart +++ b/packages/flutter_tools/lib/src/drive/web_driver_service.dart @@ -97,6 +97,7 @@ class WebDriverService extends DriverService { bool androidEmulator, int driverPort, List browserDimension, + String profileMemory, }) async { async_io.WebDriver webDriver; final Browser browser = _browserNameToEnum(browserName); diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index 45c7720ce4..849e0d548d 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -1802,13 +1802,21 @@ abstract class DevtoolsLauncher { /// Launch a Dart DevTools process, optionally targeting a specific VM Service /// URI if [vmServiceUri] is non-null. /// + /// [additionalArguments] may be optionally specified and are passed directly + /// to the devtools run command. + /// /// This method must return a future that is guaranteed not to fail, because it /// will be used in unawaited contexts. - @visibleForTesting - Future launch(Uri vmServiceUri); + Future launch(Uri vmServiceUri, {List additionalArguments}); Future close(); + /// When measuring devtools memory via addtional arguments, the launch process + /// will technically never complete. + /// + /// Us this as an indicator that the process has started. + Future processStart; + /// Returns a future that completes when the DevTools server is ready. /// /// Completes when [devToolsUrl] is set. That can be set either directly, or diff --git a/packages/flutter_tools/test/general.shard/devtools_launcher_test.dart b/packages/flutter_tools/test/general.shard/devtools_launcher_test.dart index 40e06fe6ac..092efe329b 100644 --- a/packages/flutter_tools/test/general.shard/devtools_launcher_test.dart +++ b/packages/flutter_tools/test/general.shard/devtools_launcher_test.dart @@ -275,6 +275,45 @@ void main() { await launcher.serve(); }); + testWithoutContext('DevtoolsLauncher can launch devtools with a memory profile', () async { + persistentToolState.lastDevToolsActivation = DateTime.now(); + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand( + command: [ + 'pub', + 'global', + 'list', + ], + stdout: 'devtools 0.9.6', + ), + const FakeCommand( + command: [ + 'pub', + 'global', + 'run', + 'devtools', + '--no-launch-browser', + '--vm-uri=localhost:8181/abcdefg', + '--profile-memory=foo' + ], + stdout: 'Serving DevTools at http://127.0.0.1:9100\n', + ), + ]); + final DevtoolsLauncher launcher = DevtoolsServerLauncher( + pubExecutable: 'pub', + logger: logger, + platform: platform, + persistentToolState: persistentToolState, + httpClient: FakeHttpClient.any(), + processManager: processManager, + ); + + await launcher.launch(Uri.parse('localhost:8181/abcdefg'), additionalArguments: ['--profile-memory=foo']); + + expect(launcher.processStart, completes); + expect(processManager, hasNoRemainingExpectations); + }); + testWithoutContext('DevtoolsLauncher prints error if exception is thrown during activate', () async { final DevtoolsLauncher launcher = DevtoolsServerLauncher( pubExecutable: 'pub', diff --git a/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart b/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart index 0e9e10b002..26c8678c96 100644 --- a/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart +++ b/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart @@ -4,6 +4,8 @@ // @dart = 2.8 +import 'dart:async'; + import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_tools/src/application_package.dart'; @@ -14,6 +16,7 @@ import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/convert.dart'; import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/drive/drive_service.dart'; +import 'package:flutter_tools/src/resident_runner.dart'; import 'package:flutter_tools/src/version.dart'; import 'package:flutter_tools/src/vmservice.dart'; import 'package:meta/meta.dart'; @@ -182,6 +185,39 @@ void main() { expect(testResult, 23); }); + testWithoutContext('Connects to device VM Service and runs test application with devtools memory profile', () async { + final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: [ + getVM, + ]); + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand( + command: ['dart', '--enable-experiment=non-nullable', 'foo.test', '-rexpanded'], + exitCode: 23, + environment: { + 'FOO': 'BAR', + 'VM_SERVICE_URL': 'http://127.0.0.1:1234/' // dds forwarded URI + }, + ), + ]); + final FakeDevtoolsLauncher launcher = FakeDevtoolsLauncher(); + final DriverService driverService = setUpDriverService(processManager: processManager, vmService: fakeVmServiceHost.vmService, devtoolsLauncher: launcher); + final Device device = FakeDevice(LaunchResult.succeeded( + observatoryUri: Uri.parse('http://127.0.0.1:63426/1UasC_ihpXY=/'), + )); + + await driverService.start(BuildInfo.profile, device, DebuggingOptions.enabled(BuildInfo.profile), true); + final int testResult = await driverService.startTest( + 'foo.test', + ['--enable-experiment=non-nullable'], + {'FOO': 'BAR'}, + PackageConfig([Package('test', Uri.base)]), + profileMemory: 'devtools_memory.json', + ); + + expect(launcher.closed, true); + expect(testResult, 23); + }); + testWithoutContext('Uses dart to execute the test if there is no package:test dependency', () async { final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: [ getVM, @@ -424,6 +460,7 @@ FlutterDriverService setUpDriverService({ Logger logger, ProcessManager processManager, FlutterVmService vmService, + DevtoolsLauncher devtoolsLauncher, }) { logger ??= BufferLogger.test(); return FlutterDriverService( @@ -434,6 +471,7 @@ FlutterDriverService setUpDriverService({ processManager: processManager ?? FakeProcessManager.any(), ), dartSdkPath: 'dart', + devtoolsLauncher: devtoolsLauncher ?? FakeDevtoolsLauncher(), vmServiceConnector: (Uri httpUri, { ReloadSources reloadSources, Restart restart, @@ -557,3 +595,22 @@ class FakeDartDevelopmentService extends Fake implements DartDevelopmentService disposed = true; } } + +class FakeDevtoolsLauncher extends Fake implements DevtoolsLauncher { + bool closed = false; + final Completer _processStarted = Completer(); + + @override + Future launch(Uri vmServiceUri, {List additionalArguments}) { + _processStarted.complete(); + return Completer().future; + } + + @override + Future get processStart => _processStarted.future; + + @override + Future close() async { + closed = true; + } +}