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

Issue 851228 link

Starred by 2 users
Status: Started
Owner:
sadrul@chromium.org
Cc:
nzolghadr@chromium.org
█ briander...@chromium.org
tdres...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

LatencyInfo: Simplify LatencyMap

Project Member Reported by sadrul@chromium.org, Jun 9 2018 
LatencyInfo::LatencyComponent [1] is there to allow the same component to be added more
than once into the same LatencyInfo instance. However, it doesn't look like we ever do this. So |event_count| always remains 1, and |first_event_time| and |last_event_time| are always the same as |event_time|. Investigate if LatencyComponent can be removed.

[1] https://cs.chromium.org/chromium/src/ui/latency/latency_info.h?l=110
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 11 2018 

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

commit c582f29cee30691fb70f5a31937baf38d8b2b701
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Mon Jun 11 18:14:57 2018

latency: Remove LatencyComponent, and simplify LatencyInfo.

LatencyComponent is there to allow the same component to be added more
than once into the same LatencyInfo instance. However, we never do this.
So |event_count| is always 1, which also means |first_event_time| and
|last_event_time| are the same as |event_time|. So:
 . Remove |event_count|.
 . Remove |first_event_time| and |last_event_time|, and update the usage
   to use |event_time| instead.
 . Remove LatencyComponent altogether, since it only has a single
   timestamp. Change LatencyMap to be a map from LatencyComponentType to
   a timestamp.

BUG=851228

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Iae15de50eb95e6bbab5acb8347ee48efe94ac271
Reviewed-on: https://chromium-review.googlesource.com/1093016
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566061}
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/content/browser/renderer_host/input/render_widget_host_latency_tracker.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/content/common/input/event_with_latency_info_unittest.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/services/viz/public/cpp/compositing/struct_traits_unittest.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/ipc/latency_info_param_traits_macros.h
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/ipc/latency_info_param_traits_unittest.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/latency_histogram_macros.h
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/latency_info.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/latency_info.h
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/latency_info_unittest.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/latency_tracker.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/latency_tracker.h
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/mojo/latency_info.mojom
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/mojo/latency_info.typemap
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/mojo/latency_info_struct_traits.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/mojo/latency_info_struct_traits.h
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/mojo/struct_traits_unittest.cc
[modify] https://crrev.com/c582f29cee30691fb70f5a31937baf38d8b2b701/ui/latency/mojo/traits_test_service.mojom

Comment 2 by npm@chromium.org, Sep 25 

Can this be marked as fixed now?

Comment 3 by maxlg@chromium.org, Jan 15 

Is it fixed?

Comment 4 by tdres...@chromium.org, Jan 15

Components: -Speed>Metrics
►

Sign in to add a comment