Add cooldown to LeakDetectorImpl leak report generation

Add cooldown to LeakDetectorImpl leak report generation

After a report is generated for a particular allocation size
and call site, there is a cooldown period before another
report can be generated for the same size and call site. This
prevents LeakDetectorImpl from generating too reports with
redundant data.

BUG=chromium:605314
TEST=unit tests pass

Committed: https://crrev.com/7a9e129ec81d69c64a23d1bf7ab83e3f5ae3b3d4
Cr-Commit-Position: refs/heads/master@{#388707}

[Simon Que, 2016-04-18 18:25:18]

Description was changed from

==========
Add cooldown to LeakDetectorImpl leak report generation

This prevents LeakDetectorImpl from generating too reports with
redundant data.


BUG=chromium:382705
TEST=unit tests pass
==========

to

==========
Add cooldown to LeakDetectorImpl leak report generation

After a report is generated for a particular allocation size
and call site, there is a cooldown period before another
report can be generated for the same size and call site. This
prevents LeakDetectorImpl from generating too reports with
redundant data.

BUG=chromium:382705
TEST=unit tests pass
==========

[Simon Que, 2016-04-18 18:25:18]

sque@chromium.org changed reviewers:
+ wfh@chromium.org

[Simon Que, 2016-04-18 18:25:18]

Description was changed from

==========
Add cooldown to LeakDetectorImpl leak report generation

This prevents LeakDetectorImpl from generating too reports with
redundant data.


BUG=chromium:382705
TEST=unit tests pass
==========

to

==========
Add cooldown to LeakDetectorImpl leak report generation

After a report is generated for a particular allocation size
and call site, there is a cooldown period before another
report can be generated for the same size and call site. This
prevents LeakDetectorImpl from generating too reports with
redundant data.

BUG=chromium:382705
TEST=unit tests pass
==========

[Simon Que, 2016-04-18 18:25:18]

sque@chromium.org changed reviewers:
+ wfh@chromium.org

[Will Harris, 2016-04-20 00:52:38]

are all these changes part of a master plan, how are you deciding that you need
a cooldown, are you getting data from the field now?

is there a master doc that describes the data you're using to know that a
cooldown is needed.

As you're an owner of leak_detector I'm happy for you to make these decisions,
and for my review to more be a double-check e.g. second pair of eyes to check no
mistakes, but it would be nice to track why this is needed somewhere.

[Simon Que, 2016-04-20 01:52:33]

On 2016/04/20 00:52:38, Will Harris wrote:
> are all these changes part of a master plan, how are you deciding that you
need
> a cooldown, are you getting data from the field now?
> 
> is there a master doc that describes the data you're using to know that a
> cooldown is needed.
> 
> As you're an owner of leak_detector I'm happy for you to make these decisions,
> and for my review to more be a double-check e.g. second pair of eyes to check
no
> mistakes, but it would be nice to track why this is needed somewhere.

Not part of an original plan. It occurred to me when implementing history
recording that if a size and call site is continuously leaking, and a leak
report is generated at each analysis / frame, there will be redundant frames in
the history.

e.g. suppose the frames are A-B-C-D-E-F-G-H-I... (each letter is a snapshot of
allocation breakdown) and we're recording the last five frames:

Leak Report #1, generated at G, will contain C-D-E-F-G
Leak Report #2, generated at H, will contain D-E-F-G-H
Leak Report #3, generated at I, will contain E-F-G-H-I

... and so forth. The same historical allocation breakdowns (D, E, F, G, etc)
will appear in multiple reports. That's a lot of redundant data being sent over
the UMA channel.

I can understand if this is less-than-obvious since it's something I came up
with on the fly, and it was not part of the original design docs. I can write a
simple doc that illustrates this so we have something to refer to in the future.

[Simon Que, 2016-04-20 22:06:25]

Here's a slideshow that explains it:
https://docs.google.com/presentation/d/15P1qTloLEZa20aNUl_-A6JqojTebwxTSrMdGk...

[Will Harris, 2016-04-20 22:16:06]

On 2016/04/20 22:06:25, Simon Que wrote:
> Here's a slideshow that explains it:
>
https://docs.google.com/presentation/d/15P1qTloLEZa20aNUl_-A6JqojTebwxTSrMdGk...

thanks! I think it would be nice to track each of these improvements in a
separate bug, then the slides and any discussion associated with it could be
tracked via that bug, you could mark it a blocker of the main bug, but it allows
a fuller justification of the issues and the fix to be located in one central
place linked from the CL.

I think these slides could then be linked from the bug.

[Simon Que, 2016-04-20 22:37:45]

Description was changed from

==========
Add cooldown to LeakDetectorImpl leak report generation

After a report is generated for a particular allocation size
and call site, there is a cooldown period before another
report can be generated for the same size and call site. This
prevents LeakDetectorImpl from generating too reports with
redundant data.

BUG=chromium:382705
TEST=unit tests pass
==========

to

==========
Add cooldown to LeakDetectorImpl leak report generation

After a report is generated for a particular allocation size
and call site, there is a cooldown period before another
report can be generated for the same size and call site. This
prevents LeakDetectorImpl from generating too reports with
redundant data.

BUG=chromium:605314
TEST=unit tests pass
==========

[Simon Que, 2016-04-20 22:37:58]

On 2016/04/20 22:16:06, Will Harris wrote:
> On 2016/04/20 22:06:25, Simon Que wrote:
> > Here's a slideshow that explains it:
> >
>
https://docs.google.com/presentation/d/15P1qTloLEZa20aNUl_-A6JqojTebwxTSrMdGk...
> 
> thanks! I think it would be nice to track each of these improvements in a
> separate bug, then the slides and any discussion associated with it could be
> tracked via that bug, you could mark it a blocker of the main bug, but it
allows
> a fuller justification of the issues and the fix to be located in one central
> place linked from the CL.
> 
> I think these slides could then be linked from the bug.

Done. I've updated this CL to point to the new bug.

[Will Harris, 2016-04-20 22:40:15]

perfect. great explanation as well

lgtm.

[Simon Que, 2016-04-20 23:16:06]

The CQ bit was checked by sque@chromium.org

[commit-bot: I haz the power, 2016-04-20 23:16:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1892283004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1892283004/60001

[commit-bot: I haz the power, 2016-04-21 00:15:57]

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

[commit-bot: I haz the power, 2016-04-21 00:15:58]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Simon Que, 2016-04-21 00:33:09]

https://codereview.chromium.org/1892283004/diff/80001/components/metrics/leak...
File components/metrics/leak_detector/leak_detector_impl_unittest.cc (right):

https://codereview.chromium.org/1892283004/diff/80001/components/metrics/leak...
components/metrics/leak_detector/leak_detector_impl_unittest.cc:747: // due to
the different rates at which they were generated.
I forgot to remove this in the previous patch set. This is no longer applicable
now that we have a cooldown.

[Simon Que, 2016-04-21 06:24:01]

The CQ bit was checked by sque@chromium.org

[Simon Que, 2016-04-21 06:24:01]

The patchset sent to the CQ was uploaded after l-g-t-m from wfh@chromium.org
Link to the patchset: https://codereview.chromium.org/1892283004/#ps80001
(title: "Rebased; remove JuliaSet history comparison")

[commit-bot: I haz the power, 2016-04-21 06:24:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1892283004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1892283004/80001

[commit-bot: I haz the power, 2016-04-21 07:15:30]

Description was changed from

==========
Add cooldown to LeakDetectorImpl leak report generation

After a report is generated for a particular allocation size
and call site, there is a cooldown period before another
report can be generated for the same size and call site. This
prevents LeakDetectorImpl from generating too reports with
redundant data.

BUG=chromium:605314
TEST=unit tests pass
==========

to

==========
Add cooldown to LeakDetectorImpl leak report generation

After a report is generated for a particular allocation size
and call site, there is a cooldown period before another
report can be generated for the same size and call site. This
prevents LeakDetectorImpl from generating too reports with
redundant data.

BUG=chromium:605314
TEST=unit tests pass
==========

[commit-bot: I haz the power, 2016-04-21 07:15:31]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-22 19:32:04]

Description was changed from

==========
Add cooldown to LeakDetectorImpl leak report generation

After a report is generated for a particular allocation size
and call site, there is a cooldown period before another
report can be generated for the same size and call site. This
prevents LeakDetectorImpl from generating too reports with
redundant data.

BUG=chromium:605314
TEST=unit tests pass
==========

to

==========
Add cooldown to LeakDetectorImpl leak report generation

After a report is generated for a particular allocation size
and call site, there is a cooldown period before another
report can be generated for the same size and call site. This
prevents LeakDetectorImpl from generating too reports with
redundant data.

BUG=chromium:605314
TEST=unit tests pass

Committed: https://crrev.com/7a9e129ec81d69c64a23d1bf7ab83e3f5ae3b3d4
Cr-Commit-Position: refs/heads/master@{#388707}
==========

[commit-bot: I haz the power, 2016-04-22 19:32:05]

Patchset 5 (id:??) landed as
https://crrev.com/7a9e129ec81d69c64a23d1bf7ab83e3f5ae3b3d4
Cr-Commit-Position: refs/heads/master@{#388707}