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

Issue 770710 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 725019
issue 765118



Sign in to add a comment

Run a performance-check to ensure WebView Support Library prototypes are not considerably slower than android.webkit.

Project Member Reported by gsennton@chromium.org, Oct 2 2017

Issue description

To ensure our WebView Support Library approach is feasible we should run a performance check (ensuring that we don't add too much overhead for each WebView call) for a WebView Support Library prototype.

What we could do here is call a single WebView-method lots of times, both for the support library, and for the frameworks WebView.

We should probably run such a test for the KitKat/old WebView path of the support library if we end up supporting such a path.
 
Cc: torne@chromium.org tobiasjs@chromium.org
Alright, I've used the CL in
https://chromium-review.googlesource.com/c/chromium/src/+/677392

to call a single WebView API lots of times, namely postVisualStateCallback.

The API was called 10k times providing the following timings:
189 ms, android.webkit postVisualStateCallback
855 ms, support library postVisualStateCallback

so the support library call is substantially slower.


Not sure whether this affects any of our decisions to go through with the support library - these are very small absolute time differences (less than 0.1ms), and off the top of my head I can't think of any WebView APIs that should be called super-often.
Note: the WebView used in comment #1 is a debug-build.

Comment 3 by boliu@chromium.org, Oct 9 2017

dupeMethod looks to be the most expensive part, can that just happen once and have the result saved?
Good question, we would have to store the 'dupeMethod' for each method we want to call across the support library <-> glue boundary, so we would probably have to store this in some kind of map (or have every location where we create an InvocationHandler store its locally used dupeMethod in a static variable).

I'm gonna see if I can find a profiler showing stack traces to inspect what it is that's actually taking time.
Owner: gsennton@chromium.org
Status: Assigned (was: Available)
dupeMethod takes up 17% of the InvocationHandler.invoke call, the rest is the actual method invocation.

The real culprit here seems to be Proxy.newProxyInstance() - it takes up about half of the entire execution time.
If we use Proxy and InvocationHandler I don't think there's much we can do about that - whenever we need to cast something to an interface (which we have to do in each method call where we pass some support library object, e.g. setWebViewClient, or postVisualStateCallback), using Proxy, we will have to use Proxy.newProxyInstance().



I'll finish looking into how to call between the support library and the glue layer so we can say for sure whether Proxy and InvocationHandler are strictly necessary.
Actually, I'm not sure why the dupeMethod was part of that trace at all (unless the visual state callback was triggered), dupeMethod should only be called when AwContents triggers the callback - never when we just pass a callback to the glue from the support library.

I think I'll have to measure performance from both sides - i.e. both when calling from one side into the other, and when triggering a callback passed from one side to the other.
Haah, this solution is so confusing in terms of what is called when (which is not great in terms of maintainability), anyway - the reason dupeMethod is called inside AugmentedWebView.postVisualStateCallback is because AugmentedWebView is calling mProvider.insertVisualStateCallback - and mProvider, which is of type GlueWebViewChromiumProvider, is also an object called through an InvocationHandler. So the dupeMethod method I'm referring to here is the one defined in the glue, rather than the one defined in the support library (yay confusion! :D).

Call stack:

AugmentedWebView.postVisualStateCallback ->
    Proxy<WebViewProvider>.invoke() ->
        InvocationHandler<WebViewProvider>.invoke() ->
            GlueFactoryProviderFetcher.dupeMethod(insertVisualStateCallbackMethod).invoke(GlueWebViewChromiumProvider) ->
                GlueWebViewChromiumProvider.insertVisualStateCallback() ->
                    Proxy.newProxyInstance(callbackInvocationHandler)
                    AwContents.insertVisualStateCallback()

as noted in comment #5,
GlueFactoryProviderFetcher.dupeMethod takes up ~15-20% of the execution time, while
Proxy.newProxyInstance takes around half of the execution time.

After removing WebViewChromium.checkNeedsPost() from WebViewChromium.insertVisualStateCallback (that was a decent time-consumer), and pre-warming WebView before perf measurments (calling WebView.getCertificate() to trigger WebViewChromiumFactoryProvider.startYourEngines), I got the following results:

including AwContents.insertVisualStateCallback implementation:

1116 ms, android.webkit postVisualStateCallback
4687 ms, support library postVisualStateCallback InvocationHandler
1207 ms, support library postVisualStateCallback Object+reflection


Without AwContents.insertVisualStateCallback implementation (i.e. just purely calling from support library into glue):

679 ms, android.webkit postVisualStateCallback
4145 ms, support library postVisualStateCallback InvocationHandler
726 ms, support library postVisualStateCallback Object+reflection


all of the above numbers are 10k iterations of postVisualStateCallback calls.


So the overhead of calling into WebView APK code is essentially the same between an android.webkit call and a reflection call, but in the InvocationHandler approach this overhead is about 6x.
Given that most WebView APIs are fairly heavy-weight, I don't think adding an overhead like this for each method call will make a huge difference for WebView performance, but I also don't know the entire WebView API surface.

Do we have any methods which might be called very frequently?
Is there some way we can check whether apps use any of our APIs in loops, e.g. through Marmot?

Comment 10 by boliu@chromium.org, Oct 24 2017

probably most frequent are calls are the ones that happen once per draw, which is 60/s, so not high enough to worry too much about

I can't think of any API that requires apps to call in tight loops for a long period of time
Assuming (given comment #8) that the InvocationHandler approach adds around half a millisecond of execution time per call (~4k ms per 10k calls), we're looking at 60 * 0.5ms = 30ms extra execution time per second (for calls happening once per draw). This sounds OK as long as there aren't many of those calls, Bo do you know if we perform several of these, or just one, per draw?
Btw, given that we are just adding new APIs since L, it would surprise me if many of those calls are being made once per draw, but you never know I guess :)
Cc: boliu@chromium.org

Comment 13 by boliu@chromium.org, Oct 26 2017

I think there are maybe 3-4 calls that goes down to every frame. There might be a few calls up as well. But none of these are new since L, so I not really applicable here.

0.5ms per call is pretty slow for things that happen every frame. But doesn't look like we have any of those

Comment 14 by boliu@chromium.org, Oct 26 2017

I think general rule of thumb should be use InvocationHandler as default. And only switch to reflection for that call if we see a need for perf reasons.
Status: Fixed (was: Assigned)
Yeah I think #14 sounds right.

Sign in to add a comment