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

Improve modular compilation pipeline with kernel #36224

Closed
jakemac53 opened this issue Mar 14, 2019 · 9 comments
Closed

Improve modular compilation pipeline with kernel #36224

jakemac53 opened this issue Mar 14, 2019 · 9 comments
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. customer-flutter
Milestone

Comments

@jakemac53
Copy link
Contributor

The general idea here is to create a new initializeModularCompiler which can re-use Components from previous compiles (based on an input digest which we provide through the bazel worker protocol).

We should hide this behind a flag as well so we can roll it out safely.

cc @jensjoha

@jakemac53 jakemac53 added this to the Dart2.3 milestone Mar 14, 2019
@jakemac53 jakemac53 added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label Mar 14, 2019
@jakemac53
Copy link
Contributor Author

jakemac53 commented Mar 15, 2019

Note that we also need to resolve the issues around config specific imports still - but I think the pending change from @sigmundch might give us all we need (ability to provide a custom libraries.json)

@jakemac53
Copy link
Contributor Author

https://dart-review.googlesource.com/c/sdk/+/97003 gets things compiling with configurable imports, although I am having some runtime failures now when using the test package (couldn't compile it at all before though, so that is a step forward).

@vsmenon
Copy link
Member

vsmenon commented Mar 19, 2019

@jensjoha @kmillikin @jakemac53 - do we know what's needed here?

@vsmenon
Copy link
Member

vsmenon commented Mar 25, 2019

Any ETA on this?

@jensjoha
Copy link
Contributor

I think it depends on what the goal is.

I have a number of CLs out that I believe is in a "landable state" that make the kernel pipeline be "on par" with the non-kernel pipeline for some incremental changes. I've had no reviews yet though.

I also have something that makes the kernel pipeline "on par" (or faster) in all cases that I've tested, but I've basically been told that those changes cannot be made because of restrictions in package build.

@vsmenon
Copy link
Member

vsmenon commented Mar 26, 2019

For 2.3, just "on par" that we can move package:build onto. That's enough to switch over from analyzer-ddc to kernel-ddc (and lets us use the Flutter kernel transform for widget inspection).

Today, with @jakemac53 's current build-with-kernel branch, it seems about twice as slow for a small incremental edit - pretty noticeable on demos.

Beyond 2.3, definitely interested in ideas for even faster. Do you have a pointer to CLs that are out?

@jensjoha
Copy link
Contributor

https://dart-review.googlesource.com/c/sdk/+/95386 changes DDC and bazel worker to use the incremental compiler which for some changes makes it on par. It builds on a few also un-landed and un-reviewed CFE changes though.

The thing about "small incremental edits" is that it very much depend on what file is changed though. If it is a file that is (transitively) imported by lots of other files, invalidating it will pull lots of other stuff with it and thus not really be "small".

The current pipeline has tricks for not doing that when the structure of things doesn't change.

I applied a somewhat similar trick to package build for kernelm which worked (and made the kernel version always faster in my tests), but which I was wasn't allowed.

@dgrove
Copy link
Contributor

dgrove commented Mar 29, 2019

Now that https://dart-review.googlesource.com/c/sdk/+/95386 has landed, what is remaining here?

@jakemac53
Copy link
Contributor Author

I believe we can close this now, for the canonical edits I have tested with the flutter gallery we are on par or better than analyzer/ddc. Just waiting on a dev release to send out the PR for the build_* changes and publish something there.

There are still additional wins we can investigate in the future but those would be longer term and aren't blocking.

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. customer-flutter
Projects
None yet
Development

No branches or pull requests

4 participants