New issue
Advanced search Search tips

Issue 837460 link

Starred by 2 users
Status: Verified
Owner:
ntfschr@chromium.org
Closed: Apr 2018
Cc:
paulmiller@chromium.org
changwan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Cache PlatformServiceBridge#canUseGms

Project Member Reported by ntfschr@chromium.org, Apr 27 2018 
The downstream implementation of PlatformServiceBridge#canUseGms is pretty expensive (2.5 ms according to recent benchmarks [1]), so it would be great if we could cache this to avoid recomputing the same value.

Fortunately, we compute this during the constructor. We can create canUseGmsInternal to compute the value, and turn canUseGms() into an accessor.

AwSafeBrowsingApiHandler relies on calling canUseGms() during its init() method, so this may actually improve startup performance (I'll investigate if there is any positive impact).

I'll post any performance measurements to https://docs.google.com/spreadsheets/d/1TznEiJKznfQ3jzSGx6H_1MSdSEvExswRtyRzOp2hn-U/edit?usp=sharing

[1] https://android-webview.teams.x20web.corp.google.com/startup/load_chrome_blank_2018-04-20_02-27-26_78511.html
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 28 2018 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/13da4d3a3f876cebcfceb769bc1ad514b9b449ee

commit 13da4d3a3f876cebcfceb769bc1ad514b9b449ee
Author: Nate Fischer <ntfschr@google.com>
Date: Sat Apr 28 01:28:05 2018

Comment 2 by ntfschr@chromium.org, Apr 28 2018

Blocking: -817644
Status: Fixed (was: Started)
Hmm it's hard to measure startup improvement, because our startup script loads about:blank, and we don't perform Safe Browsing checks for that URL (so we only call canUseGms() once).

I'll remove the blocking issue, since this doesn't contribute to WebViewStartupInterval.

Comment 3 by sbash...@chromium.org, May 7 2018 

Please add the manual verification steps to verify the fix.

Comment 4 by ntfschr@chromium.org, May 7 2018

Status: Verified (was: Fixed)
No manual verification necessary--this is not expected to change behavior, it just provides a slight perf win.

If this change had a bug, it would mean that Safe Browsing would be completely broken. I verified locally that Safe Browsing continues to function.
►

Sign in to add a comment