Improve hot reload performance by jonahwilliams · Pull Request #28152 · flutter/flutter

jonahwilliams · 2019-02-19T17:51:13Z

Reduces devfs operations, diff of packages, and computation of dart dependencies. Instead of traversing packages, we also include information from pubspec to remove transitive dependencies, dev_dependencies, and any cached packages from changing.

We also log a warning when updating a package version or adding a new package, since in its current state this may cause a hot reload crash.

This moves the best case hot reload times for an emulator/simulator/desktop closer to 250 ms.

In cases where a large number of libraries are invalidated (50+), times are now bottle-necked by frontend server instead of filesystem operations.

Linux Emulator w/Change

Performing hot reload...                                                
Reloaded 0 of 575 libraries in 75ms.

Performing hot reload...                                                
Reloaded 0 of 575 libraries in 78ms.

Performing hot reload...                                                
Reloaded 0 of 575 libraries in 50ms.

Linux Emulator at HEAD

Performing hot reload...                                                
Reloaded 0 of 575 libraries in 412ms.

Performing hot reload...                                                
Reloaded 0 of 575 libraries in 413ms.

Performing hot reload...                                                
Reloaded 0 of 575 libraries in 395ms.

Windows Emulator with change

Performing hot reload...
Reloaded 0 of 575 libraries in 185ms.

Performing hot reload...
Reloaded 0 of 575 libraries in 185ms.

Performing hot reload...
Reloaded 0 of 575 libraries in 143ms.

Windows Emulator at HEAD

Performing hot reload...
Reloaded 0 of 575 libraries in 887ms.

Performing hot reload...
Reloaded 0 of 575 libraries in 797ms.

Performing hot reload...
Reloaded 0 of 575 libraries in 759ms.

Mac/Pixel 2 w/ change

Performing hot reload...                                                
Reloaded 0 of 577 libraries in 335ms.

Performing hot reload...                                                
Reloaded 0 of 577 libraries in 300ms.

Performing hot reload...                                                
Reloaded 0 of 577 libraries in 320ms.

Mac/Pixel 2 at HEAD

Performing hot reload...                                                
Reloaded 0 of 577 libraries in 869ms.

Performing hot reload...                                                
Reloaded 0 of 577 libraries in 857ms.

Performing hot reload...                                                
Reloaded 0 of 577 libraries in 898ms.

Improved trace for hot reload command:

We still have some additional overhead on windows from file stat commands: I think these could be removed as well.

Trace on Windows

Below is a trace captured from a hot reload (the 6th reload) on an Android emulator running on Windows. I added Timeline events for _computeDartDependencies as well as the file stat operations in the devfs

And here is the top of the CPU profile:

The timeline events were added as follows:
flutter_tools/lib/src/run_hot.dart

index 116768a..6ba6b0b 100644
--- a/packages/flutter_tools/lib/src/run_hot.dart
+++ b/packages/flutter_tools/lib/src/run_hot.dart
@@ -3,6 +3,7 @@
 // found in the LICENSE file.

 import 'dart:async';
+import 'dart:developer';

 import 'package:json_rpc_2/error_code.dart' as rpc_error_code;
 import 'package:json_rpc_2/json_rpc_2.dart' as rpc;
@@ -99,6 +100,8 @@ class HotRunner extends ResidentRunner {
   }

   Future<bool> _refreshDartDependencies() async {
+    TimelineTask task =TimelineTask();
+    task.start('Compute dart dependencies');
     if (!hotRunnerConfig.computeDartDependencies) {
       // Disabled.
       return true;
@@ -138,6 +141,7 @@ class HotRunner extends ResidentRunner {
       );
       return false;
     }
+     task.finish();
     return true;
   }

flutter_tools/lib/src/devfs.dart

index 95fc213..a78fbdd 100644
--- a/packages/flutter_tools/lib/src/devfs.dart
+++ b/packages/flutter_tools/lib/src/devfs.dart
@@ -5,6 +5,7 @@
 import 'dart:async';
 import 'package:json_rpc_2/json_rpc_2.dart' as rpc;
 import 'package:meta/meta.dart';
+import 'dart:developer';

 import 'asset.dart';
 import 'base/common.dart';
@@ -471,25 +472,31 @@ class DevFS {
     for (DevFSContent content in _entries.values) {
       content._exists = false;
     }
-
+    final TimelineTask task = TimelineTask();
     // Scan workspace, packages, and assets
     printTrace('DevFS: Starting sync from $rootDirectory');
     logger.printTrace('Scanning project files');
+    task.start('Scan Workspace');
     await _scanDirectory(rootDirectory,
                          recursive: true,
                          fileFilter: fileFilter);
+    task.finish();
+    task.start('Scan packages');
     if (fs.isFileSync(_packagesFilePath)) {
       printTrace('Scanning package files');
       await _scanPackages(fileFilter);
     }
+    task.finish();
+    task.start('Scan assets');
     if (bundle != null) {
       printTrace('Scanning asset files');
       bundle.entries.forEach((String archivePath, DevFSContent content) {
         _scanBundleEntry(archivePath, content);
       });
     }
-
     // Handle deletions.
+    task.finish();
+    task.start('diff Assets');
     printTrace('Scanning for deleted files');
     final String assetBuildDirPrefix = _asUriPath(getAssetBuildDirectory());
     final List<Uri> toRemove = <Uri>[];
@@ -551,6 +558,7 @@ class DevFS {
     // No need to send source files because all compilation is done on the
     // host and result of compilation is single kernel file.

Fixes #28150
Fixes #27446

…oad_faster

Hixie · 2019-02-21T01:40:37Z

cc @aam

Hixie · 2019-02-21T01:41:03Z

cc @zanderso for review

aam · 2019-02-21T04:11:20Z

             stayResident: stayResident,
-             ipv6: ipv6);
+             ipv6: ipv6) {
+                watcher = DirectoryWatcher(projectRootPath ?? fs.currentDirectory.path);


I thought we talked about how this will regress dependency analysis: files outside of current directory won't be watched, unused files inside of current directory will be reported to the compiler.
The right solution is to have incremental compiler report files that comprise the application(similar to how list of files produced for .dep-file used for gradle), watch those.

For most projects the current directory contains the only sources that will be changed. For instance, on a pub based workflow to update sources I need to kill the runner and run pub get too.

How much execution time are we willing to trade to make dependency tracking more accurate? The changes here represent ~0.5 seconds across all hot reloads on windows for example. Would requiring a hot restart to update other directories be a reasonable compromise?

Querying the compiler for the list of sources it used should be instantaneous - it already has this information because it just compiled all of them.

Understood. I think we both agree that the analyzer based approach is flawed - tomorrow lets discuss how we can proceed on incorporating the incremental compiler reports.

aam · 2019-02-21T04:13:32Z

    }
    assert(reassembleViews.isNotEmpty);
-    printTrace('Evicting dirty assets');
-    await _evictDirtyAssets();


Why is this getting deleted? Does flutter engine no longer need to be told to refresh assets when they are changed?

As far as I can tell we only support updating assets on hot restart. These calls are fired on every hot reload regardless of the changed files and seem to serve no purpose

I know that you can use hot reload when you (just) update assets. Well, at least it used to work in the past fine.
It's good idea to skip sending this request if no assets were changed though.

Its possible we regressed or removed this feature in the past, or I might be missing something. I will do some archaeology tomorrow to confirm

I looked into this some more, we support reloading assets but only in the case where the content of the asset itself has changed - we don't support adding/removing/renaming them.

I've added back the evict call, but we can still skip scanning the bundle since additional/removed assets wont show up until a restart anyway

Supposedly we need to scan assets to figure out which ones were changed, which ones need to be transferred to the device.

…oad_faster

zanderso · 2019-03-11T22:00:37Z

@@ -0,0 +1,105 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.


aam · 2019-03-11T22:27:49Z

-
-    try {
-      // Will return immediately if pubspec.yaml is up-to-date.
-      await pubGet(


What will happen now if user changes pubspec.yaml in application they hot reload?

We currently run a pub get when starting via flutter run or attach. It's not possible to actually update dependencies via pub and hot reload, because this changes .packages.

If changing .packages doesn't work that I would imagine is also a bug. Once that is fixed(not sure where the culprit is), without this pub get step, changes to pubspec.yaml have no chance of being picked up though.

Filled #29186

aam · 2019-03-11T22:37:27Z

-      await _scanPackages(fileFilter);
-    }
-    if (bundle != null) {
+    if (bundle != null && fullRestart) {


What is the reason for doing this for fullRestart only? Don't we need to scan assets for changes when doing hot reload as well? I think you mentioned there might be a limitation currently that we don't pick up newly added assets on hot reload, but isn't that just a bug? All pieces seems to be in place to have that functionality available.

Filled #29184

We need to update both the assets and the asset manifest. Right now we have the capability to update the assets but the manifest doesn't update correctly. Its possible this is an oversight or a bug.

Currently the asset _entries remains populated, so hot reload of assets still works.

I'm going to add this back for now, because I've found its possible to hot reload assets in the hello_world example. Its not possible in flutter_gallery where I originally tried, and the differences there are number of assets and the usage of package assets.

…back frame schedule

jonahwilliams · 2019-03-12T21:51:03Z

We should still be able to update existing packages though, will update the PR.

EDIT: same issue as adding a new package

aam · 2019-03-14T04:02:07Z

      return false;
    } else {
-      throw 'Could not parse additional details for dependency $name; line was: "$line"';
+      // We're an empty target.


is there a way to confirm that the line describes empty target and keep else { throw for futher unexpected cases?

Removed this code, instead I should use the regular yaml parser - all I need are the the list of dependencies.

aam · 2019-03-14T04:12:11Z

-        if (content is DevFSFileContent) {
-          filesUris.add(uri);
-          invalidatedFiles.add(content.file.uri.toString());
-          syncedBytes -= content.size;


How syncedBytes is going to be updated with this code gone?

I added syncedBytes which I accidentally removed from the dill sync. All other entires are assets and are handled above

aam · 2019-03-14T04:15:21Z

          ? fs.path.relative(pathToReload, from: projectRootPath)
          : pathToReload,
        );
-        if (!dirtyEntries.containsKey(entryUri)) {


Why was this if removed, I'm curious?

I'm not sure how this could ever be false - this entryUri is for the dill file and at this point we have only added assets to the dirtyEntries.

…k initial build

… synced bytes

remove compute dart dependencies

7e53db7

googlebot added the cla: yes label Feb 19, 2019

remove dependency checker

a6dd2f0

zoechi added tool Affects the "flutter" command-line tool. See also t: labels. t: hot reload Reloading code during "flutter run" labels Feb 20, 2019

jonahwilliams added 3 commits February 20, 2019 16:51

refactor devfs and hot runner to use file watching

22deb55

Merge branch 'master' of github.com:flutter/flutter into make_hot_rel…

4cd3ec9

…oad_faster

update packages

f2044aa

jonahwilliams changed the title ~~Remove compute dart dependencies~~ Improve hot reload performance Feb 21, 2019

make sure tool has dev dep on test

b855f43

Hixie reviewed Feb 21, 2019

View reviewed changes

Comment thread packages/flutter_tools/lib/src/devfs.dart Outdated

Hixie reviewed Feb 21, 2019

View reviewed changes

Comment thread packages/flutter_tools/lib/src/globals.dart Outdated

Update devfs.dart

3870bf8

Jonah Williams added 2 commits February 20, 2019 19:58

Update globals.dart

d9cde9a

Update executable.dart

77d1796

aam reviewed Feb 21, 2019

View reviewed changes

jonahwilliams added 11 commits February 20, 2019 20:54

Merge branch 'master' of github.com:flutter/flutter into make_hot_rel…

17c4a7b

…oad_faster

only check file stat on hot restart (windows)

a6929fc

add asset eviction/saving back to reload

6862f66

Merge branch 'master' of github.com:flutter/flutter into make_hot_rel…

f687a1d

…oad_faster

use per-package directory watcher (for non pubspec)

19a8025

specificially remove local devfs tests, fix imports

7f87b7e

remove whitespace

ef3165e

disable watching on dirs that dont exist

c6de808

disable if .packages is missing

13720e0

remove test that cannot run with null devFS

faf6454

make sure packages are a file and not a directory

9c5c4dc

zanderso reviewed Mar 11, 2019

View reviewed changes

Comment thread packages/flutter_tools/lib/src/run_hot.dart

add project file invalidator test case

7628220

zanderso reviewed Mar 11, 2019

View reviewed changes

Comment thread packages/flutter_tools/test/project_file_invalidator_test.dart Outdated

aam reviewed Mar 11, 2019

View reviewed changes

Comment thread packages/flutter_tools/lib/src/run_hot.dart

aam reviewed Mar 11, 2019

View reviewed changes

jonahwilliams added 5 commits March 11, 2019 15:53

fix bug in pubspec logic, add windows unit test, simplify paths, add …

09ce671

…back frame schedule

remove unused member

0a6206f

Update devfs.dart

1bc177c

fix typo

99e160c

Add warning message when updating .packages

7917230

log warning message if .packages content changes

43664a2

jonahwilliams requested review from aam and zanderso March 13, 2019 21:19

aam reviewed Mar 14, 2019

View reviewed changes

Comment thread packages/flutter_tools/lib/src/resident_runner.dart Outdated

aam reviewed Mar 14, 2019

View reviewed changes

Comment thread packages/flutter_tools/lib/src/run_hot.dart Outdated

jonahwilliams added 3 commits March 13, 2019 22:32

assume all files are initially dirty, use project invalidator to chec…

21807c5

…k initial build

revert change to pubspec parsing, use yaml directly, add dill size to…

f112341

… synced bytes

Update devfs_test.dart

52c2707

jonahwilliams mentioned this pull request Mar 15, 2019

Enable code generation features in tool (via opt-in) #29399

Merged

9 of 10 tasks

jonahwilliams requested a review from aam March 15, 2019 20:38

aam approved these changes Mar 15, 2019

View reviewed changes

jonahwilliams merged commit cd803ac into flutter:master Mar 15, 2019

jonahwilliams mentioned this pull request Mar 20, 2019

org-dartlang-debug:synthetic_debug_expression:1:1: Error: Getter not found: '_playLocal' #29232

Closed

github-actions Bot locked as resolved and limited conversation to collaborators Aug 7, 2021

		@@ -0,0 +1,105 @@
		// Copyright 2019 The Chromium Authors. All rights reserved.

Conversation

jonahwilliams commented Feb 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Linux Emulator w/Change

Linux Emulator at HEAD

Windows Emulator with change

Windows Emulator at HEAD

Mac/Pixel 2 w/ change

Mac/Pixel 2 at HEAD

Trace on Windows

Uh oh!

Uh oh!

Uh oh!

Hixie commented Feb 21, 2019

Uh oh!

Hixie commented Feb 21, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Mar 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jonahwilliams commented Feb 19, 2019 •

edited

Loading

jonahwilliams commented Mar 12, 2019 •

edited

Loading