New issue
Advanced search Search tips

Issue 631206 link

Starred by 4 users

Issue metadata

Status: WontFix
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 625701



Sign in to add a comment

Implement Estimated Input Latency prototype metric

Project Member Reported by dproy@chromium.org, Jul 25 2016

Issue description

Comment 1 by dproy@chromium.org, Jul 25 2016

Labels: -Pri-3 Pri-1
Owner: dproy@chromium.org
Status: Assigned (was: Untriaged)
Summary: Implement Estimated Input Latency prototype metric (was: Implement Estimated Input Latency prtototype metric)

Comment 3 by u...@chromium.org, Sep 2 2016

What is the current status of trace viewer EIL metric implementation?

I am currently working on GC optimization and very interested in measuring its impact on EIL.

I'd be happy to help with implementing the metric.

Comment 4 by dproy@chromium.org, Sep 2 2016

This just moved up on my queue - I should have something in the next two weeks. Does that work for you? 

The metric is already implemented in Lighthouse. The implementation needs a small fix, but then it should be straightforward to make a mirror implementation in traceviewer.

Comment 5 by u...@chromium.org, Sep 6 2016

Cc: u...@chromium.org
Thank you for the update! Yes, two weeks would work for me :)

How can I find the Lighthouse implementation?

Comment 7 by u...@chromium.org, Sep 7 2016

Thank you!

I took a look at the code. It seems to compute the percentiles of queueing times (as described in "Alternatives" section of https://docs.google.com/document/d/1b9slyaB9yho91YTOkAQfpCdULFkZM9LqsipcX3t7He8/edit).

Does this bug also cover implementing the expected queueing time together with diagnostic metrics ("Diagnostic Metrics" section of the document)?
Yeah, we'll need to add the average queueing time to lighthouse at some point. We should add the average queueing time to TBMv2.

This bug doesn't include implementing the diagnostic metrics, though my understanding is that it should be pretty straight forward in TBMv2.

Comment 9 by u...@chromium.org, Sep 7 2016

I see, thank you.

Would it be OK if I file a new bug for adding average queueing time with diagnostic metrics to TBMv2 and start working on it?

Computing average queueing time over the whole timeline is easy.

But I guess you would prefer the maximum of average queueing time in 5 second sliding window as described in the document?
Having both implementations around would be reasonable (max 5 second window, and whole timeline).

I'm not sure where Deep is at in implementation, but I'm sure we can collaborate. Deep, what would a good division of work be?
I won't mind implementing them myself, but if it's urgent, yes it would be great to have the extra help in implementing diagnostic metrics! I haven't given them too much thought.

I'm hoping to send out a first rough CL in a few hours that (almost) mirrors the lighthouse implementation, although it will need to go through a couple iterations before it will be ready to land. I'll CC you to the CL, and I think it's better if you wait till you see the CL before starting. There is some subtleties around what constitutes a task length because of the renderer scheduler that may require some discussion, and it'll also be easier to figure out how to divide the work after you see where I'm at. 
Computing diagnostic metrics (at least the category breakdown) should be as simple as plugging into:

https://cs.chromium.org/chromium/src/third_party/catapult/tracing/tracing/extras/chrome/chrome_user_friendly_category_driver.html?rcl=0&l=18
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 7 2016

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

commit 0a1f070fa70c5b4984dafa31e66a4435b9b8c0c0
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Fri Oct 07 18:34:19 2016

Roll src/third_party/catapult/ b72737924..b1d2bdcb8 (3 commits).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/b72737924a91..b1d2bdcb8df1

$ git log b72737924..b1d2bdcb8 --date=short --no-merges --format='%ad %ae %s'
2016-10-07 eakuefner Revert of [Telemetry] Add 'check_independent' command to benchmark_runner (patchset #4 id:60001 of https://codereview.chromium.org/2291313002/ )
2016-10-07 dproy postInteractiveTaskWindows function for EIL metric
2016-10-07 zhenw Revert of Update systrace and profile_chrome's default agent behavior (patchset #1 id:1 of https://codereview.chromium.org/2391383004/ )

BUG= 631206 , 653898 

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2406523002
Cr-Commit-Position: refs/heads/master@{#423916}

[modify] https://crrev.com/0a1f070fa70c5b4984dafa31e66a4435b9b8c0c0/DEPS

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 17 2016

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

commit 9772d67f6a930b7c2392dba12cf7790ae3dd45fd
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Mon Oct 17 19:43:19 2016

Roll src/third_party/catapult/ 60871179a..9a96d45e2 (1 commit).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/60871179ac16..9a96d45e278e

$ git log 60871179a..9a96d45e2 --date=short --no-merges --format='%ad %ae %s'
2016-10-17 dproy Function to get navigationStart timestamps for EIL

BUG= 631206 

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2424013003
Cr-Commit-Position: refs/heads/master@{#425752}

[modify] https://crrev.com/9772d67f6a930b7c2392dba12cf7790ae3dd45fd/DEPS

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 19 2016

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

commit 43317a84e77057745b252499905f06ed3e7b11ca
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Wed Oct 19 18:03:08 2016

Roll src/third_party/catapult/ 255286c7a..f2c0fd71e (2 commits).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/255286c7a3bb..f2c0fd71e5d2

$ git log 255286c7a..f2c0fd71e --date=short --no-merges --format='%ad %ae %s'
2016-10-19 dproy Helper function for get interactive timestamps
2016-10-19 hjd [telemetry] Install PushAppsToBackground apk

BUG= 631206 , 586148 

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=catapult-sheriff@chromium.org

Review-Url: https://chromiumcodereview.appspot.com/2436783002
Cr-Commit-Position: refs/heads/master@{#426241}

[modify] https://crrev.com/43317a84e77057745b252499905f06ed3e7b11ca/DEPS

Comment 16 by u...@chromium.org, Nov 8 2016

dproy@, is the current plan to split up https://codereview.chromium.org/2323533003 and land it in smaller chunks?
While trying to land that metric I realized that we don't really know what the right definition of estimated input latency should be yet, and it may not be the best thing to do to land several definitions in catapult and eventually confuse everyone. I'm currently working on running experiments with different definitions of Time to Interactive and Estimated Input Latency to find a definition that works for most people, and I plan to land the implementation after we reach that consensus.

This will take a while - perhaps end of this quarter or the beginning of next. In the meantime, if the simplified metric that takes the average queuing time of renderer main thread tasks over the whole timeline works for you and you want it sooner, you could maybe implement it as a v8 metric. The big CL (https://codereview.chromium.org/2323533003) works - if you want you can patch that in as a starting point. Bits and pieces of that CL has landed in extras/chrome (https://cs.chromium.org/chromium/src/third_party/catapult/tracing/tracing/extras/chrome/estimated_input_latency.html) - you can use those too if that is helpful. 

Comment 18 by u...@chromium.org, Nov 15 2016

Thanks a lot for the update, dproy@.

I see that the main document list all the competing definitions with more formal name. Very nice!

Do you think that EILIOfAPageLoad(timeline) is likely to win? Since it has a comment "this is probably our target PWM."

It depends implicitly on TTI, is there document that describes competing definitions of TTI in similar way?

Comment 19 by dproy@chromium.org, Nov 15 2016

Looks like I defined EILIOfAPageLoad as Expected Queueing time in the worst 5 second window after TTI. I don't think it is any more likely than the other ones at this point. Whether we'll go forward with this definition will depend a lot of what definition of TTI we decide to adopt.

We don't have a similar document for TTI definitions yet, but I'm working on one right now. In the meantime, there's the document that describes the classical definition: https://docs.google.com/document/u/1/d/11sWqwdfd3u1TwyZhsc-fB2NcqMZ_59Kz4XKiivp1cIg/edit?usp=drive_web

We're experimenting with varying window sizes and taking network information into account.

Comment 20 by u...@chromium.org, Nov 15 2016

I see. Thank you!
Components: Speed>Metrics
Status: WontFix (was: Assigned)
We're focusing on EQT, not EIL.
See crbug.com/625701.

Comment 23 by dproy@chromium.org, Mar 16 2018

Labels: -progressivewebmetrics

Sign in to add a comment