This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
Lower the threshold to raster cache pictures #7687
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
cbracken
approved these changes
Feb 4, 2019
Member
cbracken
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.
lgtm
Member
|
Longer term, we should really improve this logic to be a bit cleverer. |
Contributor
Author
|
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. |
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 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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