Reland change that speeds up multiple devices hot-reload (#23695)

* Revert "Revert "Run reload asynchronously so that multiple devices can reload in parallel. (#22693)" (#23598)"

This reverts commit 0b68068d.

* Fix refreshViews so it sends app-wide(rather than per-isolate) service request.

Sending per-isolate request caused dead-lock in the engine in case of more-than-one ui isolate.

packages/flutter_tools/lib/src/resident_runner.dart

@@ -79,11 +79,15 @@ class FlutterDevice {
     vmServices = localVmServices;
   }
 
-  Future<void> refreshViews() async {
+  Future<void> refreshViews() {
     if (vmServices == null || vmServices.isEmpty)
-      return;
+      return Future<void>.value(null);
+    final List<Future<void>> futures = <Future<void>>[];
     for (VMService service in vmServices)
-      await service.vm.refreshViews();
+      futures.add(service.vm.refreshViews());
+    final Completer<void> completer = Completer<void>();
+    Future.wait(futures).whenComplete(() => completer.complete(null)); // ignore: unawaited_futures
+    return completer.future;
   }
 
   List<FlutterView> get views {

packages/flutter_tools/lib/src/run_hot.dart

@@ -41,6 +41,13 @@ HotRunnerConfig get hotRunnerConfig => context[HotRunnerConfig];
 
 const bool kHotReloadDefault = true;
 
+class DeviceReloadReport {
+  DeviceReloadReport(this.device, this.reports);
+
+  FlutterDevice device;
+  List<Map<String, dynamic>> reports; // List has one report per Flutter view.
+}
+
 // TODO(flutter/flutter#23031): Test this.
 class HotRunner extends ResidentRunner {
   HotRunner(
@@ -320,8 +327,9 @@ class HotRunner extends ResidentRunner {
         return false;
     }
 
+    final List<bool> results = <bool>[];
     for (FlutterDevice device in flutterDevices) {
-      final bool result = await device.updateDevFS(
+      results.add(await device.updateDevFS(
         mainPath: mainPath,
         target: target,
         bundle: assetBundle,
@@ -332,9 +340,11 @@ class HotRunner extends ResidentRunner {
         fullRestart: fullRestart,
         projectRootPath: projectRootPath,
         pathToReload: getReloadPath(fullRestart: fullRestart),
-      );
-      if (!result)
-        return false;
+      ));
+    }
+    // If there any failures reported, bail out.
+    if (results.any((bool result) => !result)) {
+      return false;
     }
 
     if (!hotRunnerConfig.stableDartDependencies) {
@@ -344,16 +354,20 @@ class HotRunner extends ResidentRunner {
     return true;
   }
 
-  Future<void> _evictDirtyAssets() async {
+  Future<void> _evictDirtyAssets() {
+    final List<Future<Map<String, dynamic>>> futures = <Future<Map<String, dynamic>>>[];
     for (FlutterDevice device in flutterDevices) {
       if (device.devFS.assetPathsToEvict.isEmpty)
-        return;
-      if (device.views.first.uiIsolate == null)
-        throw 'Application isolate not found';
+        continue;
+      if (device.views.first.uiIsolate == null) {
+        printError('Application isolate not found for $device');
+        continue;
+      }
       for (String assetPath in device.devFS.assetPathsToEvict)
-        await device.views.first.uiIsolate.flutterEvictAsset(assetPath);
+        futures.add(device.views.first.uiIsolate.flutterEvictAsset(assetPath));
       device.devFS.assetPathsToEvict.clear();
     }
+    return Future.wait<Map<String, dynamic>>(futures);
   }
 
   void _resetDirtyAssets() {
@@ -361,46 +375,59 @@ class HotRunner extends ResidentRunner {
       device.devFS.assetPathsToEvict.clear();
   }
 
-  Future<void> _cleanupDevFS() async {
+  Future<void> _cleanupDevFS() {
+    final List<Future<void>> futures = <Future<void>>[];
     for (FlutterDevice device in flutterDevices) {
       if (device.devFS != null) {
         // Cleanup the devFS; don't wait indefinitely, and ignore any errors.
-        await device.devFS.destroy()
+        futures.add(device.devFS.destroy()
           .timeout(const Duration(milliseconds: 250))
           .catchError((dynamic error) {
             printTrace('$error');
-          });
+          }));
       }
       device.devFS = null;
     }
+    final Completer<void> completer = Completer<void>();
+    Future.wait(futures).whenComplete(() { completer.complete(null); } ); // ignore: unawaited_futures
+    return completer.future;
   }
 
   Future<void> _launchInView(FlutterDevice device,
                              Uri entryUri,
                              Uri packagesUri,
-                             Uri assetsDirectoryUri) async {
+                             Uri assetsDirectoryUri) {
+    final List<Future<void>> futures = <Future<void>>[];
     for (FlutterView view in device.views)
-      await view.runFromSource(entryUri, packagesUri, assetsDirectoryUri);
+      futures.add(view.runFromSource(entryUri, packagesUri, assetsDirectoryUri));
+    final Completer<void> completer = Completer<void>();
+    Future.wait(futures).whenComplete(() { completer.complete(null); }); // ignore: unawaited_futures
+    return completer.future;
   }
 
   Future<void> _launchFromDevFS(String mainScript) async {
     final String entryUri = fs.path.relative(mainScript, from: projectRootPath);
+    final List<Future<void>> futures = <Future<void>>[];
     for (FlutterDevice device in flutterDevices) {
       final Uri deviceEntryUri = device.devFS.baseUri.resolveUri(
         fs.path.toUri(entryUri));
       final Uri devicePackagesUri = device.devFS.baseUri.resolve('.packages');
       final Uri deviceAssetsDirectoryUri = device.devFS.baseUri.resolveUri(
         fs.path.toUri(getAssetBuildDirectory()));
-      await _launchInView(device,
+      futures.add(_launchInView(device,
                           deviceEntryUri,
                           devicePackagesUri,
-                          deviceAssetsDirectoryUri);
-      if (benchmarkMode) {
-        for (FlutterDevice device in flutterDevices)
-          for (FlutterView view in device.views)
-            await view.flushUIThreadTasks();
-      }
+                          deviceAssetsDirectoryUri));
     }
+    await Future.wait(futures);
+    if (benchmarkMode) {
+      futures.clear();
+      for (FlutterDevice device in flutterDevices)
+        for (FlutterView view in device.views)
+          futures.add(view.flushUIThreadTasks());
+      await Future.wait(futures);
+    }
+
   }
 
   Future<OperationResult> _restartFromSources({ String reason }) async {
@@ -433,19 +460,24 @@ class HotRunner extends ResidentRunner {
         device.generator.accept();
     }
     // Check if the isolate is paused and resume it.
+    final List<Future<void>> futures = <Future<void>>[];
     for (FlutterDevice device in flutterDevices) {
       for (FlutterView view in device.views) {
         if (view.uiIsolate != null) {
           // Reload the isolate.
-          await view.uiIsolate.reload();
-          final ServiceEvent pauseEvent = view.uiIsolate.pauseEvent;
-          if ((pauseEvent != null) && pauseEvent.isPauseEvent) {
-            // Resume the isolate so that it can be killed by the embedder.
-            await view.uiIsolate.resume();
-          }
+          final Completer<void> completer = Completer<void>();
+          futures.add(completer.future);
+          view.uiIsolate.reload().then((ServiceObject _) { // ignore: unawaited_futures
+            final ServiceEvent pauseEvent = view.uiIsolate.pauseEvent;
+            if ((pauseEvent != null) && pauseEvent.isPauseEvent) {
+              // Resume the isolate so that it can be killed by the embedder.
+              return view.uiIsolate.resume();
+            }
+          }).whenComplete(() { completer.complete(null); });
         }
       }
     }
+    await Future.wait(futures);
     // We are now running from source.
     _runningFromSnapshot = false;
     await _launchFromDevFS(mainPath + '.dill');
@@ -580,9 +612,7 @@ class HotRunner extends ResidentRunner {
         getReloadPath(fullRestart: false),
         from: projectRootPath,
       );
-      final Completer<Map<String, dynamic>> retrieveFirstReloadReport = Completer<Map<String, dynamic>>();
-
-      int countExpectedReports = 0;
+      final List<Future<DeviceReloadReport>> allReportsFutures = <Future<DeviceReloadReport>>[];
       for (FlutterDevice device in flutterDevices) {
         if (_runningFromSnapshot) {
           // Asset directory has to be set only once when we switch from
@@ -590,51 +620,36 @@ class HotRunner extends ResidentRunner {
           await device.resetAssetDirectory();
         }
 
-        // List has one report per Flutter view.
-        final List<Future<Map<String, dynamic>>> reports = device.reloadSources(
-          entryPath,
-          pause: pause
+        final Completer<DeviceReloadReport> completer = Completer<DeviceReloadReport>();
+        allReportsFutures.add(completer.future);
+        final List<Future<Map<String, dynamic>>> reportFutures = device.reloadSources(
+          entryPath, pause: pause
         );
-        countExpectedReports += reports.length;
-        await Future
-            .wait<Map<String, dynamic>>(reports)
-            .catchError((dynamic error) {
-              return <Map<String, dynamic>>[error];
-            })
-            .then<void>(
-              (List<Map<String, dynamic>> list) {
-                // TODO(aam): Investigate why we are validating only first reload report,
-                // which seems to be current behavior
-                final Map<String, dynamic> firstReport = list.first;
-                // Don't print errors because they will be printed further down when
-                // `validateReloadReport` is called again.
-                device.updateReloadStatus(
-                  validateReloadReport(firstReport, printErrors: false)
-                );
-                if (!retrieveFirstReloadReport.isCompleted)
-                  retrieveFirstReloadReport.complete(firstReport);
-              },
-              onError: (dynamic error, StackTrace stack) {
-                retrieveFirstReloadReport.completeError(error, stack);
-              },
-            );
+        Future.wait(reportFutures).then((List<Map<String, dynamic>> reports) { // ignore: unawaited_futures
+          // TODO(aam): Investigate why we are validating only first reload report,
+          // which seems to be current behavior
+          final Map<String, dynamic> firstReport = reports.first;
+          // Don't print errors because they will be printed further down when
+          // `validateReloadReport` is called again.
+          device.updateReloadStatus(validateReloadReport(firstReport, printErrors: false));
+          completer.complete(DeviceReloadReport(device, reports));
+        });
       }
 
-      if (countExpectedReports == 0) {
-        printError('Unable to hot reload. No instance of Flutter is currently running.');
-        return OperationResult(1, 'No instances running');
-      }
-      final Map<String, dynamic> reloadReport = await retrieveFirstReloadReport.future;
-      if (!validateReloadReport(reloadReport)) {
-        // Reload failed.
-        flutterUsage.sendEvent('hot', 'reload-reject');
-        return OperationResult(1, 'Reload rejected');
-      } else {
-        flutterUsage.sendEvent('hot', 'reload', parameters: analyticsParameters);
-        final int loadedLibraryCount = reloadReport['details']['loadedLibraryCount'];
-        final int finalLibraryCount = reloadReport['details']['finalLibraryCount'];
-        printTrace('reloaded $loadedLibraryCount of $finalLibraryCount libraries');
-        reloadMessage = 'Reloaded $loadedLibraryCount of $finalLibraryCount libraries';
+      final List<DeviceReloadReport> reports = await Future.wait(allReportsFutures);
+      for (DeviceReloadReport report in reports) {
+        final Map<String, dynamic> reloadReport = report.reports[0];
+        if (!validateReloadReport(reloadReport)) {
+          // Reload failed.
+          flutterUsage.sendEvent('hot', 'reload-reject');
+          return OperationResult(1, 'Reload rejected');
+        } else {
+          flutterUsage.sendEvent('hot', 'reload', parameters: analyticsParameters);
+          final int loadedLibraryCount = reloadReport['details']['loadedLibraryCount'];
+          final int finalLibraryCount = reloadReport['details']['finalLibraryCount'];
+          printTrace('reloaded $loadedLibraryCount of $finalLibraryCount libraries');
+          reloadMessage = 'Reloaded $loadedLibraryCount of $finalLibraryCount libraries';
+        }
       }
     } on Map<String, dynamic> catch (error, st) {
       printError('Hot reload failed: $error\n$st');
@@ -661,14 +676,22 @@ class HotRunner extends ResidentRunner {
         vmReloadTimer.elapsed.inMilliseconds);
     final Stopwatch reassembleTimer = Stopwatch()..start();
     // Reload the isolate.
+    final List<Future<void>> allDevices = <Future<void>>[];
     for (FlutterDevice device in flutterDevices) {
       printTrace('Sending reload events to ${device.device.name}');
+      final List<Future<ServiceObject>> futuresViews = <Future<ServiceObject>>[];
       for (FlutterView view in device.views) {
         printTrace('Sending reload event to "${view.uiIsolate.name}"');
-        await view.uiIsolate.reload();
+        futuresViews.add(view.uiIsolate.reload());
       }
-      await device.refreshViews();
+      final Completer<void> deviceCompleter = Completer<void>();
+      Future.wait(futuresViews).whenComplete(() { // ignore: unawaited_futures
+        deviceCompleter.complete(device.refreshViews());
+      });
+      allDevices.add(deviceCompleter.future);
     }
+
+    await Future.wait(allDevices);
     // We are now running from source.
     _runningFromSnapshot = false;
     // Check if the isolate is paused.
@@ -694,27 +717,23 @@ class HotRunner extends ResidentRunner {
     printTrace('Reassembling application');
     bool reassembleAndScheduleErrors = false;
     bool reassembleTimedOut = false;
+    final List<Future<void>> futures = <Future<void>>[];
     for (FlutterView view in reassembleViews) {
-      try {
-        await view.uiIsolate.flutterReassemble();
-      } on TimeoutException {
-        reassembleTimedOut = true;
-        printTrace('Reassembling ${view.uiIsolate.name} took too long.');
-        printStatus('Hot reloading ${view.uiIsolate.name} took too long; the reload may have failed.');
-        continue;
-      } catch (error) {
-        reassembleAndScheduleErrors = true;
-        printError('Reassembling ${view.uiIsolate.name} failed: $error');
-        continue;
-      }
-      try {
-        /* ensure that a frame is scheduled */
-        await view.uiIsolate.uiWindowScheduleFrame();
-      } catch (error) {
-        reassembleAndScheduleErrors = true;
-        printError('Scheduling a frame for ${view.uiIsolate.name} failed: $error');
-      }
+      futures.add(view.uiIsolate.flutterReassemble().then((_) {
+        return view.uiIsolate.uiWindowScheduleFrame();
+      }).catchError((dynamic error) {
+        if (error is TimeoutException) {
+          reassembleTimedOut = true;
+          printTrace('Reassembling ${view.uiIsolate.name} took too long.');
+          printStatus('Hot reloading ${view.uiIsolate.name} took too long; the reload may have failed.');
+        } else {
+          reassembleAndScheduleErrors = true;
+          printError('Reassembling ${view.uiIsolate.name} failed: $error');
+        }
+      }));
     }
+    await Future.wait(futures);
+
     // Record time it took for Flutter to reassemble the application.
     _addBenchmarkData('hotReloadFlutterReassembleMilliseconds',
         reassembleTimer.elapsed.inMilliseconds);

packages/flutter_tools/lib/src/vmservice.dart

@@ -949,15 +949,12 @@ class VM extends ServiceObjectOwner {
     return invokeRpcRaw('_getVMTimeline', timeout: kLongRequestTimeout);
   }
 
-  Future<void> refreshViews() async {
+  Future<void> refreshViews() {
     if (!isFlutterEngine)
-      return;
+      return null;
     _viewCache.clear();
-    for (Isolate isolate in isolates.toList()) {
-      await vmService.vm.invokeRpc<ServiceObject>('_flutter.listViews',
-          timeout: kLongRequestTimeout,
-          params: <String, dynamic> {'isolateId': isolate.id});
-    }
+    // Send one per-application request that refreshes all views in the app.
+    return vmService.vm.invokeRpc<ServiceObject>('_flutter.listViews', timeout: kLongRequestTimeout);
   }
 
   Iterable<FlutterView> get views => _viewCache.values;
@@ -1226,15 +1223,15 @@ class Isolate extends ServiceObjectOwner {
       Duration timeout,
       bool timeoutFatal = true,
     }
-  ) async {
-    try {
-      return await invokeRpcRaw(method, params: params, timeout: timeout, timeoutFatal: timeoutFatal);
-    } on rpc.RpcException catch (e) {
-      // If an application is not using the framework
-      if (e.code == rpc_error_code.METHOD_NOT_FOUND)
-        return null;
-      rethrow;
-    }
+  ) {
+    return invokeRpcRaw(method, params: params, timeout: timeout,
+        timeoutFatal: timeoutFatal).catchError((dynamic error) {
+            if (error is rpc.RpcException) {
+              // If an application is not using the framework
+              if (error.code == rpc_error_code.METHOD_NOT_FOUND)
+                return null;
+              throw error;
+            }});
   }
 
   // Debug dump extension methods.
@@ -1288,20 +1285,20 @@ class Isolate extends ServiceObjectOwner {
   }
 
   // Reload related extension methods.
-  Future<Map<String, dynamic>> flutterReassemble() async {
-    return await invokeFlutterExtensionRpcRaw(
+  Future<Map<String, dynamic>> flutterReassemble() {
+    return invokeFlutterExtensionRpcRaw(
       'ext.flutter.reassemble',
       timeout: kShortRequestTimeout,
       timeoutFatal: true,
     );
   }
 
-  Future<Map<String, dynamic>> uiWindowScheduleFrame() async {
-    return await invokeFlutterExtensionRpcRaw('ext.ui.window.scheduleFrame');
+  Future<Map<String, dynamic>> uiWindowScheduleFrame() {
+    return invokeFlutterExtensionRpcRaw('ext.ui.window.scheduleFrame');
   }
 
-  Future<Map<String, dynamic>> flutterEvictAsset(String assetPath) async {
-    return await invokeFlutterExtensionRpcRaw('ext.flutter.evict',
+  Future<Map<String, dynamic>> flutterEvictAsset(String assetPath) {
+    return invokeFlutterExtensionRpcRaw('ext.flutter.evict',
       params: <String, dynamic>{
         'value': assetPath,
       }
@@ -1309,8 +1306,8 @@ class Isolate extends ServiceObjectOwner {
   }
 
   // Application control extension methods.
-  Future<Map<String, dynamic>> flutterExit() async {
-    return await invokeFlutterExtensionRpcRaw(
+  Future<Map<String, dynamic>> flutterExit() {
+    return invokeFlutterExtensionRpcRaw(
       'ext.flutter.exit',
       timeout: const Duration(seconds: 2),
       timeoutFatal: false,