Skip to content

[benchmarks] disable partial repaint for multiple backdrop blur iOS macrobenchmarks. #137902

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
merged 8 commits into from
Nov 6, 2023
Merged

[benchmarks] disable partial repaint for multiple backdrop blur iOS macrobenchmarks. #137902

merged 8 commits into from
Nov 6, 2023

Conversation

jonahwilliams
Copy link
Contributor

Partial repaint is too effective, and we'd like to be able to measure performance without carefully structuring the benchmarks. For example, right now partial repaint is culling any blurs in the multibackdrop case, which we should be using to track #132735

@jonahwilliams
Copy link
Contributor Author

This will allow flutter/engine#47688 to improve a benchmark

@zanderso
Copy link
Member

zanderso commented Nov 6, 2023

Presumably, partial repaint has performance benefits in the rest of the benchmarks such that we'd still notice a drop in performance on introducing a bug even with it disabled on this one benchmark. If so, this sgtm.

@jonahwilliams
Copy link
Contributor Author

Presumably, partial repaint has performance benefits in the rest of the benchmarks

🤔 Let me double check that

@jonahwilliams
Copy link
Contributor Author

I've updated this PR so that partial repaint is only disabled for the backdrop blur case I want to measure.

@jonahwilliams jonahwilliams changed the title [benchmarks] disable partial repaint in iOS macrobenchmarks. [benchmarks] disable partial repaint for multiple backdrop blur iOS macrobenchmarks. Nov 6, 2023
zanderso
zanderso reviewed Nov 6, 2023
View reviewed changes
dev/devicelab/lib/tasks/perf_tests.dart Outdated
}
Future<void> Function()? plistReset;
if (disablePartialRepaint) {
_disablePartialRepaint(testDirectory);
Copy link
Member

Choose a reason for hiding this comment

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

If this throws, _resetManifest() won't be run.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it won't matter in this case because you can't have disablePartialRepaint and forceOpenGLES both true at the same time, but it'd be nice to find a better pattern to follow here for when the third thing comes along...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the reset commands should always be safe to run. Let me re-arrange this a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay re-arrangement concluded. We should always reset if we throw now.

jonahwilliams added 2 commits November 6, 2023 10:30
++
3c75079
Merge branch 'disable_partial_repaint_microbench' of github.com:jonah…
6ff2775 
…williams/flutter into disable_partial_repaint_microbench
@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2023
@auto-submit auto-submit bot merged commit 925f3ee into flutter:master Nov 6, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 7, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 7, 2023
@engine-flutter-autoroll
925f3ee [benchmarks] disable partial repaint for multiple backdrop bl…
1976b2a 
…ur iOS macrobenchmarks. (flutter/flutter#137902)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 7, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 7, 2023
@engine-flutter-autoroll
925f3ee [benchmarks] disable partial repaint for multiple backdrop bl…
d21675d 
…ur iOS macrobenchmarks. (flutter/flutter#137902)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 7, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 7, 2023
@engine-flutter-autoroll
925f3ee [benchmarks] disable partial repaint for multiple backdrop bl…
6a09c6b 
…ur iOS macrobenchmarks. (flutter/flutter#137902)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 7, 2023
@engine-flutter-autoroll
Roll Flutter from f5a9835 to 5a6a322 (15 revisions) (#5345)
a83b40d 
flutter/flutter@f5a9835...5a6a322

2023-11-07 engine-flutter-autoroll@skia.org Roll Packages from 49eac1f to be18d28 (3 revisions) (flutter/flutter#137999)
2023-11-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from b1f2581f05e3 to b91400976b4a (2 revisions) (flutter/flutter#138000)
2023-11-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 633065f75b1c to b1f2581f05e3 (1 revision) (flutter/flutter#137997)
2023-11-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 99149d59cb3d to 633065f75b1c (1 revision) (flutter/flutter#137996)
2023-11-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7f4d56a7f4f7 to 99149d59cb3d (1 revision) (flutter/flutter#137987)
2023-11-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from f9f822359f2b to 7f4d56a7f4f7 (1 revision) (flutter/flutter#137983)
2023-11-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from b1a5d772792f to f9f822359f2b (1 revision) (flutter/flutter#137980)
2023-11-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 461d815b48a0 to b1a5d772792f (13 revisions) (flutter/flutter#137979)
2023-11-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from bdfa8aa8f81f to 461d815b48a0 (1 revision) (flutter/flutter#137944)
2023-11-06 sokolovskyi.konstantin@gmail.com SemanticsHandle should dispatch creation and disposal events. (flutter/flutter#137960)
2023-11-06 sokolovskyi.konstantin@gmail.com ScrollActivity should dispatch creation and disposal events. (flutter/flutter#137961)
2023-11-06 sokolovskyi.konstantin@gmail.com Cover text_selection tests with leak tracking. (flutter/flutter#137009)
2023-11-06 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#137862)
2023-11-06 42216813+eliasyishak@users.noreply.github.com Migration for `HotEvent` for Flutter hot runner (flutter/flutter#137717)
2023-11-06 jonahwilliams@google.com [benchmarks] disable partial repaint for multiple backdrop blur iOS macrobenchmarks. (flutter/flutter#137902)

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 bmparr@google.com,rmistry@google.com,stuartmorgan@google.com 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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
@engine-flutter-autoroll
925f3ee [benchmarks] disable partial repaint for multiple backdrop bl…
3a7c549 
…ur iOS macrobenchmarks. (flutter/flutter#137902)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zanderso zanderso zanderso approved these changes

@dnfield dnfield Awaiting requested review from dnfield

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jonahwilliams @zanderso