-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[flutter_tools] only copy cached dill after startup #58188
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
[flutter_tools] only copy cached dill after startup #58188
Conversation
@@ -434,6 +435,15 @@ class _ResidentWebRunner extends ResidentWebRunner { | |||
return 1; | |||
} | |||
device.generator.accept(); | |||
globals.logger.printTrace('caching compiled dill...'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of duplicated here and in the hot runner, can this be inherited by both from the base class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
trackWidgetCreation: trackWidgetCreation, | ||
)); | ||
} | ||
artifactDirectory.deleteSync(recursive: true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't this deletion pre-exist the cache.dill optimization? Why is it no longer needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since it is a temp dir it gets cleaned up automatically right? Or should we still clean it out manually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer still making an effort to do manual clean-up. The automatic cleanup doesn't work in all situations, for example if the tool is killed by a signal on Windows after preExit()
runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, will add it back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
This reverts commit 39d1e4b.
Description
remove copying from the shutdown stage since that seems risky (and Android Studio behavior is unlikely to change). If we copy the first compilation we'll still get a decent dill for initialization.
Fixes #58109 (indirectly)