Emit UMAs for metrics from memory instrumentation service.

Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation interface. For now, both classes exist and are used. In
the future, the former will be removed.

This CL separates out a memory_instrumentation capability, and then provides
access to that capability from the previous services with access [content_renderer,
content_gpu, etc.] as well as content_browser. This is needed to allow the
content_browser service to access the memory instrumentation interface.

BUG=703184
Review-Url: https://codereview.chromium.org/2876693002
Cr-Commit-Position: refs/heads/master@{#471435}
Committed: https://chromium.googlesource.com/chromium/src/+/a6e5f6832a2b96892a2562b032aefcd6acf56f56

[erikchen, 2017-05-11 17:44:10]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 17:44:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[erikchen, 2017-05-11 17:46:24]

Description was changed from

==========
revert


add memory umas.

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

to

==========
Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation service. For now, both classes exist and are used. In the
future, the former will be removed.

BUG=703184
==========

[erikchen, 2017-05-11 17:47:21]

Description was changed from

==========
Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation service. For now, both classes exist and are used. In the
future, the former will be removed.

BUG=703184
==========

to

==========
Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation service. For now, both classes exist and are used. In the
future, the former will be removed.

BUG=703184
==========

[erikchen, 2017-05-11 17:49:07]

erikchen@chromium.org changed reviewers:
+ primiano@chromium.org, rkaplow@chromium.org, rockot@chromium.org

[erikchen, 2017-05-11 17:49:07]

primiano: Please review chrome/browser/DEPS and 	chrome/browser/BUILD.gn.
rkaplow: Please review *metrics*
rockot: Please review *mojo*

[Primiano Tucci (use gerrit), 2017-05-11 18:10:52]

had a general pass, LGTM

https://codereview.chromium.org/2876693002/diff/60001/chrome/browser/DEPS
File chrome/browser/DEPS (right):

https://codereview.chromium.org/2876693002/diff/60001/chrome/browser/DEPS#new...
chrome/browser/DEPS:55: "+services/service_manager",
Just checking: do we have some dependency that will generate the mojom file?
Just wondering if this is flakily dpeending on some other dependency to trigger
the resource_coordinator's mojo generation

[Ken Rockot(use gerrit already), 2017-05-11 18:24:59]

*mojo* LGTM

https://codereview.chromium.org/2876693002/diff/60001/chrome/browser/DEPS
File chrome/browser/DEPS (right):

https://codereview.chromium.org/2876693002/diff/60001/chrome/browser/DEPS#new...
chrome/browser/DEPS:55: "+services/service_manager",
On 2017/05/11 at 18:10:52, Primiano Tucci wrote:
> Just checking: do we have some dependency that will generate the mojom file?
Just wondering if this is flakily dpeending on some other dependency to trigger
the resource_coordinator's mojo generation

The interfaces target is a public dependency of resource_coordinator_cpp (and is
in fact only visible to that target anyway), so yes.

As a background task I'm working on making mojom targets emit component
libraries. That should obviate the need for this kind of weirdness moving
forward.

[erikchen, 2017-05-11 18:27:45]

Description was changed from

==========
Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation service. For now, both classes exist and are used. In the
future, the former will be removed.

BUG=703184
==========

to

==========
Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation interface. For now, both classes exist and are used. In
the future, the former will be removed.

This CL separates out a memory_instrumentation capability, and then provides
access to that capability from the previous services with access
[content_renderer,
content_gpu, etc.] as well as content_browser. This is needed to allow the
content_browser service to access the memory instrumentation interface.

BUG=703184
==========

[erikchen, 2017-05-11 20:01:26]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 20:02:56]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rkaplow, 2017-05-11 21:21:40]

https://codereview.chromium.org/2876693002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2876693002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:29532: +<histogram base="true"
name="Memory.Experimental.Browser2" units="MB">
why are these all "2" btw - is this just to match with current UMA or something
else?

https://codereview.chromium.org/2876693002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:29538: +    service..
extra . at end of summaries

https://codereview.chromium.org/2876693002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:83817: +  <suffix base="true"
name="Resident" label="Only counting resident memory."/>
I would remove the base="true" tags from all your suffixes - since
Memory.Experimental.Browser2.Resident is a true histogram and not just a base of
other histograms

There's not much documentation about this yet - see the CL description for
explaination:
https://codereview.chromium.org/2426473006

[erikchen, 2017-05-11 21:32:36]

rkaplow: PTAL

https://codereview.chromium.org/2876693002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2876693002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:29532: +<histogram base="true"
name="Memory.Experimental.Browser2" units="MB">
On 2017/05/11 21:21:39, rkaplow wrote:
> why are these all "2" btw - is this just to match with current UMA or
something
> else?

memory.experimental.renderer was already taken. [Note that there are a bunch of
experimental UMA metrics that have been added over the last 6 months].

https://codereview.chromium.org/2876693002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:29538: +    service..
On 2017/05/11 21:21:40, rkaplow wrote:
> extra . at end of summaries

Done.

https://codereview.chromium.org/2876693002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:83817: +  <suffix base="true"
name="Resident" label="Only counting resident memory."/>
On 2017/05/11 21:21:39, rkaplow wrote:
> I would remove the base="true" tags from all your suffixes - since
> Memory.Experimental.Browser2.Resident is a true histogram and not just a base
of
> other histograms
> 
> There's not much documentation about this yet - see the CL description for
> explaination:
> https://codereview.chromium.org/2426473006

Done.

[rkaplow1, 2017-05-11 21:33:37]

rkaplow@google.com changed reviewers:
+ rkaplow@google.com

[rkaplow1, 2017-05-11 21:33:38]

lgtm

[erikchen, 2017-05-11 21:39:41]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 21:40:49]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ssid, 2017-05-11 21:57:01]

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

[ssid, 2017-05-11 21:57:02]

https://codereview.chromium.org/2876693002/diff/120001/chrome/browser/metrics...
File chrome/browser/metrics/process_memory_metrics_emitter.cc (right):

https://codereview.chromium.org/2876693002/diff/120001/chrome/browser/metrics...
chrome/browser/metrics/process_memory_metrics_emitter.cc:19:
UMA_HISTOGRAM_MEMORY_LARGE_MB("Memory.Experimental.Browser2.Resident",
Curious, why Browser2 and not just Browser ?

https://codereview.chromium.org/2876693002/diff/120001/chrome/browser/metrics...
chrome/browser/metrics/process_memory_metrics_emitter.cc:68:
base::trace_event::MemoryDumpLevelOfDetail::DETAILED};
This should be LevelOfDetail::BACKGROUND.
I was sure that we would have added a dcheck somewhere not to trigger detailed
dump in summary_only mode. But, couldn't find it. We do not want to parse smaps
/ iterate vm regions for creating summary now.

[ssid, 2017-05-11 21:57:21]

Thanks for adding the test.

[rkaplow, 2017-05-11 21:58:03]

lgtm

[erikchen, 2017-05-11 22:01:38]

https://codereview.chromium.org/2876693002/diff/120001/chrome/browser/metrics...
File chrome/browser/metrics/process_memory_metrics_emitter.cc (right):

https://codereview.chromium.org/2876693002/diff/120001/chrome/browser/metrics...
chrome/browser/metrics/process_memory_metrics_emitter.cc:19:
UMA_HISTOGRAM_MEMORY_LARGE_MB("Memory.Experimental.Browser2.Resident",
On 2017/05/11 21:57:02, ssid wrote:
> Curious, why Browser2 and not just Browser ?

to match renderer2, and renderer already exists as an experimental metric.

https://codereview.chromium.org/2876693002/diff/120001/chrome/browser/metrics...
chrome/browser/metrics/process_memory_metrics_emitter.cc:68:
base::trace_event::MemoryDumpLevelOfDetail::DETAILED};
On 2017/05/11 21:57:02, ssid wrote:
> This should be LevelOfDetail::BACKGROUND.
> I was sure that we would have added a dcheck somewhere not to trigger detailed
> dump in summary_only mode. But, couldn't find it. We do not want to parse
smaps
> / iterate vm regions for creating summary now.

Done.

[erikchen, 2017-05-11 22:01:43]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[erikchen, 2017-05-11 22:03:10]

erikchen@chromium.org changed reviewers:
+ dcheng@chromium.org, thakis@chromium.org

[erikchen, 2017-05-11 22:03:11]

thakis: Please stamp chrome/browser/DEPS.
dcheng: SECURITY_OWNER review for content/public/app/mojo. Note that rockot@ has
already reviewed, and this change was discussed at
https://groups.google.com/a/chromium.org/forum/#!topic/services-dev/FRMxW4njLl8.

[commit-bot: I haz the power, 2017-05-11 22:04:17]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nico, 2017-05-11 22:08:16]

chrome/browser deps lgtm

I'm surprised you found something that chrome/ didn't already depend on!

[dcheng, 2017-05-11 22:12:27]

security lgtm

https://codereview.chromium.org/2876693002/diff/140001/chrome/browser/metrics...
File chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
(right):

https://codereview.chromium.org/2876693002/diff/140001/chrome/browser/metrics...
chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc:21:
uint64_t dump_guid,
FWIW, I still think the typical expectation for "guid" in Chrome is 128-bits of
randomness, so naming a uint64_t guid is somewhat deceiving. *shrug*

[erikchen, 2017-05-11 22:18:47]

https://codereview.chromium.org/2876693002/diff/140001/chrome/browser/metrics...
File chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
(right):

https://codereview.chromium.org/2876693002/diff/140001/chrome/browser/metrics...
chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc:21:
uint64_t dump_guid,
On 2017/05/11 22:12:27, dcheng wrote:
> FWIW, I still think the typical expectation for "guid" in Chrome is 128-bits
of
> randomness, so naming a uint64_t guid is somewhat deceiving. *shrug*

@primiano:
thakis and I had a discussion about birthday paradox and Chrome. In general,
64-bits is not enough to guarantee a low probability of collision, over all
users, over several years. [Obviously depends on how frequently the "guid" is
generated].

[Nico, 2017-05-11 22:24:39]

https://codereview.chromium.org/2876693002/diff/140001/chrome/browser/metrics...
File chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
(right):

https://codereview.chromium.org/2876693002/diff/140001/chrome/browser/metrics...
chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc:21:
uint64_t dump_guid,
On 2017/05/11 22:18:47, erikchen wrote:
> On 2017/05/11 22:12:27, dcheng wrote:
> > FWIW, I still think the typical expectation for "guid" in Chrome is 128-bits
> of
> > randomness, so naming a uint64_t guid is somewhat deceiving. *shrug*
> 
> @primiano:
> thakis and I had a discussion about birthday paradox and Chrome. In general,
> 64-bits is not enough to guarantee a low probability of collision, over all

IIRC the numbers were like "there's a 5% chance that any user has any collision
over 3 years" with 64-bit, with the failure case being "nothing bad happens". To
me that sounded like "64 bits should be plenty" and to Erik it sounded like
"64-bits are not enough" :-)

[dcheng, 2017-05-11 22:26:40]

On 2017/05/11 22:24:39, Nico wrote:
>
https://codereview.chromium.org/2876693002/diff/140001/chrome/browser/metrics...
> File chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
> (right):
> 
>
https://codereview.chromium.org/2876693002/diff/140001/chrome/browser/metrics...
> chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc:21:
> uint64_t dump_guid,
> On 2017/05/11 22:18:47, erikchen wrote:
> > On 2017/05/11 22:12:27, dcheng wrote:
> > > FWIW, I still think the typical expectation for "guid" in Chrome is
128-bits
> > of
> > > randomness, so naming a uint64_t guid is somewhat deceiving. *shrug*
> > 
> > @primiano:
> > thakis and I had a discussion about birthday paradox and Chrome. In general,
> > 64-bits is not enough to guarantee a low probability of collision, over all
> 
> IIRC the numbers were like "there's a 5% chance that any user has any
collision
> over 3 years" with 64-bit, with the failure case being "nothing bad happens".
To
> me that sounded like "64 bits should be plenty" and to Erik it sounded like
> "64-bits are not enough" :-)

FWIW, an informal survey of people ("if you see a variable named GUID, does that
imply anything to you?") suggests that most people would expect it to be related
to GenerateGUID() from base or 128-bits, etc. I don't really care about the size
of the variable, just the surprising nature of the name.

[erikchen, 2017-05-11 22:28:24]

> 
> FWIW, an informal survey of people ("if you see a variable named GUID, does
that
> imply anything to you?") suggests that most people would expect it to be
related
> to GenerateGUID() from base or 128-bits, etc. I don't really care about the
size
> of the variable, just the surprising nature of the name.

not_so_strong_guid?
at_most_one_collision_every_three_years_guid?
probably_unique_id?

[Ken Rockot(use gerrit already), 2017-05-11 22:28:58]

I generally agree that "guid" seems like a misnomer given the size as it
almost universally implies 128 bits. How about just "id"?

On Thu, May 11, 2017 at 3:26 PM, <dcheng@chromium.org> wrote:

> On 2017/05/11 22:24:39, Nico wrote:
> >
> https://codereview.chromium.org/2876693002/diff/140001/
> chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
> > File chrome/browser/metrics/process_memory_metrics_
> emitter_browsertest.cc
> > (right):
> >
> >
> https://codereview.chromium.org/2876693002/diff/140001/
> chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc#
> newcode21
> > chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc:21:
> > uint64_t dump_guid,
> > On 2017/05/11 22:18:47, erikchen wrote:
> > > On 2017/05/11 22:12:27, dcheng wrote:
> > > > FWIW, I still think the typical expectation for "guid" in Chrome is
> 128-bits
> > > of
> > > > randomness, so naming a uint64_t guid is somewhat deceiving. *shrug*
> > >
> > > @primiano:
> > > thakis and I had a discussion about birthday paradox and Chrome. In
> general,
> > > 64-bits is not enough to guarantee a low probability of collision,
> over all
> >
> > IIRC the numbers were like "there's a 5% chance that any user has any
> collision
> > over 3 years" with 64-bit, with the failure case being "nothing bad
> happens".
> To
> > me that sounded like "64 bits should be plenty" and to Erik it sounded
> like
> > "64-bits are not enough" :-)
>
> FWIW, an informal survey of people ("if you see a variable named GUID,
> does that
> imply anything to you?") suggests that most people would expect it to be
> related
> to GenerateGUID() from base or 128-bits, etc. I don't really care about
> the size
> of the variable, just the surprising nature of the name.
>
> https://codereview.chromium.org/2876693002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[erikchen, 2017-05-11 22:33:23]

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=721558

[commit-bot: I haz the power, 2017-05-11 23:59:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-11 23:59:52]

Dry run: This issue passed the CQ dry run.

[Primiano Tucci (use gerrit), 2017-05-12 01:14:37]

On 2017/05/11 22:33:23, erikchen wrote:
> Filed https://bugs.chromium.org/p/chromium/issues/detail?id=721558

Maybe this is just a naming thing. This is not supposed to be global across all
users. Just global across all processes within the scope of a tracing session.
Given that it is a monotonic counter, even if we increment this every nanosecond
should be enough for some hundred years. :) 
Unfortunately I think this is just confusion due to the legacy api and the name
doesn't make it clear that we really care about this only for tracing. 
The use case here is : when we request a dump via the devtools api, the guid is
returned. That guid will match the ID of the dump in the trace json, so
telemetry will know exactly which dump is which.

[ssid, 2017-05-12 01:50:57]

On 2017/05/12 01:14:37, Primiano Tucci wrote:
> On 2017/05/11 22:33:23, erikchen wrote:
> > Filed https://bugs.chromium.org/p/chromium/issues/detail?id=721558
> 
> Maybe this is just a naming thing. This is not supposed to be global across
all
> users. Just global across all processes within the scope of a tracing session.
> Given that it is a monotonic counter, even if we increment this every
nanosecond
> should be enough for some hundred years. :) 
> Unfortunately I think this is just confusion due to the legacy api and the
name
> doesn't make it clear that we really care about this only for tracing. 
> The use case here is : when we request a dump via the devtools api, the guid
is
> returned. That guid will match the ID of the dump in the trace json, so
> telemetry will know exactly which dump is which.

Maybe we should rename it to session_unique_id or something. I think in reality
we generate exact same guid if we request a memory dump in 2 chrome session in
the same way. But yes this would not affect anything because the id is unique in
a browser session and only that matters for telemetry.
Although renaming this in this cl would be a unrelated big clean up. All this cl
does is define a callback as specified by
base::trace_event::GlobalMemoryDumpCallback.

[Primiano Tucci (use gerrit), 2017-05-12 06:52:29]

The CQ bit was checked by primiano@chromium.org

[Primiano Tucci (use gerrit), 2017-05-12 06:52:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
primiano@chromium.org, rockot@chromium.org, rkaplow@chromium.org,
rkaplow@google.com
Link to the patchset: https://codereview.chromium.org/2876693002/#ps140001
(title: "comments from ssid.")

[commit-bot: I haz the power, 2017-05-12 06:52:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 06:52:51]

This CL has an open dependency (Issue 2871223002 Patch 60001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Primiano Tucci (use gerrit), 2017-05-12 06:53:33]

On 2017/05/12 01:50:57, ssid wrote:
> On 2017/05/12 01:14:37, Primiano Tucci wrote:
> > On 2017/05/11 22:33:23, erikchen wrote:
> > > Filed https://bugs.chromium.org/p/chromium/issues/detail?id=721558
> > 
> > Maybe this is just a naming thing. This is not supposed to be global across
> all
> > users. Just global across all processes within the scope of a tracing
session.
> > Given that it is a monotonic counter, even if we increment this every
> nanosecond
> > should be enough for some hundred years. :) 
> > Unfortunately I think this is just confusion due to the legacy api and the
> name
> > doesn't make it clear that we really care about this only for tracing. 
> > The use case here is : when we request a dump via the devtools api, the guid
> is
> > returned. That guid will match the ID of the dump in the trace json, so
> > telemetry will know exactly which dump is which.
> 
> Maybe we should rename it to session_unique_id or something. I think in
reality
> we generate exact same guid if we request a memory dump in 2 chrome session in
> the same way. But yes this would not affect anything because the id is unique
in
> a browser session and only that matters for telemetry.
> Although renaming this in this cl would be a unrelated big clean up. All this
cl
> does is define a callback as specified by
> base::trace_event::GlobalMemoryDumpCallback.

Renaming sgtm, although can't do it right now as it's called dump_guid
everywhere

[erikchen, 2017-05-12 17:30:09]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[erikchen, 2017-05-12 17:30:41]

The CQ bit was unchecked by erikchen@chromium.org

[erikchen, 2017-05-12 19:58:07]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2017-05-12 19:58:42]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[erikchen, 2017-05-12 20:00:37]

The CQ bit was unchecked by erikchen@chromium.org

[erikchen, 2017-05-12 20:31:04]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2017-05-12 20:31:32]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 20:39:28]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1494621064131060,
"parent_rev": "ac0ea495b772c2b91ff94427b5a1cb6646cc4daf", "commit_rev":
"a6e5f6832a2b96892a2562b032aefcd6acf56f56"}

[commit-bot: I haz the power, 2017-05-12 20:39:39]

Description was changed from

==========
Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation interface. For now, both classes exist and are used. In
the future, the former will be removed.

This CL separates out a memory_instrumentation capability, and then provides
access to that capability from the previous services with access
[content_renderer,
content_gpu, etc.] as well as content_browser. This is needed to allow the
content_browser service to access the memory instrumentation interface.

BUG=703184
==========

to

==========
Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation interface. For now, both classes exist and are used. In
the future, the former will be removed.

This CL separates out a memory_instrumentation capability, and then provides
access to that capability from the previous services with access
[content_renderer,
content_gpu, etc.] as well as content_browser. This is needed to allow the
content_browser service to access the memory instrumentation interface.

BUG=703184

Review-Url: https://codereview.chromium.org/2876693002
Cr-Commit-Position: refs/heads/master@{#471435}
Committed:
https://chromium.googlesource.com/chromium/src/+/a6e5f6832a2b96892a2562b032ae...
==========

[commit-bot: I haz the power, 2017-05-12 20:39:40]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/a6e5f6832a2b96892a2562b032ae...

[erikchen, 2017-05-17 18:54:39]

Description was changed from

==========
Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation interface. For now, both classes exist and are used. In
the future, the former will be removed.

This CL separates out a memory_instrumentation capability, and then provides
access to that capability from the previous services with access
[content_renderer,
content_gpu, etc.] as well as content_browser. This is needed to allow the
content_browser service to access the memory instrumentation interface.

BUG=703184

Review-Url: https://codereview.chromium.org/2876693002
Cr-Commit-Position: refs/heads/master@{#471435}
Committed:
https://chromium.googlesource.com/chromium/src/+/a6e5f6832a2b96892a2562b032ae...
==========

to

==========
Emit UMAs for metrics from memory instrumentation service.

ChromeMetricsServiceClient uses MetricsMemoryDetails to fetch and emit
memory-related UMA metrics by manually computing the metrics. This CL adds a
similar class ProcessMemoryMetricsEmitter to fetch and emit metrics from the
memory instrumentation interface. For now, both classes exist and are used. In
the future, the former will be removed.

This CL separates out a memory_instrumentation capability, and then provides
access to that capability from the previous services with access
[content_renderer,
content_gpu, etc.] as well as content_browser. This is needed to allow the
content_browser service to access the memory instrumentation interface.

BUG=703184

Review-Url: https://codereview.chromium.org/2876693002
Cr-Commit-Position: refs/heads/master@{#471435}
Committed:
https://chromium.googlesource.com/chromium/src/+/a6e5f6832a2b96892a2562b032ae...
==========

[erikchen, 2017-05-17 18:56:26]

Patchset #9 (id:160001) has been deleted