Dump /proc/<pid>/maps with every heap profile dump.

Dump /proc/<pid>/maps with every heap profile dump.


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

[Dai Mikurube (NOT FULLTIME), 2012-08-14 06:33:37]

Hi Alexander,

Could you take a look at this change?

[Alexander Potapenko, 2012-08-14 08:39:57]

http://codereview.chromium.org/10832290/diff/1/third_party/tcmalloc/chromium/...
File third_party/tcmalloc/chromium/src/deep-heap-profile.cc (right):

http://codereview.chromium.org/10832290/diff/1/third_party/tcmalloc/chromium/...
third_party/tcmalloc/chromium/src/deep-heap-profile.cc:94:
WriteMapsToFile(filename_prefix_, 0,
Why is this 0?
If the process didn't call exec() you're going to have the same memory mappings
and the same allocation history after fork(), so the only thing you need to do
is to reopen the correct /proc/<pid>/maps.

http://codereview.chromium.org/10832290/diff/1/third_party/tcmalloc/chromium/...
third_party/tcmalloc/chromium/src/deep-heap-profile.cc:97:
WriteMapsToFile(filename_prefix_, dump_count_,
Are you going to write the mappings twice if most_recent_pid_ != getpid()?

[Dai Mikurube (NOT FULLTIME), 2012-08-14 09:33:25]

Thank you for reviewing.

http://codereview.chromium.org/10832290/diff/1/third_party/tcmalloc/chromium/...
File third_party/tcmalloc/chromium/src/deep-heap-profile.cc (right):

http://codereview.chromium.org/10832290/diff/1/third_party/tcmalloc/chromium/...
third_party/tcmalloc/chromium/src/deep-heap-profile.cc:94:
WriteMapsToFile(filename_prefix_, 0,
On 2012/08/14 08:39:57, Alexander Potapenko wrote:
> Why is this 0?
> If the process didn't call exec() you're going to have the same memory
mappings
> and the same allocation history after fork(), so the only thing you need to do
> is to reopen the correct /proc/<pid>/maps.

Data in /proc/<pid>/maps is used in this dumper "deep-heap-profile", and in the
post-mortem analyzer script "dmprof".  dmprof expects a set of files for a
Chrome process:

* prefix.<pid>.maps: maps when the process started or forked
* prefix.<pid>.<nnnn>.heap: heap profile dumps
* prefix.<pid>.<nnnn>.buckets: bucket lists

To make it easy to manage all files for one process, deep-heap-profile has
dumped "maps" in the beginning of every process.  (We can copy required files by
"prefix.<pid>.*".)


By this change, deep-heap-profile dumps "maps" for every request in addition to
one in the beginning of the process.  It is to understand memory mapping history
of the process.

We will have the following "maps" files :

* prefix.<pid>.maps: maps when the process started or forked
* prefix.<pid>.<nnnn>.maps: maps when requested to dump

"0" for WriteMapsToFile is for the former.

http://codereview.chromium.org/10832290/diff/1/third_party/tcmalloc/chromium/...
third_party/tcmalloc/chromium/src/deep-heap-profile.cc:97:
WriteMapsToFile(filename_prefix_, dump_count_,
On 2012/08/14 08:39:57, Alexander Potapenko wrote:
> Are you going to write the mappings twice if most_recent_pid_ != getpid()?

Yes, I know normally "prefix.<pid>.maps" and "prefix.<pid>.0001.maps" will be
same.

Some existing scripts assume "prefix.<pid>.maps".  I'd like to dump both for a
while for compatibility.  (Will remove the former.)

I think it is not a large problem since "maps" is not large.

[Alexander Potapenko, 2012-08-14 09:36:47]

LGTM with a nit.

http://codereview.chromium.org/10832290/diff/1/third_party/tcmalloc/chromium/...
File third_party/tcmalloc/chromium/src/deep-heap-profile.cc (right):

http://codereview.chromium.org/10832290/diff/1/third_party/tcmalloc/chromium/...
third_party/tcmalloc/chromium/src/deep-heap-profile.cc:94:
WriteMapsToFile(filename_prefix_, 0,
On 2012/08/14 09:33:26, Dai Mikurube wrote:
> On 2012/08/14 08:39:57, Alexander Potapenko wrote:
> > Why is this 0?
> > If the process didn't call exec() you're going to have the same memory
> mappings
> > and the same allocation history after fork(), so the only thing you need to
do
> > is to reopen the correct /proc/<pid>/maps.
> 
> Data in /proc/<pid>/maps is used in this dumper "deep-heap-profile", and in
the
> post-mortem analyzer script "dmprof".  dmprof expects a set of files for a
> Chrome process:
> 
> * prefix.<pid>.maps: maps when the process started or forked
> * prefix.<pid>.<nnnn>.heap: heap profile dumps
> * prefix.<pid>.<nnnn>.buckets: bucket lists
> 
> To make it easy to manage all files for one process, deep-heap-profile has
> dumped "maps" in the beginning of every process.  (We can copy required files
by
> "prefix.<pid>.*".)
> 
> 
> By this change, deep-heap-profile dumps "maps" for every request in addition
to
> one in the beginning of the process.  It is to understand memory mapping
history
> of the process.
> 
> We will have the following "maps" files :
> 
> * prefix.<pid>.maps: maps when the process started or forked
> * prefix.<pid>.<nnnn>.maps: maps when requested to dump
> 
> "0" for WriteMapsToFile is for the former.

Can you please prepend a comment to "0" so that the reader could understand what
this parameter stands for? (Because zero count has a special meaning, it may be
better to note that)

[Dai Mikurube (NOT FULLTIME), 2012-08-14 10:17:35]

Thanks.  Added comments.

Will be checking the "Commit" box.

[commit-bot: I haz the power, 2012-08-14 10:18:18]

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

[commit-bot: I haz the power, 2012-08-14 12:40:09]

Change committed as 151465