Add the reason in heap profiler dumps.

Add the reason in heap profiler dumps.

BUG=123750
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202493

[Dai Mikurube (NOT FULLTIME), 2013-05-14 09:56:10]

Could you take a look when you have time?  It's NOT time-sensitive. (It doesn't
affect anywhere... just adds supplemental information in dumped files.)

[Dai Mikurube (NOT FULLTIME), 2013-05-22 21:41:33]

rebased.

[Dai Mikurube (NOT FULLTIME), 2013-05-27 08:45:40]

Hi Alexander,

Could you take a look at this?  It just adds "reason" in dee-heap-profile's
dumped files.

"reason" is the string argument of HeapProfilerDump() which is just a hint of
"why the dump is requested".  For example, "60 seconds has passed since last
dump", "by user's manual request", "1000MB are newly allocated" and else...

[Dai Mikurube (NOT FULLTIME), 2013-05-27 08:50:11]

(looking only at third_party/tcmalloc is fine, glider)

[Alexander Potapenko, 2013-05-27 08:54:44]

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
File third_party/tcmalloc/chromium/src/deep-heap-profile.h (right):

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
third_party/tcmalloc/chromium/src/deep-heap-profile.h:86: int
FillOrderedProfile(char raw_buffer[],
The style guide suggests to put the input parameters before the outputs. I think
it's reasonable to put |reason| first (better keep the output buffer size after
the buffer itself)

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
third_party/tcmalloc/chromium/src/deep-heap-profile.h:140: bool
AppendString(const char* v, int d);
While here, can you please choose more informative names for the parameters of
AppendXxx? I had a hard time guessing what's |d|.

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
File third_party/tcmalloc/chromium/src/heap-profiler.cc (right):

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
third_party/tcmalloc/chromium/src/heap-profiler.cc:228: static char*
DoGetHeapProfileLocked(char* buf, int buflen, const char* reason) {
It's probably better to put |reason| first. I don't strongly insist on this
neither here, nor in deep-heap-profile.h

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
third_party/tcmalloc/chromium/src/heap-profiler.cc:260: return
DoGetHeapProfileLocked(buffer, kProfileBufferSize, NULL);
/*reason*/NULL

[Dai Mikurube (NOT FULLTIME), 2013-05-27 10:08:23]

Thanks for your comments!  Updated the patch.

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
File third_party/tcmalloc/chromium/src/deep-heap-profile.h (right):

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
third_party/tcmalloc/chromium/src/deep-heap-profile.h:86: int
FillOrderedProfile(char raw_buffer[],
On 2013/05/27 08:54:44, Alexander Potapenko wrote:
> The style guide suggests to put the input parameters before the outputs. I
think
> it's reasonable to put |reason| first (better keep the output buffer size
after
> the buffer itself)

Thanks, sounds reasonable.  Done.

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
third_party/tcmalloc/chromium/src/deep-heap-profile.h:140: bool
AppendString(const char* v, int d);
On 2013/05/27 08:54:44, Alexander Potapenko wrote:
> While here, can you please choose more informative names for the parameters of
> AppendXxx? I had a hard time guessing what's |d|.

Done.

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
File third_party/tcmalloc/chromium/src/heap-profiler.cc (right):

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
third_party/tcmalloc/chromium/src/heap-profiler.cc:228: static char*
DoGetHeapProfileLocked(char* buf, int buflen, const char* reason) {
On 2013/05/27 08:54:44, Alexander Potapenko wrote:
> It's probably better to put |reason| first. I don't strongly insist on this
> neither here, nor in deep-heap-profile.h

Reasonable, too.  Done.

https://codereview.chromium.org/14823009/diff/16001/third_party/tcmalloc/chro...
third_party/tcmalloc/chromium/src/heap-profiler.cc:260: return
DoGetHeapProfileLocked(buffer, kProfileBufferSize, NULL);
On 2013/05/27 08:54:44, Alexander Potapenko wrote:
> /*reason*/NULL

Done.

[Alexander Potapenko, 2013-05-27 10:17:00]

LGTM

[Dai Mikurube (NOT FULLTIME), 2013-05-27 10:19:36]

Thanks!

Willchan, could you have OWNER's look on heap-profiler.cc?

[willchan no longer on Chromium, 2013-05-27 18:05:59]

lgtm

[commit-bot: I haz the power, 2013-05-28 01:33:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmikurube@chromium.org/14823009/25002

[Dai Mikurube (NOT FULLTIME), 2013-05-28 01:34:01]

Thanks!  Committing.

[commit-bot: I haz the power, 2013-05-28 03:47:34]

Change committed as 202493