-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add benchmark for platform views #52717
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
Conversation
|
|
||
| <application android:name="io.flutter.app.FlutterApplication" android:label="complex_layout" android:icon="@mipmap/ic_launcher"> | ||
| <activity android:name="io.flutter.app.FlutterActivity" | ||
| <activity android:name=".DummyPlatformViewActivity" |
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 think we should avoid touching this file right now - it would probably be better to just create a new benchmark for this rather than touching the existing one, since this is used by several other benchmarks.
I'm particularly worried about what happens if this puts a devicelab device into a bad state right now.
/cc @godofredoc
| <key>io.flutter.embedded_views_preview</key> | ||
| <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.
This should be done in a different application. This will impact other benchmarks besides this one, since it changes the threading model used by the application on iOS.
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.
This will become the default though. cc @cyanglaz what is the status of that?
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.
Still have several PRs waiting on different people to reviews. It might take some time as people have limited availability working from home. If you want to land this benchmark soon, we should create a different App.
dnfield
left a comment
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.
Code overall looks good, I'm worried about impact application changes might have on other benchmarks.
|
I created a separate app in the meanwhile. Adding a new app to the flutter repo involves a non-trivial diff, so I tried to reuse as much of the existing one. Once thread merging becomes the new default, |
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.
LGTM! Thanks!
As long as ["linux/android"] and ["mac/ios"] always running on the same android and ios devices.
Fixes #52684
This PR adds a layout to the complex_layout app, which renders a few dummy platform views and an animated overlay on top of each platform view. The driver test scrolls the list and gathers information such as the 90th percentile frame build time in millisec.