Implement Estimated Input Latency prototype metric |
|||||
Issue descriptionIt should be implemented both as an UMA and in trace viewer metrics panel. Some discussion here: https://docs.google.com/document/d/1b9slyaB9yho91YTOkAQfpCdULFkZM9LqsipcX3t7He8/edit?pref=2&pli=1#heading=h.gnf9l43pafqk and here: https://docs.google.com/document/d/11aFLCWTgfLgbi_5cJT4pejzklRmgWYRGpXOgPO5xH-k/edit?pref=2&pli=1#heading=h.rs8xbnuavmty
,
Jul 25 2016
,
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.
,
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.
,
Sep 6 2016
Thank you for the update! Yes, two weeks would work for me :) How can I find the Lighthouse implementation?
,
Sep 6 2016
It's spread out between these two files: https://github.com/GoogleChrome/lighthouse/blob/8f42309ff86dc08b70ef2dcabe6ad2e03de08bab/lighthouse-core/audits/estimated-input-latency.js and https://github.com/GoogleChrome/lighthouse/blob/8f42309ff86dc08b70ef2dcabe6ad2e03de08bab/lighthouse-core/lib/traces/tracing-processor.js#L185
,
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)?
,
Sep 7 2016
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.
,
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?
,
Sep 7 2016
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?
,
Sep 7 2016
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.
,
Sep 7 2016
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
,
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
,
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
,
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
,
Nov 8 2016
dproy@, is the current plan to split up https://codereview.chromium.org/2323533003 and land it in smaller chunks?
,
Nov 8 2016
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.
,
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?
,
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.
,
Nov 15 2016
I see. Thank you!
,
Feb 3 2017
,
Oct 3 2017
,
Mar 16 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dproy@chromium.org
, Jul 25 2016Owner: dproy@chromium.org