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

Add ui_benchmarks #18945

Merged
merged 4 commits into from
Jun 11, 2020
Merged

Add ui_benchmarks #18945

merged 4 commits into from
Jun 11, 2020

Conversation

liyuqian
Copy link
Contributor

@liyuqian liyuqian commented Jun 9, 2020 •
edited
Loading

We first add a PlatformMessageResponseDartComplete benchmark to test #18838. It improves from ~7600 us to ~1200 us after that PR on my MacBook Pro.

@liyuqian liyuqian requested a review from jason-simmons June 9, 2020 22:48
@auto-assign auto-assign bot requested a review from gaaclarke June 9, 2020 22:48
@liyuqian liyuqian added perf: speed Performance issues related to (mostly rendering) speed severe: performance Relates to speed or footprint issues. affects: tests labels Jun 9, 2020
@liyuqian
Copy link
Contributor Author

liyuqian commented Jun 9, 2020

@jason-simmons : I don't know why your PR didn't make a difference for BM_PlatformMessageResponseDartCompleteInner... Maybe you can have a test locally on your machine? I will also try sample or perf record to see if I can find something once I had more time.

For now, I think this is good enough to test #18838 and prevent related regressions.

@liyuqian liyuqian mentioned this pull request Jun 9, 2020
Closed
@jason-simmons
Copy link
Member

PlatformMessageResponseDart::Complete queues a task to the UI thread, but the benchmark does not wait for that task to finish.

So in BM_PlatformMessageResponseDartCompleteInner the inner loop will queue a series of tasks and then exit. The benchmark is actually trying to measure the execution of those tasks, but that isn't captured because those tasks are running on the UI thread while the benchmark thread has moved on.

BM_PlatformMessageResponseDartCompleteOuter is more accurate becuase it only queues one PlatformMessageResponseDart task for each call to RunInIsolateScope. RunInIsolateScope runs a UI thread task and waits for it to complete. So when RunInIsolateScope exits, the previously queued PlatformMessageResponseDart UI thread task must also have completed. Thus each iteration of the outer loop (except the first) measures one full run of the PlatformMessageResponseDart task.

I'd recommend removing the BM_PlatformMessageResponseDartCompleteInner benchmark. It's difficult to understand what it is measuring if it isn't waiting for the UI thread tasks to complete.

@liyuqian liyuqian force-pushed the ui_benchmark branch 2 times, most recently from 3c0d77a to 0f4b7de Compare June 10, 2020 05:35
@liyuqian
Copy link
Contributor Author

@jason-simmons , I forgot that the copy happens asynchronously on the UI thread, thank you! I've corrected my benchmark by properly waiting for the UI thread and measuring the copy. Everything is now working as expected and I no longer need that strange inner/outer comparison.

@jason-simmons
Copy link
Member

The new design LGTM.

The license file needs to be updated, and there are some other failures on Cirrus.

liyuqian added 2 commits June 10, 2020 13:00
@liyuqian
Add ui_benchmarks
6a1314e 
We first add a PlatformMessageResponseDartComplete benchmark to test
flutter#18838. Specifically, it improves
from ~7600 us to ~1200 us after that PR on my MacBook Pro.
@liyuqian
Update license
c38019a
liyuqian added 2 commits June 10, 2020 20:41
@liyuqian
Add export_dynamic_symbols
630e32d 
This should fix the Linux host_debug_unopt test
@liyuqian
Update GN format
bcab0f8
@liyuqian liyuqian added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jun 11, 2020
@fluttergithubbot fluttergithubbot merged commit 369e0a9 into flutter:master Jun 11, 2020
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 11, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 11, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
f2efe20
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 11, 2020
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 11, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
635b07b
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 12, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 12, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
eed561b
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 12, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
e442097
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 12, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 16, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
c0a1e19
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 16, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
5751f2d
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 16, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
aa91b91
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 16, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
6718924
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
d429bf5
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
0fac939
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
b7381ba
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
110aad6
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
b169d41
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
b9eb1c6
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
0f48d9c
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
077ff0f
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
45493c7
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
489253c
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
95faff0
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
7b4beda
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
70b9db4
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
0fda8a3
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
4e1eb04
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 17, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
1f0f064
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
62716d1
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
cf4a1f5
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
b4fdb3e
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
033136d
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
ba65c38
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2020
@engine-flutter-autoroll
369e0a9 Add ui_benchmarks (flutter/engine#18945)
a743c0f
zljj0818 pushed a commit to zljj0818/flutter that referenced this pull request Jun 22, 2020
@engine-flutter-autoroll @zljj0818
Roll Engine from 965fbbe to 801559a (50 revisions) (flutter#59384)
8d1e5c6 
* 141ee78 Roll Dart SDK from 2b917f5b6a0e to 0fab083e1c2b (15 revisions) (flutter/engine#18974)

* f5ab179 Call Shell::NotifyLowMemoryWarning on Android Trim and LowMemory events (flutter/engine#18979)

* 4dabac9 Roll Fuchsia Linux SDK from 8AiUM... to ThzHh... (flutter/engine#18976)

* 50cae02 Reland: Add RAII wrapper for EGLSurface (flutter/engine#18977)

* db7f226 Include the memory header as unique_ptr is used in ASCII Trie. (flutter/engine#18978)

* b19a17d Implement an EGL resource context for the Linux shell. (flutter/engine#18918)

* ca26b75 Make Linux shell plugin constructor descriptions consistent (flutter/engine#18940)

* efd1452 Roll Skia from 9c401e7e1ace to f69e40841eb9 (11 revisions) (flutter/engine#18983)

* e5845af Put JNI functions under an interface (flutter/engine#18903)

* 1ddc6e1 Call destructor and fix check (flutter/engine#18985)

* 7f71f1d Roll Fuchsia Mac SDK from oWhyp... to gAD3P... (flutter/engine#18982)

* 74291b7 Roll Dart SDK from 0fab083e1c2b to e62b89c56d01 (10 revisions) (flutter/engine#18987)

* 2739bbf [web] Provide a hook to disable location strategy (flutter/engine#18969)

* 336d000 Roll Skia from f69e40841eb9 to 553496b66f12 (2 revisions) (flutter/engine#18992)

* b82769b Roll Skia from 553496b66f12 to 0ad37b87549b (2 revisions) (flutter/engine#18993)

* f5ca58b Roll Dart SDK from e62b89c56d01 to b0d2b97d2cd7 (4 revisions) (flutter/engine#18994)

* 2a82a08 [dart] Account for compiler api change (flutter/engine#19002)

* 369e0a9 Add ui_benchmarks (flutter/engine#18945)

* a0a92d6 Roll Skia from 0ad37b87549b to de175abede4d (1 revision) (flutter/engine#18995)

* cea5a9c Roll Dart SDK from b0d2b97d2cd7 to f043f9e5f6ea (6 revisions) (flutter/engine#18997)

* d417772 Roll Fuchsia Mac SDK from gAD3P... to Wj0yo... (flutter/engine#19001)

* 4538622 Roll Skia from de175abede4d to 32d5cfa1f35e (15 revisions) (flutter/engine#19005)

* 71fce02 Fix shift-tab not working by adding more GTK->GLFW key mappings. (flutter/engine#18988)

* 5ddc122 Fix inverted check in creating resource surface (flutter/engine#18989)

* 87d8888 Show warning if method response errors occur and error not handled. (flutter/engine#18946)

* 5171fbd Roll Skia from 32d5cfa1f35e to 21bbfc6c2dfe (5 revisions) (flutter/engine#19006)

* 4bd6aea Always send key events, even if they're used for text input. (flutter/engine#18991)

* 9f898e9 Don't process key events when the text input is not requested (flutter/engine#18990)

* 48d7091 Roll buildroot for Windows warning update (flutter/engine#19000)

* 5f37029 Roll Dart SDK from f043f9e5f6ea to f0ea02bc51f8 (22 revisions) (flutter/engine#19010)

* ac809b4 onBeginFrame JNI (flutter/engine#18866)

* e1c622b Make SKSL caching test work on Fuchsia on arm64 (flutter/engine#18572)

* 2651beb Exit before pushing a trace event when layer tree holder is empty (flutter/engine#19008)

* acf048b Remove log added for local testing (flutter/engine#19012)

* 1f3aa23 Roll Dart SDK from f0ea02bc51f8 to 0b64f5488965 (9 revisions) (flutter/engine#19013)

* 8dcc95d Roll Fuchsia Mac SDK from Wj0yo... to gR0Zc... (flutter/engine#19015)

* f6455fa Roll Dart SDK from 0b64f5488965 to 50836c171e91 (4 revisions) (flutter/engine#19017)

* b2fea9d Roll Skia from 21bbfc6c2dfe to 30212b7941d6 (6 revisions) (flutter/engine#19009)

* 0065646 Roll Skia from 30212b7941d6 to 3d6bf04366f6 (17 revisions) (flutter/engine#19020)

* 0a852d8 Revert "Call Shell::NotifyLowMemoryWarning on Android Trim and LowMemory events (flutter#18979)" (flutter/engine#19023)

* 194acdf apply null safety syntax to mobile dart:ui (flutter/engine#18933)

* b0a0e0e Roll Skia from 3d6bf04366f6 to 637838d20abd (2 revisions) (flutter/engine#19021)

* be499ab Roll Fuchsia Mac SDK from gR0Zc... to H-uAk... (flutter/engine#19022)

* 8c24c41 Roll Skia from 637838d20abd to ac16760df463 (1 revision) (flutter/engine#19025)

* 7cb7003 onEndFrame JNI (flutter/engine#18867)

* e3fdb23 [fuchsia] Add ability to configure separate data and asset dirs (flutter/engine#18858)

* 983b6e1 Call Shell::NotifyLowMemory when backgrounded/memory pressure occurs on Android (flutter/engine#19026)

* f7d241f Wire up channel for restoration data (flutter/engine#18042)

* e84d497 Fix hit testing logic in fuchsia a11y (flutter/engine#19029)

* 801559a Revert to last-known-good-rev of Dart SDK (flutter/engine#19031)
@liyuqian liyuqian deleted the ui_benchmark branch June 22, 2020 20:42
@liyuqian liyuqian mentioned this pull request Jul 24, 2020
Merged
1 task
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@engine-flutter-autoroll @mingwandroid
Roll Engine from 965fbbe to 801559a (50 revisions) (flutter#59384)
18322b3 
* 141ee78 Roll Dart SDK from 2b917f5b6a0e to 0fab083e1c2b (15 revisions) (flutter/engine#18974)

* f5ab179 Call Shell::NotifyLowMemoryWarning on Android Trim and LowMemory events (flutter/engine#18979)

* 4dabac9 Roll Fuchsia Linux SDK from 8AiUM... to ThzHh... (flutter/engine#18976)

* 50cae02 Reland: Add RAII wrapper for EGLSurface (flutter/engine#18977)

* db7f226 Include the memory header as unique_ptr is used in ASCII Trie. (flutter/engine#18978)

* b19a17d Implement an EGL resource context for the Linux shell. (flutter/engine#18918)

* ca26b75 Make Linux shell plugin constructor descriptions consistent (flutter/engine#18940)

* efd1452 Roll Skia from 9c401e7e1ace to f69e40841eb9 (11 revisions) (flutter/engine#18983)

* e5845af Put JNI functions under an interface (flutter/engine#18903)

* 1ddc6e1 Call destructor and fix check (flutter/engine#18985)

* 7f71f1d Roll Fuchsia Mac SDK from oWhyp... to gAD3P... (flutter/engine#18982)

* 74291b7 Roll Dart SDK from 0fab083e1c2b to e62b89c56d01 (10 revisions) (flutter/engine#18987)

* 2739bbf [web] Provide a hook to disable location strategy (flutter/engine#18969)

* 336d000 Roll Skia from f69e40841eb9 to 553496b66f12 (2 revisions) (flutter/engine#18992)

* b82769b Roll Skia from 553496b66f12 to 0ad37b87549b (2 revisions) (flutter/engine#18993)

* f5ca58b Roll Dart SDK from e62b89c56d01 to b0d2b97d2cd7 (4 revisions) (flutter/engine#18994)

* 2a82a08 [dart] Account for compiler api change (flutter/engine#19002)

* 369e0a9 Add ui_benchmarks (flutter/engine#18945)

* a0a92d6 Roll Skia from 0ad37b87549b to de175abede4d (1 revision) (flutter/engine#18995)

* cea5a9c Roll Dart SDK from b0d2b97d2cd7 to f043f9e5f6ea (6 revisions) (flutter/engine#18997)

* d417772 Roll Fuchsia Mac SDK from gAD3P... to Wj0yo... (flutter/engine#19001)

* 4538622 Roll Skia from de175abede4d to 32d5cfa1f35e (15 revisions) (flutter/engine#19005)

* 71fce02 Fix shift-tab not working by adding more GTK->GLFW key mappings. (flutter/engine#18988)

* 5ddc122 Fix inverted check in creating resource surface (flutter/engine#18989)

* 87d8888 Show warning if method response errors occur and error not handled. (flutter/engine#18946)

* 5171fbd Roll Skia from 32d5cfa1f35e to 21bbfc6c2dfe (5 revisions) (flutter/engine#19006)

* 4bd6aea Always send key events, even if they're used for text input. (flutter/engine#18991)

* 9f898e9 Don't process key events when the text input is not requested (flutter/engine#18990)

* 48d7091 Roll buildroot for Windows warning update (flutter/engine#19000)

* 5f37029 Roll Dart SDK from f043f9e5f6ea to f0ea02bc51f8 (22 revisions) (flutter/engine#19010)

* ac809b4 onBeginFrame JNI (flutter/engine#18866)

* e1c622b Make SKSL caching test work on Fuchsia on arm64 (flutter/engine#18572)

* 2651beb Exit before pushing a trace event when layer tree holder is empty (flutter/engine#19008)

* acf048b Remove log added for local testing (flutter/engine#19012)

* 1f3aa23 Roll Dart SDK from f0ea02bc51f8 to 0b64f5488965 (9 revisions) (flutter/engine#19013)

* 8dcc95d Roll Fuchsia Mac SDK from Wj0yo... to gR0Zc... (flutter/engine#19015)

* f6455fa Roll Dart SDK from 0b64f5488965 to 50836c171e91 (4 revisions) (flutter/engine#19017)

* b2fea9d Roll Skia from 21bbfc6c2dfe to 30212b7941d6 (6 revisions) (flutter/engine#19009)

* 0065646 Roll Skia from 30212b7941d6 to 3d6bf04366f6 (17 revisions) (flutter/engine#19020)

* 0a852d8 Revert "Call Shell::NotifyLowMemoryWarning on Android Trim and LowMemory events (flutter#18979)" (flutter/engine#19023)

* 194acdf apply null safety syntax to mobile dart:ui (flutter/engine#18933)

* b0a0e0e Roll Skia from 3d6bf04366f6 to 637838d20abd (2 revisions) (flutter/engine#19021)

* be499ab Roll Fuchsia Mac SDK from gR0Zc... to H-uAk... (flutter/engine#19022)

* 8c24c41 Roll Skia from 637838d20abd to ac16760df463 (1 revision) (flutter/engine#19025)

* 7cb7003 onEndFrame JNI (flutter/engine#18867)

* e3fdb23 [fuchsia] Add ability to configure separate data and asset dirs (flutter/engine#18858)

* 983b6e1 Call Shell::NotifyLowMemory when backgrounded/memory pressure occurs on Android (flutter/engine#19026)

* f7d241f Wire up channel for restoration data (flutter/engine#18042)

* e84d497 Fix hit testing logic in fuchsia a11y (flutter/engine#19029)

* 801559a Revert to last-known-good-rev of Dart SDK (flutter/engine#19031)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jason-simmons jason-simmons jason-simmons approved these changes

@gaaclarke gaaclarke Awaiting requested review from gaaclarke

Assignees
No one assigned
Labels
affects: tests cla: yes perf: speed Performance issues related to (mostly rendering) speed severe: performance Relates to speed or footprint issues. waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@liyuqian @jason-simmons @googlebot @fluttergithubbot