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

Provide workaround for expression compilation frontend issue #18657

Merged
merged 1 commit into from Jun 28, 2018

Conversation

aam
Copy link
Member

@aam aam commented Jun 20, 2018

This disables attempt for frontend to initialize from kernel file since that breaks expression compilation. Underlying issue is tracked in dart-lang/sdk#33087

cc @kmillikin

@aam aam requested a review from a-siva June 20, 2018 23:58
@a-siva
Copy link
Contributor

a-siva commented Jun 21, 2018

won't this regress hot reload performance because now it has to reparse from sources?

@aam
Copy link
Member Author

aam commented Jun 21, 2018

It will regress performance, but current state of things is probably worse where expression evaluation simply doesn't work resulting in widget creation tracker and IDE plugin failures.

@jacob314
Copy link
Contributor

I would really like to see this workaround land so tip of trunk is usable with Flutter debugging tools. Most but not all of the exceptions thrown evaluating expressions go away with this change. Hot reload performance is doesn't seem that much slower with this change based on my experiments.

I would also be fine with a short term patch to switch expression evaluation back to the Dart 1 version it was previously on. That was also a fine workaround as most expressions evaluated are quite simple.

@jacob314
Copy link
Contributor

Adding offline discussions to the bug. To clarify, this change slows initial startup of the app a bit but that is already slow due to typical android or ios build times. hot reload performance after the app starts up is not impacted.

@devoncarew
Copy link
Member

To echo what Jacob said, we do need this to land soon, as a mitigation for debugging issues until the fix in the Dart sdk can be rolled into Flutter.

@a-siva
Copy link
Contributor

a-siva commented Jun 28, 2018

We have been seeing a steady degradation in performance of flutter benchmarks over time, sustaining performance is very critical and I would like for changes to not regress performance.
The change on the Dart side has already landed to fix the root issue and should be available in flutter once a roll is done. Rolls of Dart are currently blocked and maybe you should escalate resolving that if you think this bug is becoming critical.

@aam
Copy link
Member Author

aam commented Jun 28, 2018

The change on the Dart side has already landed to fix the root issue

As I indicated on dart-lang/sdk#33087 (comment) the issue is still not fixed (hopefully with proposed dart-lang/sdk#33087 (comment) it will though, but it is still to be reviewed and landed).

@devoncarew
Copy link
Member

We have been seeing a steady degradation in performance of flutter benchmarks overtime, sustaining performance is very critical

Definitely agree with you about that, however the current behavior is that debugging breaks more or less completely the first time you hit a breakpoint:

flutter/flutter-intellij#2447

It doesn't help to be fast here if its at the expense of something fundamental like debugging being broken. I do think we should land pragmatic fixes to get master more functional wrt debugging.

Rolls of Dart are currently blocked and maybe you should escalate resolving that if you think this bug is becoming critical.

I'm happy to push on this - I agree that rolls are blocked up, and that's impacting our ability to move fixes downstream.

@jacob314
Copy link
Contributor

There is hope rolls from Dart will be unblocked later today. I appreciate this is a tricky issue. On one hand we hurt user trust if we ship changes that slow startup further and on the other hand we hurt user trust in our debugging tools when they throw uncaught exceptions on startup.

Perhaps as a workaround we need to disable all impacted functionality in the IntelliJ plugin for the set of known bad engine revisions. We can display a toast message in the impacted debugging tools indicating that there is a known bug for the version they are using so the functionality is disabled.
E.g.
Engine • revision 6fe7484

I don't know if our users update their plugins frequently enough for this to be useful.

@zoechi
Copy link
Contributor

zoechi commented Jun 28, 2018

Today I practically couldn't debug my app or tests.
Every time a breakpoint was hit, I got a red exception output about "loader called on null" and then the code continued to run ignoring any breakpoints.
A workaround would be very much appreciated.
Switching back to older versions causes other issues, so also not really an option.

@aam aam merged commit 183ed46 into flutter:master Jun 28, 2018
@aam aam deleted the temp-workaround-init-from-dill branch June 28, 2018 18:17
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants