Skip to content

incremental compiler should be able to initialize it's state from a previously generated dill file #31691

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

Closed
aam opened this issue Dec 19, 2017 · 10 comments
Assignees
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@aam
Copy link
Contributor

aam commented Dec 19, 2017

IKG used to be able to persist its state on the disk in file byte store, which sped up flutter debug/flutter run Flutter development cycle.

That ability was [temporarily] lost with introduction of "Minimal" IKG, this bug is to track bringing that functionality back.

@aam aam added the legacy-area-front-end Legacy: Use area-dart-model instead. label Dec 19, 2017
@kmillikin
Copy link

The "byte store functionality for IKG" is an implementation detail. It's not a bug to not have it. It's not even a feature you would wish for.

I think the issue here is something like, "initial hot reload should have the same latency as subsequent reloads", or something like that. Does that seem right? If so, let's change the name of this issue.

You shouldn't assume that we know the what flutter debug/flutter run development cycle is, either. Can you give us a specific workflow that you have in mind and describe it in sufficient detail that we can reproduce it?

@aam
Copy link
Contributor Author

aam commented Dec 22, 2017

I think the issue here is something like, "initial hot reload should have the same latency as subsequent
reloads", or something like that. Does that seem right?

No, performance of hot reload is different matter since that happens while frontend [server] remains resident in the memory. Here we talk about repeated launches of new instances of frontend [server].

You shouldn't assume that we know the what flutter debug/flutter run development cycle is, either.

Okay, easiest way to reproduce this is (given that you set up Flutter by following https://flutter.io/getting-started/) to build bundled examples/flutter_gallery twice in a row: flutter build flx --preview-dart-2 -v. Observe that performance of the second run is more or less the same as the first run. Expectation would be that second run would reuse compilation results of the first run and would be much faster(even if application code does change from run to run).

@aam aam added this to the 2.0-alpha milestone Dec 22, 2017
@jensjoha
Copy link
Contributor

jensjoha commented Jan 3, 2018

A small comment on the above:

Having just tried the flutter build flx thing, it is true that without --preview-dart-2 it's faster the second time than the first time, but only if there's no changes to the source files.

Changing a single file gets the runtime back on line with the initial runtime.
(What seems to happen is that it takes an md5 of all files, and then skipping the build if it's the same as last time -- which it's not when I changed a file...).

@kmillikin kmillikin changed the title Re-enable byte store functionality for IKG flutter build should be incremental Jan 4, 2018
@kmillikin kmillikin added the P2 A bug or feature request we're likely to work on label Jan 4, 2018
@a-siva
Copy link
Contributor

a-siva commented Jan 9, 2018

@jensjoha I don't understand what you mean by 'it is faster the second time than the first time, but only if there are no changes to the source file'.

The use case that this issue was created to address was the following scenario

> cd myapp
> flutter build flx  (invokes the frontend server and builds a dill file of the application and packages it)
> flutter run (sends the flx over to the device and runs it)
   >> reload (this is going to cause a full recompile of the app as the front end server instance does not have any cached state, doesn't matter if  nothing was changed or a single file in the app was changed, the resulting dill file will have to be reloaded by the VM causing all the application libraries to be reloaded)
   >> reload (no compilation will happen as nothing has changed)

We are trying to address this discrepancy between the first reload and second reload by stating that the front end should be able to populate the state by reading the dill file that has already been produced in the build step.

Please do not confuse this with how the system works in the dart 1.0 world, the implementation mechanism is different and there is a limitation we currently have with the first reload .

@jensjoha
Copy link
Contributor

Thank you for providing the actual scenario. That's new information.

The previously stated information was that doing

$ flutter build flx --preview-dart-2 -v
$ flutter build flx --preview-dart-2 -v

the second run should be faster than the first run.

My comment was that doing

$ flutter build flx -v
$ flutter build flx -v

(i.e. without --preview-dart-2) the second run is indeed faster, but doing something like

$ flutter build flx -v
$ echo "// modified" >> somefile
$ flutter build flx -v

(i.e. changing a single file) and the second invocation of flutter build is basically the same speed as the first invocation.

To me it seemed from the comment

Expectation would be that second run would reuse compilation results of the first run and would be much faster(even if application code does change from run to run).

that the second build always being faster was the current behavior (i.e. without --preview-dart-2) which I just wanted to point out was not really the case.

I do have a question in regards to the scenario though: Why would you do a flutter build first, when flutter run does the build for you?

@a-siva
Copy link
Contributor

a-siva commented Jan 11, 2018

The issue is not just running 'flutter build' and following it up with 'flutter run'.
We run into the same issue if a user runs 'flutter run' plays with the app, quits goes for a coffee and
comes back and resumes work by running 'flutter run' again.
We end up initializing the incremental kernel compiler by compiling the sources again even though
no change was made.

The idea here is that the incremental kernel compiler should be able to initialize it's state by
just reading the application dill file that exists.

I will rename this bug so that this is communicated clearly

@a-siva a-siva changed the title flutter build should be incremental incremental compiler should be able to initialize it's state from a previously generated dill file Jan 11, 2018
@a-siva a-siva modified the milestones: 2.0-alpha1, 2.0-beta1 Jan 11, 2018
@a-siva a-siva added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Jan 25, 2018
@a-siva
Copy link
Contributor

a-siva commented Jan 25, 2018

Changing this to P1 as this is becoming an issue when we do 'flutter run' in strong mode, we see errors/warnings twice on the console, once from the build step and once from the run step.

@a-siva
Copy link
Contributor

a-siva commented Feb 6, 2018

reopening this issue to track completing the loop by having flutter tools use this feature in the build/run cycle.

@a-siva a-siva reopened this Feb 6, 2018
@jensjoha
Copy link
Contributor

This was added in flutter/engine#4642.
Is there anything else missing or can we close this issue?

@aam
Copy link
Contributor Author

aam commented Feb 12, 2018

I believe that was that, thanks @jensjoha . Closing this.

@aam aam closed this as completed Feb 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

4 participants