[GRC] Tab CPU Usage Observer

[GRC] Tab CPU Usage Observer

This is a CoordinationUnitGraphObserver implementation that performs tab-level CPU usage attribution.

Each process is given a namespace via a ProcessObserver class which
itself holds onto a collection of TabObserver class instances that
correspond to the tabs that are contained within it.

Currently the implementation makes some naive assumptions:
- Process CPU is evenly attributable among the tabs within it.
  For example, if a process with two CPU tabs has 50% utilization then
  each tab is attributed with 25%.
- Any parentless-frame is assumed to correspond to a tab. In reality
  a tab (e.g. WebContents) can have have multiple parentless-frames
  because multiple navigations can exist within a single instance.

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306

[matthalp, 2017-06-19 02:46:49]

Description was changed from

==========
Initialize CL

BUG=
==========

to

==========
NOCOMMIT PROTOTYPE [GRC] Tab CPU Usage Observer V2

This is an alternative implementation of the Tab CPU Usage Observer
(https://codereview.chromium.org/2925123002) based on an alternative
implementation of the CoordinationUnitGraphObserver framework
(https://codereview.chromium.org/2942403002).

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306
==========

[matthalp, 2017-06-19 02:46:49]

matthalp@google.com changed reviewers:
+ oysteine@chromium.org, zhenw@chromium.org

[matthalp, 2017-06-19 02:58:16]

Description was changed from

==========
NOCOMMIT PROTOTYPE [GRC] Tab CPU Usage Observer V2

This is an alternative implementation of the Tab CPU Usage Observer
(https://codereview.chromium.org/2925123002) based on an alternative
implementation of the CoordinationUnitGraphObserver framework
(https://codereview.chromium.org/2942403002).

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306
==========

to

==========
NOCOMMIT PROTOTYPE [GRC] Tab CPU Usage Observer: Traditional Observer
Implementation

This is an alternative implementation of the Tab CPU Usage Observer
(https://codereview.chromium.org/2925123002) based on an alternative
implementation of the CoordinationUnitGraphObserver framework
(https://codereview.chromium.org/2942403002).

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306
==========

[lpy, 2017-06-19 04:34:41]

lpy@chromium.org changed reviewers:
+ lpy@chromium.org

[lpy, 2017-06-19 04:34:53]

Description was changed from

==========
NOCOMMIT PROTOTYPE [GRC] Tab CPU Usage Observer: Traditional Observer
Implementation

This is an alternative implementation of the Tab CPU Usage Observer
(https://codereview.chromium.org/2925123002) based on an alternative
implementation of the CoordinationUnitGraphObserver framework
(https://codereview.chromium.org/2942403002).

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306
==========

to

==========
NOCOMMIT PROTOTYPE [GRC] Tab CPU Usage Observer: Traditional Observer
Implementation

This is an alternative implementation of the Tab CPU Usage Observer
(https://codereview.chromium.org/2925123002) based on an alternative
implementation of the CoordinationUnitGraphObserver framework
(https://codereview.chromium.org/2942403002).

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306
==========

[lpy, 2017-06-19 04:34:53]

lpy@chromium.org changed reviewers:
- lpy@chromium.org

[matthalp, 2017-06-21 01:52:23]

Description was changed from

==========
NOCOMMIT PROTOTYPE [GRC] Tab CPU Usage Observer: Traditional Observer
Implementation

This is an alternative implementation of the Tab CPU Usage Observer
(https://codereview.chromium.org/2925123002) based on an alternative
implementation of the CoordinationUnitGraphObserver framework
(https://codereview.chromium.org/2942403002).

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306
==========

to

==========
[GRC] Tab CPU Usage Observer

This is a CoordinationUnitGraphObserver implementation that performs tab-level
CPU usage attribution.

Each process is given a namespace via a ProcessObserver class which
itself holds onto a collection of TabObserver class instances that
correspond to the tabs that are contained within it.

Currently the implementation makes some naive assumptions:
- Process CPU is evenly attributable among the tabs within it.
  For example, if a process with two CPU tabs has 50% utilization then
  each tab is attributed with 25%.
- Any parentless-frame is assumed to correspond to a tab. In reality
  a tab (e.g. WebContents) can have have multiple parentless-frames
  because multiple navigations can exist within a single instance.

R=*oysteine@chromium.org,*zhenw@chromium.org,lpy@google.com,fmeawad@chromium.org
BUG=724306
==========

[matthalp, 2017-06-21 01:53:14]

Description was changed from

==========
[GRC] Tab CPU Usage Observer

This is a CoordinationUnitGraphObserver implementation that performs tab-level
CPU usage attribution.

Each process is given a namespace via a ProcessObserver class which
itself holds onto a collection of TabObserver class instances that
correspond to the tabs that are contained within it.

Currently the implementation makes some naive assumptions:
- Process CPU is evenly attributable among the tabs within it.
  For example, if a process with two CPU tabs has 50% utilization then
  each tab is attributed with 25%.
- Any parentless-frame is assumed to correspond to a tab. In reality
  a tab (e.g. WebContents) can have have multiple parentless-frames
  because multiple navigations can exist within a single instance.

R=*oysteine@chromium.org,*zhenw@chromium.org,lpy@google.com,fmeawad@chromium.org
BUG=724306
==========

to

==========
[GRC] Tab CPU Usage Observer

This is a CoordinationUnitGraphObserver implementation that performs tab-level
CPU usage attribution.

Each process is given a namespace via a ProcessObserver class which
itself holds onto a collection of TabObserver class instances that
correspond to the tabs that are contained within it.

Currently the implementation makes some naive assumptions:
- Process CPU is evenly attributable among the tabs within it.
  For example, if a process with two CPU tabs has 50% utilization then
  each tab is attributed with 25%.
- Any parentless-frame is assumed to correspond to a tab. In reality
  a tab (e.g. WebContents) can have have multiple parentless-frames
  because multiple navigations can exist within a single instance.

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306
==========

[lpy, 2017-06-23 21:20:58]

lpy@chromium.org changed reviewers:
+ lpy@chromium.org

[lpy, 2017-06-23 21:20:59]

https://codereview.chromium.org/2946683002/diff/20001/services/resource_coord...
File services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.h
(right):

https://codereview.chromium.org/2946683002/diff/20001/services/resource_coord...
services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.h:1: //
Copyright 2017 The Chromium Authors. All rights reserved.
How does the observer communicate with tab manager or outside in general?

[lpy, 2017-06-23 21:21:08]

Description was changed from

==========
[GRC] Tab CPU Usage Observer

This is a CoordinationUnitGraphObserver implementation that performs tab-level
CPU usage attribution.

Each process is given a namespace via a ProcessObserver class which
itself holds onto a collection of TabObserver class instances that
correspond to the tabs that are contained within it.

Currently the implementation makes some naive assumptions:
- Process CPU is evenly attributable among the tabs within it.
  For example, if a process with two CPU tabs has 50% utilization then
  each tab is attributed with 25%.
- Any parentless-frame is assumed to correspond to a tab. In reality
  a tab (e.g. WebContents) can have have multiple parentless-frames
  because multiple navigations can exist within a single instance.

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306
==========

to

==========
[GRC] Tab CPU Usage Observer

This is a CoordinationUnitGraphObserver implementation that performs tab-level
CPU usage attribution.

Each process is given a namespace via a ProcessObserver class which
itself holds onto a collection of TabObserver class instances that
correspond to the tabs that are contained within it.

Currently the implementation makes some naive assumptions:
- Process CPU is evenly attributable among the tabs within it.
  For example, if a process with two CPU tabs has 50% utilization then
  each tab is attributed with 25%.
- Any parentless-frame is assumed to correspond to a tab. In reality
  a tab (e.g. WebContents) can have have multiple parentless-frames
  because multiple navigations can exist within a single instance.

R=oysteine@chromium.org,zhenw@chromium.org
BUG=724306
==========

[lpy, 2017-06-27 06:46:35]

My comments are mainly in cpu usage observer, overall l-g to me.

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
File services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.cc
(right):

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.cc:92: //
to be attributable for the Process's measured CPU usage.
This comment is confusing to me

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.cc:94: if
(child->id().type == CoordinationUnitType::kFrame) {
Why not just search CU that is a TabCU? Is it because of OOIF?

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.cc:143:
DCHECK_EQ(0u, tab_observers_.count(tab_cu_id));
No need to put a DCHECK here.

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.cc:207:
tab_coordination_unit->id().type == CoordinationUnitType::kFrame) {
CoordinationUnitType::kWebContents?

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.cc:208:
DCHECK_EQ(1u, process_observers_.count(process_cu_id));
no need to dcheck.

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.cc:231:
DCHECK_EQ(1u, process_observers_.count(process_coordination_unit->id()));
no need to dcheck.

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.cc:237:
DCHECK_EQ(1u, tab_observers_.count(tab_coordination_unit->id()));
ditto

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
File services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.h
(right):

https://codereview.chromium.org/2946683002/diff/40001/services/resource_coord...
services/resource_coordinator/coordination_unit/tab_cpu_usage_observer.h:25:
class TabObserver {
If it's not used outside then we don't need to expose it.

[Zhen Wang, 2017-06-30 16:38:59]

Should this be closed as we are using some other approach?

[matthalp, 2017-06-30 16:39:32]

On 2017/06/30 at 16:38:59, zhenw wrote:
> Should this be closed as we are using some other approach?

Yes -- closing.