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

Run all microbenchmarks #154374

Merged
merged 6 commits into from
Aug 30, 2024
Merged

Conversation

jtmcdole
Copy link
Contributor

Two things:

Re-land: Uninstall microbenchmarks before running them.
Flakes in #153828 stem from adb saying the app isn't installed, but then failing to install wtih -r. Several other tests uninstall the app before trying to run it. Previous fix called uninstall between tests, but iOS takes 12 to 13 seconds to perform uninstall / install, which timed out the test. Just uninstall the one time since we only care about any lingering apps with different keys.
Potential solution #153828

Make things go fast
Instead of installing 21 different compilations of the same app to get results; compile and run them together. Locally on Mac+iOS, this should takes ~3 minutes instead of ~15 minutes.

@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems a: animation Animation APIs a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: gestures flutter/packages/flutter/gestures repository. labels Aug 29, 2024
@jtmcdole jtmcdole requested review from zanderso and Piinks August 29, 2024 23:20
@jtmcdole
Copy link
Contributor Author

jtmcdole commented Aug 29, 2024

Mac+iOS

> time ../../bin/cache/dart-sdk/bin/dart bin/test_runner.dart test -t microbenchmarks_ios -d <my device>

  25.68s user 9.63s system 17% cpu 3:19.04 total

Mac+Android

time ../../bin/cache/dart-sdk/bin/dart bin/test_runner.dart test -t microbenchmarks -d <my other device>
  8.00s user 2.63s system 7% cpu 2:20.63 total

Flakes in flutter#153828 stem from adb saying the app isn't installed,
but then failing to install wtih `-r`. Several other tests uninstall
the app before trying to run it.

Potential solution flutter#153828
@jtmcdole jtmcdole force-pushed the runAllMicrobenchmarks branch from 1a353e6 to 7e958be Compare August 30, 2024 15:24
@jtmcdole jtmcdole added team: benchmark Performance issues found by inspecting benchmarks c: tech-debt Technical debt, code quality, testing, etc. and removed a: text input Entering text in a text field or keyboard related problems a: animation Animation APIs a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: gestures flutter/packages/flutter/gestures repository. labels Aug 30, 2024
@zanderso
Copy link
Member

Does this README need an update? https://github.com/flutter/flutter/blob/master/dev/benchmarks/microbenchmarks/README.md

It'll be interesting to see whether there are shifts in the metrics in the benchmarks that come later in the sequence. That would indicate that we still have some post-install or first-run warmup happening somewhere.

@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems a: animation Animation APIs a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: gestures flutter/packages/flutter/gestures repository. and removed c: tech-debt Technical debt, code quality, testing, etc. labels Aug 30, 2024
@jtmcdole
Copy link
Contributor Author

Does this README need an update? https://github.com/flutter/flutter/blob/master/dev/benchmarks/microbenchmarks/README.md

It'll be interesting to see whether there are shifts in the metrics in the benchmarks that come later in the sequence. That would indicate that we still have some post-install or first-run warmup happening somewhere.

I'll update it. I ran the benchmarks against a mokey and an iphone se and they were comperable to skia perf numbers.

@jtmcdole
Copy link
Contributor Author

Does this README need an update? https://github.com/flutter/flutter/blob/master/dev/benchmarks/microbenchmarks/README.md

It'll be interesting to see whether there are shifts in the metrics in the benchmarks that come later in the sequence. That would indicate that we still have some post-install or first-run warmup happening somewhere.

Done.

@jtmcdole jtmcdole added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 30, 2024
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
Reverts: flutter#154374
Initiated by: jtmcdole
Reason for reverting: A different benchmark was using the microbenchmark parser and timing out.
Original PR Author: jtmcdole

Reviewed By: {zanderso}

This change reverts the following previous change:
Two things:

**Re-land**: Uninstall microbenchmarks before running them.
Flakes in flutter#153828 stem from adb saying the app isn't installed, but then failing to install wtih `-r`. Several other tests uninstall the app before trying to run it. Previous fix called uninstall between tests, but iOS takes 12 to 13 seconds to perform uninstall / install, which timed out the test.  Just uninstall the one time since we only care about any lingering apps with different keys.
Potential solution flutter#153828

**Make things go fast**
Instead of installing 21 different compilations of the same app to get results; compile and run them together. Locally on Mac+iOS, this should takes ~3 minutes instead of ~15 minutes.
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
**Three things**

Re-lands flutter#154374

New: fix `platform_channels_benchmarks` to print the "done" key. Updated notes for the microbenchmark parser. There are no other users of `microbenchmarks.readJsonResults`.

Re-Re-land: Uninstall microbenchmarks before running them.
Flakes in flutter#153828 stem from adb saying the app isn't installed, but then failing to install wtih -r. Several other tests uninstall the app before trying to run it. Previous fix called uninstall between tests, but iOS takes 12 to 13 seconds to perform uninstall / install, which timed out the test. Just uninstall the one time since we only care about any lingering apps with different keys.
Potential solution flutter#153828

Re-land Make things go fast
Instead of installing 21 different compilations of the same app to get results; compile and run them together. Locally on Mac+iOS, this should takes ~3 minutes instead of ~15 minutes.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: animation Animation APIs a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App f: gestures flutter/packages/flutter/gestures repository. team: benchmark Performance issues found by inspecting benchmarks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants