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

Enable DisplayList benchmarks to be built on Android #31268

Merged
merged 4 commits into from
Feb 8, 2022

Conversation

gw280
Copy link
Contributor

@gw280 gw280 commented Feb 4, 2022

This allows the display_list_benchmarks target to be built on Android.

  • Only initialise ICU in benchmarking::Main() when not on Android (it causes an abort on startup, and we don't use ICU for the DisplayList benchmarks)
  • Don't build the software benchmarks on mobile platforms as we don't use software rendering and the suite takes a really long time to run
  • Don't tie testing:opengl (TestGLSurface etc) to SwiftShader as we use them for benchmarking and we want to use the native/hw OpenGL driver for those

@gw280 gw280 requested a review from flar February 4, 2022 20:51
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@gw280 gw280 force-pushed the gwright-benchmarks-android branch from 6822ef2 to 30dacc6 Compare February 5, 2022 01:00
@gw280 gw280 requested review from zanderso and removed request for flar February 7, 2022 23:46
@gw280 gw280 removed the needs tests label Feb 7, 2022
deps += [ "//flutter/testing:opengl" ]
}

if (is_mac || is_ios) {
Copy link
Member

Choose a reason for hiding this comment

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

Does this work?

Suggested change
if (is_mac || is_ios) {
if ((is_mac && enable_unittests) || is_ios) {

Copy link
Contributor Author

@gw280 gw280 Feb 7, 2022

Choose a reason for hiding this comment

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

That would work, but I think it makes more sense to expose the Metal utilities on all platforms that support Metal (iOS & Mac). If there isn't a target that depends on it, it won't get built anyway.

@gw280 gw280 force-pushed the gwright-benchmarks-android branch from 1e7e10f to 0515610 Compare February 7, 2022 23:55
testing/BUILD.gn Outdated Show resolved Hide resolved
display_list/BUILD.gn Outdated Show resolved Hide resolved
@gw280 gw280 merged commit 6f99a71 into flutter:main Feb 8, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 8, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 8, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 8, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants