Add peak memory usage metric

Add peak memory usage metric

PeakMemoryUsageTracker is used to track the max Memory.Total2 so far.
When the peak increases we send the peak memory usage relative to the amount of physical memory.
By sending how high the peak memory usage is relative to available memory, I hope to find out how often users encounter OOM situations.

BUG=

[keishi, 2016-12-12 09:29:17]

Description was changed from

==========
Add peak memory usage metric

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

to

==========
Add peak memory usage metric

PeakMemoryUsageTracker is used to track the max Memory.Total2 so far.
When the peak increases we send the peak memory usage relative to the amount of
physical memory.
By sending how high the peak memory usage is relative to available memory, I
hope to find out how often users encounter OOM situations.

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

[keishi, 2016-12-12 09:31:01]

keishi@chromium.org changed reviewers:
+ haraken@chromium.org, sky@chromium.org, tasak@google.com

[keishi, 2016-12-12 09:31:02]

I am writing a document on this but I think the biggest problems with the
current memory usage UMAs are:
- Shared memory size is not tracked
- Peak memory usage is under represented
- No data about per-allocator memory usage

primiano@ is working to separate memory-infra from tracing and that will enable
us to have very good UMAs to address these issues.

But in the meantime I would like to start by adding these two metrics.
- Renderer’s per allocator allocations (using tasak’s hack) for DidFinishLoad
and ProcessShutdown. https://codereview.chromium.org/2566043004/
- Peak memory usage relative to physical memory This CL

[haraken, 2016-12-12 11:40:59]

haraken@chromium.org changed reviewers:
+ primiano@chromium.org

[haraken, 2016-12-12 11:41:00]

This is an interesting metric. LGTM to try.

It will take months to figure out how to collect consistent UMAs from the
renderer. So, +1 to starting experiment like this in parallel with the work of
implementing a right infrastructure to collect memory-infra-based UMAs.

https://codereview.chromium.org/2566083002/diff/1/chrome/browser/metrics/metr...
File chrome/browser/metrics/metrics_memory_details.h (right):

https://codereview.chromium.org/2566083002/diff/1/chrome/browser/metrics/metr...
chrome/browser/metrics/metrics_memory_details.h:47: bool Update(int
memory_usage);

unsigned long long?

https://codereview.chromium.org/2566083002/diff/1/chrome/browser/metrics/metr...
chrome/browser/metrics/metrics_memory_details.h:51: int peak_;

unsigned long long?

[Primiano Tucci (use gerrit), 2016-12-12 11:58:44]

My concern here is that we are now introducing a concept of "Peak" in UMA that
will not match the peak that we will have in other memory metrics thanks to the
work that ssid is doing in [1] and will make the current UMA even harder to
understand.
I think that the major problem of UMA is the lack of defined metrics that don't
match what we do in the other scenarios.
And this CL seems to add more entropy to the system, introducing a notion of
"peak memory usage, where, however, peak is not a real peak but only the peak of
the metric sampler"

[1]
https://docs.google.com/document/d/1-6Y6jsH6qCRIs3b6PNgHJk3SayYoibko0ysb9XpAG...

[haraken, 2016-12-12 12:26:28]

On 2016/12/12 11:58:44, Primiano Tucci wrote:
> My concern here is that we are now introducing a concept of "Peak" in UMA that
> will not match the peak that we will have in other memory metrics thanks to
the
> work that ssid is doing in [1] and will make the current UMA even harder to
> understand.

Hmm, how can we make the two more consistent?

> I think that the major problem of UMA is the lack of defined metrics that
don't
> match what we do in the other scenarios.
> And this CL seems to add more entropy to the system, introducing a notion of
> "peak memory usage, where, however, peak is not a real peak but only the peak
of
> the metric sampler"
> 
> [1]
>
https://docs.google.com/document/d/1-6Y6jsH6qCRIs3b6PNgHJk3SayYoibko0ysb9XpAG...

I was thinking that the peak detection is for background tracing, which won't be
able to be enabled on production builds by default. So I thought we'll need
another (less accurate) way to measure the peak memory at the per-15-second
interval (where UMA reporting is triggered).

[Primiano Tucci (use gerrit), 2016-12-12 13:13:04]

On 2016/12/12 12:26:28, haraken wrote:
> On 2016/12/12 11:58:44, Primiano Tucci wrote:
> > My concern here is that we are now introducing a concept of "Peak" in UMA
that
> > will not match the peak that we will have in other memory metrics thanks to
> the
> > work that ssid is doing in [1] and will make the current UMA even harder to
> > understand.
> 
> Hmm, how can we make the two more consistent?

So here's my concern in practice:
As long as you introduce UMA of the form "Memoy.MemoryCoordinator.X" it is clear
that this is something specific to MC and only you folks will use it.
But if you introduce Memory.Peak, now everybody will start monitoring that as a
general metric. People will use to draw their own conclusions in their
experiments and most of the time the data presented by this UMA will be
misleading (see below).
In other words, I think that if we add general-purpose UMA like this we have
have to be confident we are delivering a quality signal and not just noise.
People are then going to spend lot of time on these signals and bike-shed on
them. And we don't want to end these discussion with the usual "ah no this is
just because of the way the UMA is sampled".

With reference to the current CL, I have lot of questions, on top of the ones
inherited by the weakness of Memory.Total* itself. Specifically:
- how frequently is the sampling? Is this going to bias only towards
long-lasting allocations?
- who resets this peak? from what I read here this is doomed to max-out for ever
with no windowing. Which means that this is going to be always dominated by
things like rastering during flinging or large JS heaps due to the content. This
is even worse on Svelte where, IIRC, we recycle the renderer only for
browser-initiated navigations (e.g., typing a new address in the omnibox) and
not by renderer-initiated navigations (clicking on links).
- Why the peak is relative to physical memory and not absolute? This seems a
somewhat arbitrary decision.
- The CL title says that the driving motivation is OOM. Do we have any evidence
that % peak correlated with OOM? On android OOMs happen way sooner than getting
to the physical memory limit, because the lowmemkiller has more heuristics
(background vs foreground, LRU, type of binding)


> > I think that the major problem of UMA is the lack of defined metrics that
> don't
> > match what we do in the other scenarios.
> > And this CL seems to add more entropy to the system, introducing a notion of
> > "peak memory usage, where, however, peak is not a real peak but only the
peak
> of
> > the metric sampler"
> > 
> > [1]
> >
>
https://docs.google.com/document/d/1-6Y6jsH6qCRIs3b6PNgHJk3SayYoibko0ysb9XpAG...
> 
> I was thinking that the peak detection is for background tracing, which won't
be
> able to be enabled on production builds by default. So I thought we'll need
> another (less accurate) way to measure the peak memory at the per-15-second
> interval (where UMA reporting is triggered).

No, one of the points of peak detection is that we can also use it to trigger
UMA once we have memory-infra uma.

[haraken, 2016-12-12 15:57:07]

On 2016/12/12 13:13:04, Primiano Tucci wrote:
> On 2016/12/12 12:26:28, haraken wrote:
> > On 2016/12/12 11:58:44, Primiano Tucci wrote:
> > > My concern here is that we are now introducing a concept of "Peak" in UMA
> that
> > > will not match the peak that we will have in other memory metrics thanks
to
> > the
> > > work that ssid is doing in [1] and will make the current UMA even harder
to
> > > understand.
> > 
> > Hmm, how can we make the two more consistent?
> 
> So here's my concern in practice:
> As long as you introduce UMA of the form "Memoy.MemoryCoordinator.X" it is
clear
> that this is something specific to MC and only you folks will use it.
> But if you introduce Memory.Peak, now everybody will start monitoring that as
a
> general metric. People will use to draw their own conclusions in their
> experiments and most of the time the data presented by this UMA will be
> misleading (see below).
> In other words, I think that if we add general-purpose UMA like this we have
> have to be confident we are delivering a quality signal and not just noise.
> People are then going to spend lot of time on these signals and bike-shed on
> them. And we don't want to end these discussion with the usual "ah no this is
> just because of the way the UMA is sampled".

Yeah, this makes a lot of sense.

The point is that we'll need a lot of experiments to get more understanding of
the renderer and choose UMAs which should be promoted to Memory.XXX. I agree
that we should avoid using Memory.XXX until we are confident that the UMA is
something we want to keep tracking in long term. Maybe we can use
Memory.Experiments.XXX.

keishi@: Would you try to publish your survey document before landing these
UMAs? I think the document will clarify the motivation.


> 
> With reference to the current CL, I have lot of questions, on top of the ones
> inherited by the weakness of Memory.Total* itself. Specifically:
> - how frequently is the sampling? Is this going to bias only towards
> long-lasting allocations?
> - who resets this peak? from what I read here this is doomed to max-out for
ever
> with no windowing. Which means that this is going to be always dominated by
> things like rastering during flinging or large JS heaps due to the content.
This
> is even worse on Svelte where, IIRC, we recycle the renderer only for
> browser-initiated navigations (e.g., typing a new address in the omnibox) and
> not by renderer-initiated navigations (clicking on links).
> - Why the peak is relative to physical memory and not absolute? This seems a
> somewhat arbitrary decision.
> - The CL title says that the driving motivation is OOM. Do we have any
evidence
> that % peak correlated with OOM? On android OOMs happen way sooner than
getting
> to the physical memory limit, because the lowmemkiller has more heuristics
> (background vs foreground, LRU, type of binding)
> 
> 
> > > I think that the major problem of UMA is the lack of defined metrics that
> > don't
> > > match what we do in the other scenarios.
> > > And this CL seems to add more entropy to the system, introducing a notion
of
> > > "peak memory usage, where, however, peak is not a real peak but only the
> peak
> > of
> > > the metric sampler"
> > > 
> > > [1]
> > >
> >
>
https://docs.google.com/document/d/1-6Y6jsH6qCRIs3b6PNgHJk3SayYoibko0ysb9XpAG...
> > 
> > I was thinking that the peak detection is for background tracing, which
won't
> be
> > able to be enabled on production builds by default. So I thought we'll need
> > another (less accurate) way to measure the peak memory at the per-15-second
> > interval (where UMA reporting is triggered).
> 
> No, one of the points of peak detection is that we can also use it to trigger
> UMA once we have memory-infra uma.