Unverified Commit 5dfe7e6d authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] attempt to stabilize hot restart benchmark the old fashioned way (#67971)

A change which sped up hot restart locally caused many of the devicelab measures to regress. I think this is because we do not measure when the isolate is actually "ready", so starting a reload or restart prematurely can cause time spent doing initialization to be registered as part of the reload operation.

A fix for this would be to have the framework include some sort of "initialization complete" event ... but it is not clear what the correct trigger would be. Perhaps after the first frame is successfully registered?

(9a3a0dc1 caused the benchmark regression - possibly since we spend less time syncing files now so we start the restart earlier)
parent 46238de8
...@@ -26,7 +26,7 @@ TaskFunction createHotModeTest({String deviceIdOverride, Map<String, String> env ...@@ -26,7 +26,7 @@ TaskFunction createHotModeTest({String deviceIdOverride, Map<String, String> env
final File benchmarkFile = file(path.join(_editedFlutterGalleryDir.path, 'hot_benchmark.json')); final File benchmarkFile = file(path.join(_editedFlutterGalleryDir.path, 'hot_benchmark.json'));
rm(benchmarkFile); rm(benchmarkFile);
final List<String> options = <String>[ final List<String> options = <String>[
'--hot', '-d', deviceIdOverride, '--benchmark', '--verbose', '--resident', '--output-dill', path.join('build', 'app.dill') '--hot', '-d', deviceIdOverride, '--benchmark', '--verbose', '--resident', '--output-dill',
]; ];
int hotReloadCount = 0; int hotReloadCount = 0;
Map<String, dynamic> twoReloadsData; Map<String, dynamic> twoReloadsData;
......
...@@ -237,7 +237,6 @@ class HotRunner extends ResidentRunner { ...@@ -237,7 +237,6 @@ class HotRunner extends ResidentRunner {
if (debuggingOptions.fastStart) { if (debuggingOptions.fastStart) {
await restart( await restart(
fullRestart: true, fullRestart: true,
benchmarkMode: !debuggingOptions.startPaused,
reason: 'restart', reason: 'restart',
silent: true, silent: true,
); );
...@@ -246,14 +245,18 @@ class HotRunner extends ResidentRunner { ...@@ -246,14 +245,18 @@ class HotRunner extends ResidentRunner {
appStartedCompleter?.complete(); appStartedCompleter?.complete();
if (benchmarkMode) { if (benchmarkMode) {
// Wait multiple seconds for the isolate to have fully started.
await Future<void>.delayed(const Duration(seconds: 10));
// We are running in benchmark mode. // We are running in benchmark mode.
globals.printStatus('Running in benchmark mode.'); globals.printStatus('Running in benchmark mode.');
// Measure time to perform a hot restart. // Measure time to perform a hot restart.
globals.printStatus('Benchmarking hot restart'); globals.printStatus('Benchmarking hot restart');
await restart(fullRestart: true, benchmarkMode: true); await restart(fullRestart: true);
// Wait for notifications to finish. attempt to work around // Wait multiple seconds to stabilize benchmark on slower devicelab hardware.
// timing issue caused by sentinel. // Hot restart finishes when the new isolate is started, not when the new isolate
await Future<void>.delayed(const Duration(seconds: 1)); // is ready. This process can actually take multiple seconds.
await Future<void>.delayed(const Duration(seconds: 10));
globals.printStatus('Benchmarking hot reload'); globals.printStatus('Benchmarking hot reload');
// Measure time to perform a hot reload. // Measure time to perform a hot reload.
await restart(fullRestart: false); await restart(fullRestart: false);
...@@ -462,22 +465,10 @@ class HotRunner extends ResidentRunner { ...@@ -462,22 +465,10 @@ class HotRunner extends ResidentRunner {
deviceAssetsDirectoryUri)); deviceAssetsDirectoryUri));
} }
await Future.wait(futures); await Future.wait(futures);
if (benchmarkMode) {
futures.clear();
for (final FlutterDevice device in flutterDevices) {
final List<FlutterView> views = await device.vmService.getFlutterViews();
for (final FlutterView view in views) {
futures.add(device.vmService
.flushUIThreadTasks(uiIsolateId: view.uiIsolate.id));
}
}
await Future.wait(futures);
}
} }
Future<OperationResult> _restartFromSources({ Future<OperationResult> _restartFromSources({
String reason, String reason,
bool benchmarkMode = false,
}) async { }) async {
final Stopwatch restartTimer = Stopwatch()..start(); final Stopwatch restartTimer = Stopwatch()..start();
// TODO(aam): Add generator reset logic once we switch to using incremental // TODO(aam): Add generator reset logic once we switch to using incremental
...@@ -562,32 +553,6 @@ class HotRunner extends ResidentRunner { ...@@ -562,32 +553,6 @@ class HotRunner extends ResidentRunner {
// Send timing analytics. // Send timing analytics.
globals.flutterUsage.sendTiming('hot', 'restart', restartTimer.elapsed); globals.flutterUsage.sendTiming('hot', 'restart', restartTimer.elapsed);
if (benchmarkMode) {
final List<Future<void>> isolateNotifications = <Future<void>>[];
for (final FlutterDevice device in flutterDevices) {
try {
await device.vmService.streamListen('Isolate');
} on vm_service.RPCError {
// Do nothing, we're already subscribed.
}
isolateNotifications.add(
device.vmService.onIsolateEvent.firstWhere((vm_service.Event event) {
return event.kind == vm_service.EventKind.kIsolateRunnable;
}),
);
}
await Future.wait(isolateNotifications);
final List<Future<void>> futures = <Future<void>>[];
for (final FlutterDevice device in flutterDevices) {
final List<FlutterView> views = await device.vmService.getFlutterViews();
for (final FlutterView view in views) {
futures.add(device.vmService
.flushUIThreadTasks(uiIsolateId: view.uiIsolate.id));
}
}
await Future.wait(futures);
}
// Toggle the main dill name after successfully uploading. // Toggle the main dill name after successfully uploading.
_swap =! _swap; _swap =! _swap;
...@@ -626,7 +591,6 @@ class HotRunner extends ResidentRunner { ...@@ -626,7 +591,6 @@ class HotRunner extends ResidentRunner {
Future<OperationResult> restart({ Future<OperationResult> restart({
bool fullRestart = false, bool fullRestart = false,
String reason, String reason,
bool benchmarkMode = false,
bool silent = false, bool silent = false,
bool pause = false, bool pause = false,
}) async { }) async {
...@@ -658,7 +622,6 @@ class HotRunner extends ResidentRunner { ...@@ -658,7 +622,6 @@ class HotRunner extends ResidentRunner {
sdkName: sdkName, sdkName: sdkName,
emulator: emulator, emulator: emulator,
reason: reason, reason: reason,
benchmarkMode: benchmarkMode,
silent: silent, silent: silent,
); );
if (!silent) { if (!silent) {
...@@ -687,7 +650,6 @@ class HotRunner extends ResidentRunner { ...@@ -687,7 +650,6 @@ class HotRunner extends ResidentRunner {
String sdkName, String sdkName,
bool emulator, bool emulator,
String reason, String reason,
bool benchmarkMode,
bool silent, bool silent,
}) async { }) async {
if (!canHotRestart) { if (!canHotRestart) {
...@@ -713,7 +675,6 @@ class HotRunner extends ResidentRunner { ...@@ -713,7 +675,6 @@ class HotRunner extends ResidentRunner {
// handling, at least until we can refactor the underlying code. // handling, at least until we can refactor the underlying code.
result = await asyncGuard(() => _restartFromSources( result = await asyncGuard(() => _restartFromSources(
reason: reason, reason: reason,
benchmarkMode: benchmarkMode,
)); ));
if (!result.isOk) { if (!result.isOk) {
restartEvent = 'restart-failed'; restartEvent = 'restart-failed';
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment