I guess this is related to  crbug.com/613419 ? or a duplicate thereof

That is probably related, I'll sync with Yoland to avoid any potential collisions/duplicated work. But note that this issue is specifically for the changes needed in the WebView shell apk (not telemetry).

perezju, did you sync with Yoland? do we need a separate bug here.

This sounds like the same thing; the webview shell is what contains the telemetry activity, and I don't think there's going to be any difference which activity we measure this in..

I think  issue 613419  is a duplicate of 614371, I have a meeting later with Yoland today. But, either way, we need this change in the WebView shell.

 Issue 613419  has been merged into this issue.

Can we add the trace instrumentation in webview's code & not the embedder code? That way we can get data for arbitrary embedder of webview.

probably. Bo you deal with traces all the time. wdyt?

> Can we add the trace instrumentation in webview's code & not the embedder code? That way we can get data for arbitrary embedder of webview.

Hmm, there definitely are a lot of stuff that runs in the OS to initialize webview before hitting any chromium code. But then those are specific to the OS, and should not be affected chromium changes (I would hope?), so we probably should not be monitoring perf in chromium bots.

So maybe ok to instrument the chromium part only? opinions here torne?

I see in https://codereview.chromium.org/1994403002/, we add the instrumentation as follows:
...
Trace.beginSection(startUpTraceTag == null ? DEFAULT_START_UP_TRACE_TAG : startUpTraceTag);
WebView webView = new WebView(this);
Trace.endSection();
...

Instead, I propose that we move the Trace.beginSection & Trace.endSection calls inside the android.webkit.WebView's constructor. 

android.webkit.WebView's constructor is in android code though, not chromium

I see. Seems like we are stuck with setting it in the embedder code in the short-term. Though is it possible for us to upstream the trace probes there? If Android regress the implementation of non-chromium part of WebView, do you folks think it make sense for us to also track that?

Unfortunately it's definitely *not* the case that the OS's webview initialisation code is unaffected by chromium changes. The framework loads and executes both Java and native code from the WebView APK, and the time it takes to do this depends on exactly what our code looks like. One significant example we've already had is relocation packing, which makes a large difference to library load time. We could certainly add trace instrumentation in the chromium code *as well*, to track specific parts of the initialisation, but I think we still need the top-level metric too.

Also, we do want to be able to compare this metric across different Android versions, and it will be easiest if we can do this consistently so we are not making an unfair comparison. We won't be able to compare to L/M fairly if we rely on having tracing in the WebView constructor, since those don't have that.

I don't think being able to run this with different apps is actually useful. There's almost nothing we do during this time which should be affected by the embedder - pretty much the only way that it can have any effect on this is "how big is the webview cache/cookiejar/etc", which if we want to test, we could test much more reliably by constructing artificial (and consistent) scenarios instead of relying on real apps' behaviour which are likely to be non-repeatable. Any benchmarking on real apps is just going to be noisier.

@torne: the ability to compare across different Android versions is a fair point. I agree with putting the instrumentation the webview shell is the way to go now. Looks like this bug is mostly done with Yoland' patch then?


+Alex, Charlie: with Torne's point about we need both top-level & chromium level data, it seems like beyond the basic startup metrics, we also need to get clock sync between Chromium & Systrace working well to further explore other sub metrics for startup too.

We could probably do most things without clock sync. We can measure the total entirely on the systrace side (using the change implemented here), we can measure various parts of the framework initialisation on the systrace side (using already-existing systraces in android.webkit.WebViewFactory), and we can measure most parts of the chromium init entirely on the chromium side. I don't think actually having the two sides line up is necessary for most of this, since the respective pairs of start/end times would be using the same clock source. There might be some gaps that aren't covered by a specific trace doing it this way, but we'd still see changes in the "sizes" of the gaps by seeing when the overall time changes without the individual components changing enough to explain it?

Yoland, is this bug now done? Looks like all required pieces are now in place, right?

Yes