Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow IKG to be provided a startup state #31167

Closed
sigmundch opened this issue Oct 20, 2017 · 9 comments
Closed

allow IKG to be provided a startup state #31167

sigmundch opened this issue Oct 20, 2017 · 9 comments
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.

Comments

@sigmundch
Copy link
Member

We believe we need a mechanism to provide an initial state to IKG (as if it was restarted after an initial compile).

Context: flutter hot-reload might otherwise run into a data race condition.

@aam @a-siva - let me know if this correctly represents the scenario you described to me recently:

  • a flutter-build compiles an app using one instance of IKG (call it ikg-build)
  • a flutter-run starts the app using the bits created in ikg-build, it also start a server that can rebuild and do hot-reloads. This server uses a different IKG (ikg-run).
  • because ikg-run is new, it doesn't know what code was sent to the device. Today we "quickly" request a compile and hope that the state of ikg-run is the same as what it was sent. Now, when a future edit comes in and ikg-run does a computeDelta, then we send the relevant data down.

The potential data-race comes if an edit happens after ikg-build compiles and before ikg-run compiles. Then the hot-reload bits sent to the VM will be inconsistent.

I believe it is not possible to use the same process for both ikg-build and ikg-run. So one idea here is to able to start ikg-run with a known state.

@scheglov - what do you think?

@sigmundch sigmundch added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label Oct 20, 2017
@scheglov
Copy link
Contributor

If ikg-build puts results into the same byte store, then ikg-run will read them and serve for initial start of the app. If there are no changes in between, we will able to find the results. If there were changes, ikg-run will recompile changed libraries and reuse the rest.

I would like to learn what flutter-build does in general, and why we need to send the kernels it produced.

Starting two IKG instances does not seem right. We would need to read all sources twice, process them, read results from the byte store, deserialize them, read platform kernel, pay for performing JIT twice, etc. Plus this problem with the initial state.

@aam
Copy link
Contributor

aam commented Oct 20, 2017

I would like to learn what flutter-build does in general, and why we need to send the kernels it produced.

Quoting the command line help:

─➤  bin/flutter help build

Flutter build commands.

Usage: flutter build <subcommand> [arguments]
-h, --help    Print this usage information.

Available subcommands:
  aot   Build an ahead-of-time compiled snapshot of your app's Dart code.
  apk   Build an Android APK file from your app.
  flx   Build a Flutter FLX file from your app.
  ios   Build an iOS application bundle (Mac OS X host only).

Run "flutter help" to see global options.

╰─➤  bin/flutter help run  

Run your Flutter app on an attached device.

First thing we do when user does flutter run is we build flx-file (with complete kernel files) and send it to the device for immediate execution. Meanwhile on the host we initialize IKG so we can process users' hot reload requests(when user is ready with some changes) by recompiling edited files and send incremental kernel files to the device for hot reload.

flutter build is done in a separate process(driven by gradle script) from flutter run, so there is no one IKG instance that can serve both flutter build and flutter run.

@scheglov
Copy link
Contributor

flutter build is done in a separate process(driven by gradle script) from flutter run, so there is no one IKG instance that can serve both flutter build and flutter run.

OK, I see.
flutter build flx builds a flx file and this happens before flutter run.

Why is it necessary to flx files separately, outside of flutter run?

Why is it necessary to reuse these outside artifacts during flutter run?

First thing we do when user does flutter run is we build flx-file (with complete kernel files) and send it to the device for immediate execution.

So, we build the initial flx file during flutter run?

Meanwhile on the host we initialize IKG so we can process users' hot reload requests(when user is ready with some changes) by recompiling edited files and send incremental kernel files to the device for hot reload.

So, if IKG is created as a result of flutter run and this flutter run also builds the initial flx file, is it possible to build the initial flx file using the created IKG?

@aam
Copy link
Contributor

aam commented Oct 20, 2017

So, we build the initial flx file during flutter run?

flutter build flx is spawned as part of flutter run, yes, but in case it was done previously and no sources were edited it becomes a no-op(gradle is used to track dependencies).

So, if IKG is created as a result of flutter run and this flutter run also builds the initial flx file, is it
possible to build the initial flx file using the created IKG?

Since flutter build flx is executed via gradle, flx-file construction during flutter run can be bypassed. So it could happen that app launches on the device right away since flx is already up to date and ready to be uploaded and started on the device. I don't think we want to lose that feature.

@scheglov
Copy link
Contributor

scheglov commented Oct 20, 2017

Why is it necessary to build flx files separately, outside of flutter run?
Do users really first manually perform build, and then run?
Or all builds are part of runs and we compile outside only for dependencies tracking?

I get the part about tracking dependencies, and the fact that we don't need to rebuild if we already have up to date results. We definitely don't want to lose this. But IKG also tracks dependencies for Dart files. If there are no changes, IKG will serve kernel results from the cache. If the kernel file is appended to whatever is the rest the of FLX file, it is not different than pre-building full FLX file. It actually might be more efficient, because we separate often changing kernel files from more stable VM, graphic resources, and whatever else is in FLX files (what is there BTW?).

And the race condition in question can probably be solved easily - we need to extend DeltaProgram (or LibraryCycleResult mentioned in "Hot reload input and IKG") to include signatures for library cycles. Then flutter build can store it somewhere next to the FLX file and pass it back to a new IKG API, e.g. setState(Map<Uri, String> signatures).

@sigmundch
Copy link
Member Author

Why is it necessary to build flx files separately, outside of flutter run?

My general understanding is that there might be constraints here from how android/gradle works. While it might be possible to change some of the details, I believe it is not an easy thing to do.

And the race condition in question can probably be solved easily - we need to extend DeltaProgram (or LibraryCycleResult mentioned in "Hot reload input and IKG") to include signatures for library cycles. Then flutter build can store it somewhere next to the FLX file and pass it back to a new IKG API, e.g. setState(Map<Uri, String> signatures).

This sounds great!

@scheglov
Copy link
Contributor

See https://dart-review.googlesource.com/#/c/sdk/+/15649

My general understanding is that there might be constraints here from how android/gradle works. While it might be possible to change some of the details, I believe it is not an easy thing to do.

Is it the best for our users that we do some work twice?

How often users have pre-build FLX file vs. make changes and run, so first build using one IKG and then run using the second IKG? If the second case is the most typical use case, we should avoid duplicating work and change the way how we build and run.

whesse pushed a commit that referenced this issue Oct 20, 2017
R=ahe@google.com, paulberry@google.com, sigmund@google.com

Bug: #31167
Change-Id: I48971fc86c440fec636a775fc68d4292b0309dfa
Reviewed-on: https://dart-review.googlesource.com/15649
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
@scheglov scheglov self-assigned this Oct 21, 2017
@scheglov
Copy link
Contributor

OK, I added ability to set the initial state.

But I think we still need to think about making the system overall most performant.

@sigmundch
Copy link
Member Author

Thanks Konstantin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.
Projects
None yet
Development

No branches or pull requests

3 participants