[native_assets_cli][native_assets_builder] Hot reload and hot restart

[dcharkes]

Whether hot reload and hot restart can be supported depends on the asset type.

• For bundled native code, I expect us to be able to support hot restart for dynamic libraries. (Static linking & static libraries of course requires rebuilding & restarting the native app.) [native_assets] Hot restart native libs sdk#55850
• For bundled data assets that are used as bytes/strings in Dart code, we should be able to both support hot restart and hot reload.
• For other asset types it depends on the embedder if that embedder knows how to hot reload / hot restart such asset types. (Jars for example possibly if we implement such thing in Flutter.)

Now this poses some questions for how the protocol should work:

If we blindly invoke the build and link hooks, we might get longer running builds for asset types that are not going to be hot-reload. E.g. hot reloading data assets should be fast especially if these assets are on disk already, but a locally built dynamic library will take considerably longer. We could add a BuildConfig / LinkConfig option with filteredAssetTypes.

However, adding such configuration option would break caching between the full build and the for-hot-reload build. (For example the list of dependencies is combined for all assets.)

We could fix caching by splitting the initial invocation of build and link hooks into multiple invocations per asset type. Then rerunning for a specific asset type would work.

cc @mosuem @HosseinYousefi @liamappelbe @mkustermann

[mosuem]

For bundled data assets that are used as bytes/strings in Dart code, we should be able to both support hot restart and hot reload.

Does is make sense to rebuild assets on hot reload? Some assets might take a long time to build or link... Should the user specify if an asset is fast-reloadable? It is impossible to tell for the hooks if an asset takes long to build.

[mkustermann]

If we blindly invoke the build and link hooks, we might get longer running builds for asset types that are not going to be hot-reload. E.g. hot reloading data assets should be fast especially if these assets are on disk already, but a locally built dynamic library will take considerably longer. We could add a BuildConfig / LinkConfig option with filteredAssetTypes.

I don't think this is an issue, because the building tool specifies which asset types are supported / should be built. So in

• the initial build the invocation of hook/build.dart, the build config will say
  => buildConfig.assetTypes: ['native-library', 'fragment-shader', 'image', ...]
• in the hot-restart (possibly hot-reload) invocation of hook/build.dart the build config will say
  => buildConfig.assetTypes: ['fragment-shader', 'image', ...] (i.e. a subset)

So the flutter run can decide which asset types it may support for hot-reload and just ask for those, no?

[dcharkes]

So the flutter run can decide which asset types it may support for hot-reload and just ask for those, no?

I believe the caching would not work in that case. The cached results of fragment-shader and image from the main build would not be reused in the first hot reload if none of the assets actually changed.

(Edit: Maybe that is not an issue. Flutter also prebuilds a kernel file as a copy... So maybe we can pre-populate the hot-reload assets as well.)

[mkustermann]

I believe the caching would not work in that case. The cached results of fragment-shader and image from the main build would not be reused in the first hot reload if none of the assets actually changed.

Please clearify what you mean here. If we run flutter run and then later on run flutter run (or flutter build) again, then it should perform incremental builds. Why is this any different in hot-reload? Is the implementation currently setup to not support this, or is this a fundamental issue?

[mkustermann]

As a side note: We should also ensure that dependencies (e.g. .d files) are recorded for each asset and not on a global level. This will ensure that if the building tool only wants to rebuild certain assets (e.g. shaders) only changes to those assets will cause an invocation of hook/build.dart

[dcharkes]

Why is this any different in hot-reload? Is the implementation currently setup to not support this, or is this a fundamental issue?

This is a conceptual issue: We report the dependencies of all assets together. If only a subset of assets is required for hot reload, we can't use the dependencies list for the full build to determine whether or not we need to re-invoke the hook or not. So the first build hook invocation on the first hot reload will always need to run (unless none of the dependencies changed, also of the assets that cannot be hot reloaded).

As a side note: We should also ensure that dependencies (e.g. .d files) are recorded for each asset and not on a global level. This will ensure that if the building tool only wants to rebuild certain assets (e.g. shaders) only changes to those assets will cause an invocation of hook/build.dart

This, exactly.

[dcharkes]

An alternative design suggested by @HosseinYousefi: Center the hook interaction around whether something is a hot-restart invocation or not.

• BuildConfig gets a bool isHotRestart getter.
• BuildConfig gets a Iterable<String> supportHotreloadAssetTypes getter.
• BuildOutput gets a addDependencies(Iterable<Uri> files, bool invokeForHotReload).

In the style of #1375 option 1, we would run hot-restart hook invocations in the same directory as the cold start invocation. Hook writers can then simply re-output all assets that they don't want to rebuild and rebuild the assets that they want. This enables hook writers to decide per asset if they want it to be hot restarted (maybe some heavy to compile data asset should not be hot restarted).

The caching in the native assets builder needs to slightly be changed:

• For hot restarts, we can look at the timestamp of the last hook invocation compared to the last-modified on all dependencies.
• For cold starts, we need to either have saved the timestamp of the last cold start hook invocation, or we can check all the timestamps of all asset files produced.

@mosuem This is a solution for what we were talking about last week on whether the hot-restart build directory is shared with the cold-start build directory. WDYT?

The main idea of this approach is to keep the concepts simpler:

• No need for dependencies per asset or per asset type. Hook writers explicitly say whether deps are for hot-restart or not
• No need for dependencies/assetIds that need to be changed in the build config. Hook writers just use the file system for caching (option 1 in [native_assets_cli] In-hook caching #1375).
• Single directory for where all builds are happening (for the same OS and same architecture), and users can rely on state.
  • And in addition this helps caching, the first hot-restart doesn't have to redo work that was done for the first cold start.

[dcharkes]

Whether we have dependencies per asset (#1368) is connected to whether our protocol has a concept of dry run. (Thanks @mkustermann!)

• If we have a dryRun (possibly to be renamed to listAssets),
  • The non-dry-run build invocation should not be able to add, remove or rename assets.
  • We would need hookDependencies in the dry run which influences which assets are reported.
  • We would have assetDependencies in the build hook which report dependencies per asset.
  • The build config should list which assets should be built.
  • If we want to support adding assets in hot restart, we need to rerun the dryRun to see if new assets are listed, and then also rerun the non dry run to actually produce those assets.
• If we remove dryRun
  • Then the build hook outputs the list of assets.
  • The dependencies of the build hook contain both the dependencies that influence the list of assets as well as the assets themselves.
  • The build hook will be rerun for hot restarts.
  • It is natural that hot restarts can add, remove and rename assets.

Getting rid of dry run would simplify the abstractions for our users.

Getting rid of dry run requires:

• Implementing file locking based concurrency control between flutter run (needs the asset list) and flutter assemble (needs the asset list). [native_assets_builder] Concurrent execution of NativeAssetsBuilder #1319
  • Addressing that issue should in general make the flutter build faster, the first one of the two should lock, and the other should wait until the lock is released. (per arch)

[dcharkes]

Write up of discussion with @mkustermann:

hot-reload per-asset (not proposed)

@HosseinYousefi's suggestion (#1207 (comment)) of making some assets hot restartable and others not, does not communicate to the hook that some asset types are not hot-restartable (e.g. code assets for example).

hot-reload per-asset-type (proposed solution)

config.supportedAssetTypes -> config.buildAssetTypes

Currently we have config.supportedAssetTypes which allows writing fallback logic

  if (config.supportsCode) {
    // add code asssets
  } else if (config.supportsData) {
    // add data assets - and do some fallback at runtime
  } else {
    throw Exception("Not supported!");
  }

However, this eager erroring leads to failing to run Dart code even if a path is exercised at runtime.

If instead we change it to config.buildAssetTypes, we would fail at runtime if the assets are not there. But the program won't fail if the assets are not accessed at runtime.

That also means, we can have a hot-reload invocation simply only pass the asset types buildAssetTypes = ['data'] config.

If the assets are built instead of only reported, the sharedOutputDirectory should be used to avoid re-building them in the hook in a hot reload.

Dependencies per asset type

This also means we should report the dependencies per asset type.

This requires changing the JSON (and the API).

Users cannot opt out of hot restarting an asset type

With this design, users cannot opt out of having their data assets hot restarted. (If need be, we could always add a config.isHotRestart later.)

[HosseinYousefi]

of making some assets hot restartable and others not, does not communicate to the hook that some asset types are not hot-restartable

Can you elaborate? Imagine we have CodeAsset extends Asset and Asset has an isHotRestartable getter.

[dcharkes]

of making some assets hot restartable and others not, does not communicate to the hook that some asset types are not hot-restartable

Can you elaborate? Imagine we have CodeAsset extends Asset and Asset has an isHotRestartable getter.

The hook doesn't know if the embedder (hook invoker) supports hot restart for a certain asset type unless we stick that info somewhere in the JSON. @mkustermann suggested that instead of having a separate hotRestartableAssetTypes in the config, we'd simply have buildAssetTypes (instead of supportedAssetTypes) to tell which asset types the embedder wants to be built. (This avoid having a concept of hotreload/hotrestart alltogether in the JSON protocol.)

[dcharkes]

Circling some conclusions from an offline conclusion back here:

Protocol design

• We'd like to support "derived-assets" on top of data assets. And such derived assets types might not support hot reload. So tieing hot-reload to "base-asset-type" is prohibitive.
• We don't like to bleed in embedder/framework-specific concepts to the protocol. E.g. some embedders might have hot-reload and hot-restart and others might not.

Design idea:

• config.buildAssetTypes as detailed in previous post
• config.flutterConfig.isHotReload=true

Hook-writers, and hook-helper-library-writers can choose to look at that config variable and decide to not report specific assets and their dependencies in an invocation.

The config.outputDirectory is per distinct config. So, varying that bool makes it a different output dir. And the hook will only be re-run when dependencies are reported, and the list of dependencies is smaller for config.flutterConfig.isHotReload=true.

The embedder/framework decides the semantics of their own custom config, and if hook-writers or hook-helper-library-writers use that custom config they need to respect those semantics.

In this case, config.flutterConfig.isHotReload=true must report the same or a subset of the assets and dependencies of config.flutterConfig.isHotReload=false. And flutter_tools will use the output.dependencies from the cold-run to decide whether it should invoke the hook at all on the first hot reload or hot restart.

Performance / caching / impl details

We have three layers where caching happens

1. embedder/framework: flutter_tools, dartdev
2. package:native_assets_builder
3. hook/build.dart

We'd like to avoid re-doing work (hashing file contents, or checking file timestamps) on the different layers.

• Layer 1-2 is a programmatic interface, and implementation details. We can opt to only do the checking on layer 1, and pass down which files changed in the programmatic interface to layer 2.
• Layer 2-3 is the protocol. We'd like to avoid invoking the hook as much as possible. But if we invoke the hook we're probably doing actual work anyway, so then it doesn't matter.

Fast path in layer 1 on hot reload: None of the assets changed: the native assets builder doesn't need to be reinvoked on a hot restart.
Fast path in layer 1 on hot reload: The contents of a data asset changed in the file, but the hook only reported the file, so layer 2 and 3 don't need to be invoked on a hot restart. Only the data asset needs to be synced to device.
Non-fast path: A developer changes a file that is transformed in a hook to a data asset that is reported by that hook. If the developer changes that file, that hot reload will take a bit of time. If the user did not expect hot reload to have an effect, then why would he edit the file and hot reload in the first place.

For v1.0

The config.supportedAssetTypes -> config.buildAssetTypes rename (and semantics change) needs to happen.

Thanks @mkustermann @HosseinYousefi @mosuem and @liamappelbe for the input! 👌