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

Issue 781782 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Compute and report shared memory footprint to UMA/UKM

Project Member Reported by lalitm@chromium.org, Nov 6 2017

Issue description

Background context: go/memory-infra

It has long been a goal to expose shared memory footprint to UMA/UKM. With the recent work on porting the memory graph calculation code to Chrome, this metric is now able to be computed.

Solving this issue requires a few steps:
1. Add graph processing code up to weak node removal (done in bug crbug/768373).
2. Compute shared memory footprint per-process using algorithm to apportion blame to processes based on importance.
3. Add proto fields to UKM/UMA for performing this calculation.
4. Set proto fields to computed processes.
5. Ensure that the shared footprint makes sense in the field.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 7 2017

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

commit 6b20dfea56f4ceb5894a678b457ad075879a79ac
Author: Lalit Maganti <lalitm@chromium.org>
Date: Tue Nov 07 15:49:21 2017

memory-infra: compute shared memory footprint per process

This change adds a method to compute the shared memory footprint for each process.
This works by finding all the global nodes which are owned by shared memory nodes
in the graph and then attempting to figure out which ones should be apportioned
some of the size of the global node.

The code to decide sizes works by first considering the priority of each edge in
the graph which points to the global dump and only selecting the ones with the
highest importance. The size is then split between all the shared memory nodes
with the same importance and thus the process associated with each of these nodes
is incremented by the appropriate size

Bug:  781782 
Change-Id: I9fce23cc2947658a8831244a46ffa7ca5bcb783d
Reviewed-on: https://chromium-review.googlesource.com/738050
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514477}
[modify] https://crrev.com/6b20dfea56f4ceb5894a678b457ad075879a79ac/base/memory/shared_memory_tracker.cc
[modify] https://crrev.com/6b20dfea56f4ceb5894a678b457ad075879a79ac/base/memory/shared_memory_tracker.h
[modify] https://crrev.com/6b20dfea56f4ceb5894a678b457ad075879a79ac/services/resource_coordinator/memory_instrumentation/graph.cc
[modify] https://crrev.com/6b20dfea56f4ceb5894a678b457ad075879a79ac/services/resource_coordinator/memory_instrumentation/graph.h
[modify] https://crrev.com/6b20dfea56f4ceb5894a678b457ad075879a79ac/services/resource_coordinator/memory_instrumentation/graph_processor.cc
[modify] https://crrev.com/6b20dfea56f4ceb5894a678b457ad075879a79ac/services/resource_coordinator/memory_instrumentation/graph_processor.h
[modify] https://crrev.com/6b20dfea56f4ceb5894a678b457ad075879a79ac/services/resource_coordinator/memory_instrumentation/graph_processor_unittest.cc
[modify] https://crrev.com/6b20dfea56f4ceb5894a678b457ad075879a79ac/services/resource_coordinator/memory_instrumentation/graph_unittest.cc

Project Member

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

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

commit c5fab70936acefea55759a49fb3d467ac9e70776
Author: Lalit Maganti <lalitm@chromium.org>
Date: Mon Nov 13 12:45:57 2017

memory-infra: include computation of shared memory footprint as part of infra service output

This CL achieves the long present goal to compute the shared memory footprint
of each process in Chrome. It does this by utilising earlier work to perform
the computation and adds it to the existing struct of metrics reported
by the memory-infra service.

For more context, see https://goo.gl/3kPb9S

Bug:  781782 
Change-Id: I6b9603e54ddd76e84fcb4898eb38d6be5c0c233f
Reviewed-on: https://chromium-review.googlesource.com/757139
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515933}
[modify] https://crrev.com/c5fab70936acefea55759a49fb3d467ac9e70776/chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc
[modify] https://crrev.com/c5fab70936acefea55759a49fb3d467ac9e70776/chrome/browser/profiling_host/background_profiling_triggers_unittest.cc
[modify] https://crrev.com/c5fab70936acefea55759a49fb3d467ac9e70776/services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc
[modify] https://crrev.com/c5fab70936acefea55759a49fb3d467ac9e70776/services/resource_coordinator/memory_instrumentation/graph_processor.cc
[modify] https://crrev.com/c5fab70936acefea55759a49fb3d467ac9e70776/services/resource_coordinator/memory_instrumentation/graph_processor.h
[modify] https://crrev.com/c5fab70936acefea55759a49fb3d467ac9e70776/services/resource_coordinator/memory_instrumentation/graph_processor_unittest.cc
[modify] https://crrev.com/c5fab70936acefea55759a49fb3d467ac9e70776/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.cc
[modify] https://crrev.com/c5fab70936acefea55759a49fb3d467ac9e70776/services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 15 2017

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

commit b9ba0b6267dbd27da4e3c294a43838f43ea0a9f3
Author: Lalit Maganti <lalitm@chromium.org>
Date: Wed Nov 15 13:22:50 2017

memory-infra: Add UKM and UMA constants for shared memory footprint

Now that we have the code for computing the shared memory footprint, we
should add the means to report the data to UMA/UKM.

See go/czhuu for context

Bug:  781782 
Change-Id: I38a7b3e2e7794b09ef779e728ca998efba7659be
Reviewed-on: https://chromium-review.googlesource.com/756750
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516680}
[modify] https://crrev.com/b9ba0b6267dbd27da4e3c294a43838f43ea0a9f3/chrome/browser/metrics/process_memory_metrics_emitter.cc
[modify] https://crrev.com/b9ba0b6267dbd27da4e3c294a43838f43ea0a9f3/chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc
[modify] https://crrev.com/b9ba0b6267dbd27da4e3c294a43838f43ea0a9f3/tools/metrics/histograms/histograms.xml
[modify] https://crrev.com/b9ba0b6267dbd27da4e3c294a43838f43ea0a9f3/tools/metrics/ukm/ukm.xml

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 23 2017

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

commit e15b47cd03542eba57cc447579db5e81452fcc8b
Author: Lalit Maganti <lalitm@chromium.org>
Date: Thu Nov 23 14:22:02 2017

memory-infra: fix shared memory reporting and footprint calculation

Bug:  781782 
Change-Id: I01a935b3399f1faf42b1efeaa14decbc6bd49c65
Reviewed-on: https://chromium-review.googlesource.com/785953
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518927}
[modify] https://crrev.com/e15b47cd03542eba57cc447579db5e81452fcc8b/base/trace_event/memory_infra_background_whitelist.cc
[modify] https://crrev.com/e15b47cd03542eba57cc447579db5e81452fcc8b/services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.cc

Comment 5 by lalitm@chromium.org, Nov 30 2017

Status: Verified (was: Started)
Looks like these stats are coming in and making some sense so marking as verified.

Sign in to add a comment