Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@liyuqian
Copy link
Contributor

@liyuqian liyuqian commented Feb 4, 2019

After removing clips by default, the OpCount of a picture drops
significantly. That makes our old threshold suboptimal. The new
threshold reflects the clip change and will improve our scroll
performance by ~10% for complex_layout and flutter_gallery scroll
benchmarks:

(flutter_gallery) home_scroll_perf frame raster time:
average 9.1ms -> 7.4ms
90th_percentile 11.3ms -> 9.2ms
99th_percentile 45ms -> 38ms

complex_layout_scroll_perf frame raster time:
average 4.8 -> 4.4ms
90th_percentile 7.8ms -> 5.4ms
99th_percentile 19ms -> 17ms

This should help mitigate issues like
flutter/flutter#24782

After removing clips by default, the OpCount of a picture drops
significantly. That makes our old threshold suboptimal. The new
threshold reflects the clip change and will improve our scroll
performance by ~10% for complex_layout and flutter_gallery scroll
benchmarks:

(flutter_gallery) home_scroll_perf frame raster time:
average		9.1ms	-> 7.4ms
90th_percentile	11.3ms	-> 9.2ms
99th_percentile	45ms	-> 38ms

complex_layout_scroll_perf frame raster time:
average		4.8	-> 4.4ms
90th_percentile	7.8ms	-> 5.4ms
99th_percentile	19ms	-> 17ms

This should also help mitigate issues like
flutter/flutter#24782
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

lgtm

@cbracken
Copy link
Member

cbracken commented Feb 4, 2019

Longer term, we should really improve this logic to be a bit cleverer.

@liyuqian
Copy link
Contributor Author

liyuqian commented Feb 4, 2019

Agree. My current concern is that we don't have a comprehensive test suite to judge how good our heuristics are. Once we have a better performance infra, I think we should be able to work on this much more confidently.

@liyuqian liyuqian merged commit 2f18c32 into flutter:master Feb 4, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 5, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Feb 5, 2019
flutter/engine@fde5900...174b73c

git log fde5900..174b73c --no-merges --oneline
174b73c Roll src/third_party/dart 3e5ed47777..b53dceadaa (5 commits) b53dceadaa Update pubspec for dev_compiler and sourcemap_testing eefc18dc82 Add a hook to AnalysisDriver on the current session to be discarded. cda43dabda Use findNode.simple in migration test bd51d6f120 [vm] Build scopes for implicit getters of static const fields b876d12f03 Revert "[vm/compiler] Continued graph checker development"
2f18c32 Lower the threshold to raster cache pictures (flutter/engine#7687)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
liyuqian added a commit that referenced this pull request Feb 5, 2019
liyuqian added a commit that referenced this pull request Feb 6, 2019
kangwang1988 pushed a commit to XianyuTech/flutter that referenced this pull request Feb 12, 2019
flutter/engine@fde5900...174b73c

git log fde5900..174b73c --no-merges --oneline
174b73c Roll src/third_party/dart 3e5ed47777..b53dceadaa (5 commits) b53dceadaa Update pubspec for dev_compiler and sourcemap_testing eefc18dc82 Add a hook to AnalysisDriver on the current session to be discarded. cda43dabda Use findNode.simple in migration test bd51d6f120 [vm] Build scopes for implicit getters of static const fields b876d12f03 Revert &flutter#34;[vm/compiler] Continued graph checker development&flutter#34;
2f18c32 Lower the threshold to raster cache pictures (flutter/engine#7687)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
liyuqian added a commit to liyuqian/engine that referenced this pull request Feb 17, 2019
This reverts commit 68d9ac4.

flutter#7759 has landed without any
unexpected regressions. Hence we'll reland this as planned.
liyuqian added a commit that referenced this pull request Feb 19, 2019
This reverts commit 68d9ac4.

#7759 has landed without any
unexpected regressions. Hence we'll reland this as planned.
@liyuqian liyuqian deleted the more_raster_cache branch June 10, 2019 22:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants