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

Issue 717629 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
inactive
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 719582
issue 719605



Sign in to add a comment

Report input latency metrics via UKM/RAPPOR in Mus+Ash

Project Member Reported by mfomitchev@chromium.org, May 2 2017

Issue description

RAPPOR Service is used to report some of the scroll latency metrics. It is being replaced with UKM. Both live in the browser, and need to report metrics on GPU Swap, e.g. see   RenderWidgetHostLatencyTracker::ReportRapporScrollLatency(), ChromeContentBrowserClient::GetRapporService().

In order for this reporting to work with Mus, we need to do this reporting from the Mus(viz) process, e.g. in LatencyTracker::ReportRapporScrollLatency(). 

This means we either need mojo API for the UKM/RAPPOR Service, or we need to add a callback to the browser on GPU Swap when there is latency information available (which seems undesirable).

UKM Client side design doc:
https://docs.google.com/document/d/19cNXkhhm9npbZPnob8Ba_LgE2dFWNwKajLmf5VnAsFU/edit#heading=h.tosb3mqhyhr6



 
Owner: oysteine@chromium.org
Status: Assigned (was: Available)
I wouldn't say that UKM is replacing RAPPOR - RAPPOR has benefits over UKM (they ahve different populations due to different privacy requirements).

We have plans to add a mojo interface for UKM. Assigning to oystein@
Blockedon: 719605 719582
Owner: mfomitchev@chromium.org
Summary: Report input latency metrics via UKM/RAPPOR in Mus+Ash (was: UKM/RAPPOR in Mus+Ash: Mojo API for UKM?)
 Issue 719582  tracks the work for creating the Mojo interface for UKM.
 Issue 719605  tracks work for adding UKM alternatives input latency metrics that are currently reported via RAPPOR. Input Dev team will then consider getting rid of the RAPPOR input latency metrics.

We need to make sure that the new UKM metrics work in Mus+Ash. Then hopefully we don't have to worry about the existing RAPPOR latency metrics.

I am taking this issue back and going to use it to track Mus+Ash-specific work for this (if any is needed)

Comment 3 by sky@chromium.org, Jun 8 2017

Blocking: 731255

Comment 4 by sky@chromium.org, Oct 11 2017

Mikhail, is there still work that needs to be done here for mushrome?
Cc: nzolghadr@chromium.org
The way  issue 719605  was addressed - UKM metrics are only reported when RenderWidgetHostLatencyTracker is used, i.e. when the display compositor is in the browser process, so it doesn't work for Mushrome.

@nzolghadr - can you think of any reason we can't put ReportUkmScrollLatency implementation directly into LatencyTracker?
At the time I was adding RAPPOR there was no LatencyTracker if I recall and there was only RenderWidgetHostLatencyTracker. Then later that you created LatencyTracker you kept the implementation in RenderWidgetHostLatencyTracker. As far as I can think there shouldn't be any problem with moving the implementation of reports up as long as the parameters and functions make sense. 
RAPPOR reporting was (and still is) using RapporService, which is only available in the browser process. I see that UKM reporting is similarly using UkmService, which is currently only initialized for the browser process. Do you know if it would be possible/easy to also initialize it for the GPU process?

Comment 8 by rkaplow@google.com, Oct 12 2017

We have a mojo interface for UKM now. That should allow you to call UKM from within the gpu process. We have updated docs at go/ukm-api
Blocking: -731255
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 9 2017

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

commit 2149a8f76f4eeb86d92f55ba32922a4b1fb60902
Author: Mikhail Fomitchev <mfomitchev@chromium.org>
Date: Thu Nov 09 17:45:43 2017

Moving UKM scroll latency reporting from RWHLatencyTracker to LatencyTracker.

We want UKM scroll latency reporting to work for --enable-viz (i.e. for the
case when display compositor is in the GPU process). This means that we need to
report these metrics in LatencyTracker::ReportUkmScrollLatency, not in
RWHLatencyTrcker::ReportUkmScrollLatency.

This CL adds ukm::SourceId to LatencyInfo. The source id is set on the
LatencyInfo object in RWHLatencyTracker::OnInputEvent. This way LatencyTracker
(which does not have access to RWHDelegate) can use the ukm::SourceId from
LatencyInfo when reporting UKM scroll latency.

TBR=sadrul@chromium.org

Bug:  717629 
Change-Id: Ib449bcaaf6153b67d2ffdf2f28bf4749f78c80fa
Reviewed-on: https://chromium-review.googlesource.com/744702
Commit-Queue: Mikhail Fomitchev <mfomitchev@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515202}
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/components/viz/service/display_embedder/display_output_surface.cc
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/content/browser/renderer_host/input/render_widget_host_latency_tracker.cc
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/content/browser/renderer_host/input/render_widget_host_latency_tracker.h
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/services/metrics/public/cpp/ukm_recorder.h
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/BUILD.gn
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/DEPS
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/ipc/latency_info_param_traits.cc
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/ipc/latency_info_param_traits_unittest.cc
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/latency_info.cc
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/latency_info.h
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/latency_tracker.cc
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/latency_tracker.h
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/mojo/latency_info.mojom
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/mojo/latency_info_struct_traits.cc
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/mojo/latency_info_struct_traits.h
[modify] https://crrev.com/2149a8f76f4eeb86d92f55ba32922a4b1fb60902/ui/latency/mojo/struct_traits_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 13 2017

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

commit 742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb
Author: Mikhail Fomitchev <mfomitchev@chromium.org>
Date: Mon Nov 13 19:55:00 2017

Initialize a global instance of MojoUkmRecorder for the GPU process.

We need to be able to log UKM metrics from the compositor thread of the GPU
process, so initialize a global instance of MojoUkmRecorder to do that.

TBR=sky@chromium.org
(for DEPS change)

Bug:  717629 
Change-Id: I69bedeed7c16bb69b50b7e12da590ba1c69a3c9c
Reviewed-on: https://chromium-review.googlesource.com/744544
Commit-Queue: Mikhail Fomitchev <mfomitchev@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516015}
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/components/viz/service/main/BUILD.gn
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/components/viz/service/main/DEPS
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/components/viz/service/main/viz_main_impl.cc
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/components/viz/service/main/viz_main_impl.h
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/content/public/app/mojo/content_gpu_manifest.json
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/services/metrics/public/cpp/mojo_ukm_recorder.cc
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/services/metrics/public/cpp/mojo_ukm_recorder.h
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/services/viz/main.cc
[modify] https://crrev.com/742e391ee9b9e8d051ad73d3eefb74fc9dcf0bdb/services/viz/service.cc

Status: Fixed (was: Assigned)
UKM  works with Mus/Viz now, and RAPPOR is more or less deprecated, so I am going to consider this fixed.

Sign in to add a comment