switch to incremental compiler for kernel-service

[sigmundch]

Currently kernel-service.dart uses the batch compiler, we'd like to turn on the incremental compiler by default.

[sigmundch]

We'd like to start getting more test coverage for the incremental compiler as well.

Question: can we switch to use it for our unit tests? Or is this blocked at this time?

We might need some additional data, in particular:

• if we don't have a reasonable performance of the incremental compiler, it might make unit tests too slow
• the mixin transformer needs to be removed (we've agreed to apply the transformation in the VM instead), but many unit tests might be fine without this.
• are we considering adding a new test configuration? (I fear the matrix might get too big unless we restrict it to a small test suite)

/cc @peter-ahe-google

[peter-ahe-google]

I don't think we can just make the switch from one day to another. I strongly believe that the incremental compiler needs to have its own test configuration so we can tell incremental issues from other issues.

[a-siva]

Is the bug talking about turning on the incremental compiler in flutter engine or in command line Dart?

For flutter engine we will turn on incremental compiler when developing and use the batch compiler when AOT code is being generated and the plan was to do something similar on command line Dart too,

[sigmundch]

Thanks for clarifying - here we were mainly thinking about command-line Dart

[aam]

Current state is that incremental compiler is used for debug builds, can't be used for profile/release build because it doesn't support an option to link-in platform.dill.

[sigmundch]

@aam - we'd be happy to add the option of linking platform.dill (I assume you mean to do the same we do with linkedDependencies in the batch compiler, correct?).

I'm curious: why is that needed? I thought the VM will link platform.dill internally when deserializing the rest of the app.

[aam]

@sigmundch , it's because of gen_snapshot, that needs vmservice library coming from vm_platform.dill

[sigmundch]

Thanks for clarifying. Does this mean that you only use it for gen_snapshot, but that for a normal run you'll not provide platform.dill (so we spend less time linking and coping bytes)?

[aam]

Right, we only use link-platform option when building profile/release version of Flutter app.

[a-siva]

we got rid of vm_platform.dill, gen_snapshot does not use it anymore.

[sigmundch]

cool - does that mean we can close this issue?

[aam]

cool - does that mean we can close this issue?

Not sure. I believe we still use [vm_]platform.dill (generated from flutter_patched_sdk) when compiling Flutter app into single .dill file: https://github.com/flutter/engine/blob/master/frontend_server/lib/server.dart#L170

[a-siva]

I don't think we can close this bug yet, the initial clarification on this bug was that command line Dart should use the incremental kernel generator and we have not done that yet.

[sigmundch]

Sorry - I meant to ask, do we still need to add linkedDependencies support in IKG (which I filed as a separate bug now #31461) or is that no longer blocking this bug?

[aam]

Sorry, I missed the comment where @sigmundch said that in this bug we were mainly thinking about command-line Dart. So my comments regarding Flutter IKG for profile/release needing linkedDependencies are not applicable here.

[bkonyi]

The CL for this issue is basically ready to land. However, some minor changes need to be made in the Flutter engine to set callbacks for file operations first. Should be done by EOD 2018/02/05.

[peter-ahe-google]

That's awesome news, @bkonyi
FWIW, I'm also trying to get better coverage of incremental compiler in the SDK.

[bkonyi]

This change was landed in the following CL.