Skip to content

More documentation about warm-up frames #132085

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 1 commit into from
Aug 8, 2023
Merged

More documentation about warm-up frames #132085

auto-submit merged 1 commit into from
Aug 8, 2023

Conversation

@Hixie
Copy link
Contributor

@Hixie Hixie commented Aug 7, 2023

No description provided.

@Hixie
Copy link
Contributor Author

Hixie commented Aug 7, 2023 •
edited
Loading

Fixes #125326

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Aug 7, 2023
@dkwingsmt
Copy link
Contributor

dkwingsmt commented Aug 7, 2023

cc @loic-sharma for having this question.

loic-sharma
loic-sharma approved these changes Aug 7, 2023
View reviewed changes
Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for documenting this!

@loic-sharma
Copy link
Member

loic-sharma commented Aug 7, 2023

FYI, this does not address the problem described by #125326. As documented on FlutterView.render, the Dart app can only call FlutterView.render within the scope of PlatformDispatcher.onBeginFrame or PlatformDispatcher.onDrawFrame.

However, hot reload's warm up frame does call FlutterView.render because sendFramesToEngine isn't set to false.

For more details from the engine side, see the issue above:

This eventually calls into Animator::Render, which attempts to reuse the pending producer continuation. Normally the producer continuation is created by the vsync in Animator::BeginFrame. However, in the hot reload scenario we skipped waiting for a vsync, never called Animator::BeginFrame, and never created a producer continuation. As a resrult, the pending producer continuation is likely already committed and the scene will be dropped.

You can confirm this behavior by setting a breakpoint here and hot reloading: https://github.com/flutter/engine/blob/609e71543cb4bc996348ae316ff00a0ed252518c/shell/common/animator.cc#L157

@Hixie
Copy link
Contributor Author

Hixie commented Aug 7, 2023

@loic-sharma I'm not sure I understand, can you elaborate? This documentation mentions that the warm-up frame will not be rendered, are you saying it should explain why it does not do so in the hot reload case (because the scene will be dropped, etc, as described above) vs other cases? Or are you saying we should change the logic itself?

@loic-sharma
Copy link
Member

loic-sharma commented Aug 7, 2023

@Hixie We should change the logic itself such that hot reload's warm up frame does not result in calls to FlutterView.render, similar to how the first warm up frame does not result in calls to FlutterView.render. Rendering a scene outside of a vsync is unsafe and can cause problems in theory.

@Hixie Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 8, 2023
@auto-submit auto-submit bot merged commit 9dbd1e9 into flutter:master Aug 8, 2023
@Hixie Hixie deleted the warmup branch August 8, 2023 22:07
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 9, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 9, 2023
@engine-flutter-autoroll
9dbd1e9 More documentation about warm-up frames (flutter/flutter#132085)
2ddd98c
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 9, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 9, 2023
@engine-flutter-autoroll
9dbd1e9 More documentation about warm-up frames (flutter/flutter#132085)
9f5f6db
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 9, 2023
@engine-flutter-autoroll
Roll Flutter from 436df69 to f4c25bb (28 revisions) (#4666)
881c1f5 
flutter/flutter@436df69...f4c25bb

2023-08-09 [email protected] Revert "Handle breaking changes in leak_tracker." (flutter/flutter#132223)
2023-08-09 [email protected] Roll Flutter Engine from 8ec575168d44 to 3d3fb4faed02 (3 revisions) (flutter/flutter#132193)
2023-08-09 [email protected] Use pattern matching to avoid strange type annotations (flutter/flutter#131964)
2023-08-09 [email protected] Roll Flutter Engine from 6839b88283b6 to 8ec575168d44 (1 revision) (flutter/flutter#132182)
2023-08-09 [email protected] Roll Flutter Engine from 29b117aa4192 to 6839b88283b6 (1 revision) (flutter/flutter#132181)
2023-08-09 [email protected] Roll Flutter Engine from 82292b8390cb to 29b117aa4192 (5 revisions) (flutter/flutter#132179)
2023-08-09 [email protected] Add more info to `OverlayState.insert` error messages (flutter/flutter#129363)
2023-08-08 [email protected] Add `textCapitalization` property for `SearchBar` and `SearchAnchor` (flutter/flutter#131459)
2023-08-08 [email protected] Roll Flutter Engine from 934ebb005d02 to 82292b8390cb (3 revisions) (flutter/flutter#132166)
2023-08-08 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.21.2 to 2.21.3 (flutter/flutter#132165)
2023-08-08 [email protected] Revert of #120385 (flutter/flutter#132167)
2023-08-08 [email protected] Add `PopupMenuButton.iconColor`, `PopupMenuTheme.iconSize` and fix button icon using unexpected color propert (flutter/flutter#132054)
2023-08-08 [email protected] More PageStorage clarity in the documentation (flutter/flutter#131954)
2023-08-08 [email protected] Document that missed_frame_build_budget_count is misleading (flutter/flutter#132137)
2023-08-08 [email protected] Locate the template directory using a TemplatePathProvider. (flutter/flutter#132156)
2023-08-08 [email protected] Roll Flutter Engine from dd03fae51d38 to 934ebb005d02 (4 revisions) (flutter/flutter#132159)
2023-08-08 [email protected] Roll Flutter Engine from 22bd35a19352 to dd03fae51d38 (3 revisions) (flutter/flutter#132149)
2023-08-08 [email protected] Fix `TabBarTheme.indicatorColor` not applied in Material 2 (flutter/flutter#132123)
2023-08-08 [email protected] Paginated Data Table : Fixed Row number at footer in last page  (flutter/flutter#130389)
2023-08-08 [email protected] [web] Use benchmark callback from `dart:ui_web` (flutter/flutter#132087)
2023-08-08 [email protected] Roll Flutter Engine from 99fdac88f3c6 to 22bd35a19352 (3 revisions) (flutter/flutter#132141)
2023-08-08 [email protected] [framework] Add Search Web to selection controls for iOS (flutter/flutter#131898)
2023-08-08 [email protected] Remove Iterator from _History (flutter/flutter#132101)
2023-08-08 [email protected] Update dartdoc driver to match current behaviour (flutter/flutter#132078)
2023-08-08 [email protected] More documentation about warm-up frames (flutter/flutter#132085)
2023-08-08 [email protected] Roll Flutter Engine from 146c4c9487fc to 99fdac88f3c6 (3 revisions) (flutter/flutter#132135)
2023-08-08 [email protected] Roll Packages from d7ee75a to ac41376 (8 revisions) (flutter/flutter#132133)
2023-08-08 [email protected] Handle breaking changes in leak_tracker. (flutter/flutter#131998)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@HansMuller HansMuller mentioned this pull request Aug 11, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
@engine-flutter-autoroll
9dbd1e9 More documentation about warm-up frames (flutter/flutter#132085)
b216d5b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@loic-sharma loic-sharma loic-sharma approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Hixie @dkwingsmt @loic-sharma