New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 899983 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 18
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

19.6%-31.1% regression in webview_startup_cpu_time,webview_startup_wall_time at 603401:603425

Project Member Reported by 42576172...@developer.gserviceaccount.com, Oct 29

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=899983

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=0693618411ea10cd80ccf2e5a838f175fd2fe0fa9ec7db3632348612bc6eb4d7


Bot(s) for this bug's original alert(s):

Android Nexus5X WebView Perf
Android Nexus6 WebView Perf

system_health.webview_startup - Benchmark documentation link:
  None
Cc: js...@chromium.org
Owner: js...@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/11ac3bd9e40000

Roll ICU to ICU 63.1 + Chromium patches by jshin@chromium.org
https://chromium.googlesource.com/chromium/src/+/5afd3007276ae994a1c9fc5bb7da7363babb76e3
webview_startup_cpu_time: 124.6 → 159.8 (+35.24)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  None
Cc: -js...@chromium.org aheninger@google.com mscherer@google.com
Markus and Andy, can you think of anything in ICU 63.1 (not present in ICU 62.1) that can slow down the "initial loading' by 17% ~ 30% on mobile devices (Nexus 5X, Nexus 6). This 30% jump occurred when ICU was updated from 62.1 to 63.1. 

Sorry for being vague. I'm not sure exactly what part of ICU is used by the time webview is started with apparently an empty page.  The data file has to be mmap'd. Timezone has to be set up. Perhaps, BreakIterators (line and grapheme) may have to be created (even though it's an empty page)
Is it possible to get profile data out of  perf bot? If not,  how can I profile the webview start-up on Android?  


It looks like it might be due to a change I made in UnicodeSet. I submitted

https://unicode-org.atlassian.net/browse/ICU-20250 "ICU 63 UnicodeSet startup performance regression"

and will start testing/measuring/experimenting.
Status: ExternalDependency (was: Assigned)
The upstreaming is looking into an issue. A likely culprit was identified. 

https://unicode-org.atlassian.net/browse/ICU-20250  

Jungshik -- The pinpoint report gives you the trace: 
Go to https://pinpoint-dot-chromeperf.appspot.com/job/11ac3bd9e40000 and click on "trace" link with and without your patch. 
That pinpoint report didn't help much, unfortunately. 

However,  bug 901532  (20ms regression in Windows render process start-up) comes with a detailed profile data and points exactly where the icu bug in comment 7 deals with. 


I made the CodePointTrie builder faster, see https://github.com/unicode-org/icu/pull/265

Please let me know what this does for webview_startup_cpu_time, compared with ICU 62 and ICU 63.
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/deps/icu.git/+/45f655f2feb7069a7b9b47d1b1a596807bfd4220

commit 45f655f2feb7069a7b9b47d1b1a596807bfd4220
Author: Jungshik Shin <jshin@chromium.org>
Date: Thu Nov 08 06:23:05 2018

Cherry-pick: UnicodeSet performance enhancement

The start-up performance regression reported for Android
WebView and Windows Chrome is traced to UnicodeSet creation
for Unicode character properties.

This upstream PR speeds up UnicodeSet creation.

Upstream bug:
  https://unicode-org.atlassian.net/browse/ICU-20250

Upstream PR: https://github.com/unicode-org/icu/pull/265

TBR=ftang@chromium.org

Bug:  899983 , 901532 
Test: Android webview start-up perf.
Change-Id: Iffb6eac4ddde2cabd2f55fe718a9b4e3f39b4e05
Reviewed-on: https://chromium-review.googlesource.com/c/1325398
Reviewed-by: Jungshik Shin <jshin@chromium.org>

[modify] https://crrev.com/45f655f2feb7069a7b9b47d1b1a596807bfd4220/README.chromium
[add] https://crrev.com/45f655f2feb7069a7b9b47d1b1a596807bfd4220/patches/uniset_perf.patch
[modify] https://crrev.com/45f655f2feb7069a7b9b47d1b1a596807bfd4220/source/common/umutablecptrie.cpp

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/169ffa2be40000

All of the runs failed. The most common error (1/20 runs) was:
IOError: [Errno 2] No such file or directory: '/b/swarming/w/itf1i4bd/tmpB3xyiJtelemetry/histograms.json'
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/16d00c2ee40000

All of the runs failed. The most common error (1/20 runs) was:
IOError: [Errno 2] No such file or directory: '/b/s/w/it0KZW2k/tmpNdQ5Httelemetry/histograms.json'
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/13c4b875e40000

All of the runs failed. The most common error (20/20 runs) was:
PathMissingError: Unable to find apk specified by --webview-embedder-apk=None
Ushesh, Android pinpoint has been failing for several hours (since around 20:00 in US Pacific Time).  Where should I report that?  

https://chrome-swarming.appspot.com/bot?id=build224-b7&sort_stats=total%3Adesc
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/16d77bdbe40000

All of the runs failed. The most common error (20/20 runs) was:
PathMissingError: Unable to find apk specified by --webview-embedder-apk=None
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/1696bd4fe40000

All of the runs failed. The most common error (20/20 runs) was:
PathMissingError: Unable to find apk specified by --webview-embedder-apk=None
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/158b414fe40000

All of the runs failed. The most common error (20/20 runs) was:
PathMissingError: Unable to find apk specified by --webview-embedder-apk=None
Project Member

Comment 25 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f2d5503db76711b8738358ee2272b3ac6fc3d208

commit f2d5503db76711b8738358ee2272b3ac6fc3d208
Author: Jungshik Shin <jshin@chromium.org>
Date: Fri Nov 09 02:25:36 2018

Roll ICU to 45f655f

https://chromium.googlesource.com/chromium/deps/icu.git/+log/834113a..45f655f

45f655f Cherry-pick: UnicodeSet performance enhancement
d2afb05 Delete another empty block after trim data

TBR=ftang@chromium.org,gsathya@chromium.org

Bug:  899983 , 901532 , v8:8414 
Test: Android webview start-up perf
Test: v8: intl/regress-8414.js
Change-Id: Ibc3e84fb26c4b3a158823e9ce6cee709b3161da7
Reviewed-on: https://chromium-review.googlesource.com/c/1325564
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606705}
[modify] https://crrev.com/f2d5503db76711b8738358ee2272b3ac6fc3d208/DEPS

I didn't have much luck with pinpoint today. The CL was landed and will see how the graph goes. 
Ushesh,  the graphs at https://chromeperf.appspot.com/group_report?bug_id=899983
extend only up to 20181107T23:00Z (about 40 hours ago).  Perhaps, the delay of 24+ hours (because build and test run take time for each CL ? ) is expected.  
I don't know the specifics of how long it takes for the data to show up in those graphs
https://chromeperf.appspot.com/group_report?sid=0693618411ea10cd80ccf2e5a838f175fd2fe0fa9ec7db3632348612bc6eb4d7  
has the latest pinpoint data:

* Nexus 5X cpu_time (load_chrome_blank)
  a. Before ICU 63.1 : 124 +- 5 ms  ;  603400
  b. Right after ICU 63.1 roll: 157 +- 6 ms   ; 603410 
  c. Right before ICU roll with a cherry-pick: 156 +- 4.8 ms  ; 606030
  d. After ICU roll with c.p : 131 +- 3.8 ms  ; 607096

* Nexus 6 cpu_time (load_chrome_blank)
  a. 146.5 +- 2 ms    ; 603409 
  b. 173.3 (1.6) ms   ; 603425
  c. 180.1 (2.7) ms   ; 606075
  d. 162.5 (2.0) ms   ; 607124 

We reverted about 60~70% of the perf regression.  


Issue 901834 has been merged into this issue.
I just merged https://github.com/unicode-org/icu/pull/278 into upstream ICU. Should be faster now than ICU 62.
Project Member

Comment 32 by bugdroid1@chromium.org, Nov 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/deps/icu.git/+/d13a96f4d6e4e4241b8e7aa346a857c206564d9c

commit d13a96f4d6e4e4241b8e7aa346a857c206564d9c
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Nov 14 17:25:24 2018

Unicoset Perf fix #2

Cherry-pick another upstream patch to improve the performance
of UnicodeSet.

https://github.com/unicode-org/icu/pull/278

Bug:  899983 , 901532 
Test: Android webview start-up perf and Windows perf graph
Change-Id: I2c2a22a398883178734a2c2a6c5975d9551b039e
TBR=ftang@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1335799
Reviewed-by: Jungshik Shin <jshin@chromium.org>

[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/README.chromium
[add] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/patches/uniset_perf2.patch
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/characterproperties.cpp
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/ucptrie.cpp
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/umutablecptrie.cpp
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/unicode/uniset.h
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/uniset.cpp
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/uniset_closure.cpp
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/uniset_props.cpp
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/uprops.h
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/uset.cpp
[modify] https://crrev.com/d13a96f4d6e4e4241b8e7aa346a857c206564d9c/source/common/usetiter.cpp

Project Member

Comment 33 by bugdroid1@chromium.org, Nov 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7344a24efd2a222d6d45b709767cd4d050af10a3

commit 7344a24efd2a222d6d45b709767cd4d050af10a3
Author: Jungshik Shin <jshin@chromium.org>
Date: Sun Nov 18 01:37:31 2018

Roll ICU to 407b393 from 45f655f

  https://chromium.googlesource.com/chromium/deps/icu.git/+log/45f655f..407b393

The following changes are included:

  407b393 Update IANA timezone db to 2018g
  797b7c7 Add more locale variants
  d13a96f Unicoset Perf fix #2
  ecae5c0 Adjust calendar locale data trimming on Andorid

The ICU data files for mobile platforms got smaller. For desktop,
the size got increased by ~59 kb for more locale variant support.

TBR=ftang@chromium.org

Bug: 473288,863749, 899983 , 901532 , v8:8432 
Test: v8: intl/regress-8413*
Test: Android webview start-up perf and Windows perf graph
Test: See  crbug.com/900232 
Test: See ICU 407b393 CL description for tz test.
Change-Id: Id3ec92cbbe82275b120ffa9747e110db598905dc
Reviewed-on: https://chromium-review.googlesource.com/c/1341468
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609129}
[modify] https://crrev.com/7344a24efd2a222d6d45b709767cd4d050af10a3/DEPS

As of now, th latest pinpoint available at https://chromeperf.appspot.com/group_report?bug_id=899983 is 609124 (5 CLs before the above ICU roll, which is 609129). Perhaps, in a few hours, new numbers will be available. 
At commit position 609133 (first one after the latest ICU roll), the Nexus 5X cpu_time is 126ms ± 3.5ms. This is roughly the same as it was before ICU 63 with 124±5ms.

I was hoping for even a little lower... but I guess it's back to as good as it gets.

No Nexus 6P time yet.
Status: Fixed (was: ExternalDependency)
Thank you, Markus, for checking the pinpoint and, needless to say, making two quick fixes !

Before the initial report, it was 123 (sigma=4). After the latest ICU roll ,it's 126(sigma 3.5). I'd declare that this issue is fully resolved.   
Cc: nedngu...@google.com crouleau@chromium.org js...@chromium.org
 Issue 901532  has been merged into this issue.
At commit position 609145 (first one after the latest ICU roll), the Nexus 6 cpu_time is 154ms ± 1.9ms. A little higher than the 147±2.2 before ICU 63, but after ICU 63 and before the first ICU improvement the time had trended up a little too (roughly from 173 to 180), so the extra few ms may not (all) be due to ICU.
Status: Verified (was: Fixed)
Thanks again, Markus. I agree with you on comment 38. 

Sign in to add a comment