Use find_runtime_symbols in dmprof.

Use find_runtime_symbols in dmprof.


BUG=123763
TEST=use dmprof.


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

[Dai Mikurube (NOT FULLTIME), 2012-07-22 13:45:58]

Hi Marc,

Could you take a look at this change?  It enables find_runtime_symbols
(http://codereview.chromium.org/10795028/) instead of pprof in dmprof.

It's just quick enabling patch.  For its coding style and ugly command line, I'm
preparing another large change at http://codereview.chromium.org/10802049/ (it's
not ready for review yet).

[M-A Ruel, 2012-07-22 19:49:06]

http://codereview.chromium.org/10795043/diff/2002/tools/deep_memory_profiler/...
File tools/deep_memory_profiler/dmprof (right):

http://codereview.chromium.org/10795043/diff/2002/tools/deep_memory_profiler/...
tools/deep_memory_profiler/dmprof:582: prepared_data_dir = tempfile.mkdtemp()
Is it wanted that this directory is leaked?

[M-A Ruel, 2012-07-22 19:51:07]

http://codereview.chromium.org/10795043/diff/2002/tools/deep_memory_profiler/...
File tools/deep_memory_profiler/dmprof (right):

http://codereview.chromium.org/10795043/diff/2002/tools/deep_memory_profiler/...
tools/deep_memory_profiler/dmprof:16: FIND_RUNTIME_SYMBOLS_PATH =
os.path.join(os.path.dirname(__file__),
os.path.dirname(os.path.abspath(__file__))
is safer

http://codereview.chromium.org/10795043/diff/2002/tools/deep_memory_profiler/...
tools/deep_memory_profiler/dmprof:602: symbol_f.write('%s %s\n' % (address,
symbol.strip()))
I don't see why you are making a local variable and two lines after you are not
using it.

[Dai Mikurube (NOT FULLTIME), 2012-07-23 00:48:44]

Thanks, Marc.

Updated.

http://codereview.chromium.org/10795043/diff/2002/tools/deep_memory_profiler/...
File tools/deep_memory_profiler/dmprof (right):

http://codereview.chromium.org/10795043/diff/2002/tools/deep_memory_profiler/...
tools/deep_memory_profiler/dmprof:16: FIND_RUNTIME_SYMBOLS_PATH =
os.path.join(os.path.dirname(__file__),
On 2012/07/22 19:51:07, Marc-Antoine Ruel wrote:
> os.path.dirname(os.path.abspath(__file__))
> is safer

Done.

http://codereview.chromium.org/10795043/diff/2002/tools/deep_memory_profiler/...
tools/deep_memory_profiler/dmprof:582: prepared_data_dir = tempfile.mkdtemp()
On 2012/07/22 19:49:06, Marc-Antoine Ruel wrote:
> Is it wanted that this directory is leaked?

Ah, no.  Moved these calls and used shutil.rmtree.

http://codereview.chromium.org/10795043/diff/2002/tools/deep_memory_profiler/...
tools/deep_memory_profiler/dmprof:602: symbol_f.write('%s %s\n' % (address,
symbol.strip()))
On 2012/07/22 19:51:07, Marc-Antoine Ruel wrote:
> I don't see why you are making a local variable and two lines after you are
not
> using it.

Done.

[M-A Ruel, 2012-07-23 12:01:20]

lgtm with fix.

http://codereview.chromium.org/10795043/diff/9001/tools/deep_memory_profiler/...
File tools/deep_memory_profiler/dmprof (right):

http://codereview.chromium.org/10795043/diff/9001/tools/deep_memory_profiler/...
tools/deep_memory_profiler/dmprof:597: prepare_symbol_info(maps_path,
prepared_data_dir)
wrap lines 597-606 into try: and line 607 inside finally:, otherwise you'll leak
on exception.

[Dai Mikurube (NOT FULLTIME), 2012-07-23 13:44:01]

Thanks, Marc.

Checked the "Commit".

On 2012/07/23 12:01:20, Marc-Antoine Ruel wrote:
> lgtm with fix.
> 
>
http://codereview.chromium.org/10795043/diff/9001/tools/deep_memory_profiler/...
> File tools/deep_memory_profiler/dmprof (right):
> 
>
http://codereview.chromium.org/10795043/diff/9001/tools/deep_memory_profiler/...
> tools/deep_memory_profiler/dmprof:597: prepare_symbol_info(maps_path,
> prepared_data_dir)
> wrap lines 597-606 into try: and line 607 inside finally:, otherwise you'll
leak
> on exception.

[commit-bot: I haz the power, 2012-07-23 13:44:06]

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

[commit-bot: I haz the power, 2012-07-23 15:48:43]

Change committed as 147856