Estimates a path in host from a corresponding path in Android device.

Estimates a path in host from a corresponding path in Android device.

For Android, dmprof.py should find symbol information from binaries in
the host instead of the Android device because dmprof.py doesn't run on
the Android device.  They are in different directories.  This change
estimates a path in the host from a corresponding path in the Android
device.

BUG=231800
NOTRY=true

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

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

Marcus,

I guess you're not sure about this script internals, but could you take a look
just at the change points?  It just does an estimation as described in the
"Description (commit log)".

(|fake_directories| is a dict that stores directory conversions.)

[bulach, 2013-05-14 14:44:44]

lgtm, I'm not all familiar, but the changes make sense to me..

just some suggestions to clarify it a little bit.

https://codereview.chromium.org/15035009/diff/1/tools/deep_memory_profiler/dm...
File tools/deep_memory_profiler/dmprof.py (right):

https://codereview.chromium.org/15035009/diff/1/tools/deep_memory_profiler/dm...
tools/deep_memory_profiler/dmprof.py:1064: # Auto-estimating the binary with
symbol information located on the host
nit, I think this would be clearer:
# Translate the symbol information from the device to the
# corresponding files in the host.

https://codereview.chromium.org/15035009/diff/1/tools/deep_memory_profiler/dm...
tools/deep_memory_profiler/dmprof.py:1068: lib_on_device =
Command._suppose_fake_directories(prefix)
nit: perhaps
device_lib = Command._get_device_lib(prefix)

https://codereview.chromium.org/15035009/diff/1/tools/deep_memory_profiler/dm...
tools/deep_memory_profiler/dmprof.py:1110: def
_suppose_fake_directories(prefix):
nit: perhaps a comment

"""Return the path for the application library.

On android, returns the name if the map contains only one application library
(that is, an .so inside "/data/app-lib").
Returns None otherwise.
"""

[Dai Mikurube (NOT FULLTIME), 2013-05-14 17:21:47]

Thanks for the comment!  Updated the patch.

https://codereview.chromium.org/15035009/diff/1/tools/deep_memory_profiler/dm...
File tools/deep_memory_profiler/dmprof.py (right):

https://codereview.chromium.org/15035009/diff/1/tools/deep_memory_profiler/dm...
tools/deep_memory_profiler/dmprof.py:1064: # Auto-estimating the binary with
symbol information located on the host
On 2013/05/14 14:44:44, bulach wrote:
> nit, I think this would be clearer:
> # Translate the symbol information from the device to the
> # corresponding files in the host.

To make it clear that it's an estimation,

1) Renamed all fake_directories to alternative_dirs

2) Rewrote this comment to:

# Translate the symbol information from the Android device to the
# corresponding files in the host.  The location of the files in the host
# is estimated from the binary path on the device.  Use --alternative-dirs
# to specify the location manually.

3) Changed the behavior and added a comment on _estimate_alternative_dirs (fka
_suppose_fake_directories)

https://codereview.chromium.org/15035009/diff/1/tools/deep_memory_profiler/dm...
tools/deep_memory_profiler/dmprof.py:1068: lib_on_device =
Command._suppose_fake_directories(prefix)
On 2013/05/14 14:44:44, bulach wrote:
> nit: perhaps
> device_lib = Command._get_device_lib(prefix)

Finally, it's _estimate_alternative_dirs.

https://codereview.chromium.org/15035009/diff/1/tools/deep_memory_profiler/dm...
tools/deep_memory_profiler/dmprof.py:1110: def
_suppose_fake_directories(prefix):
On 2013/05/14 14:44:44, bulach wrote:
> nit: perhaps a comment
> 
> """Return the path for the application library.
> 
> On android, returns the name if the map contains only one application library
> (that is, an .so inside "/data/app-lib").
> Returns None otherwise.
> """

As written above.

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

+peria as a reviewer.

It's an additional change to http://crrev.com/198881.

[peria, 2013-05-15 17:03:21]

please go forward, if my understanding is wrong.

https://codereview.chromium.org/15035009/diff/5001/tools/deep_memory_profiler...
File tools/deep_memory_profiler/dmprof.py (right):

https://codereview.chromium.org/15035009/diff/5001/tools/deep_memory_profiler...
tools/deep_memory_profiler/dmprof.py:1063: # Translate the symbol information
from the Android device to the
I'm not sure, but does this method work only for Android?
If not, please put "if for Android ..."

[Dai Mikurube (NOT FULLTIME), 2013-05-15 22:44:38]

Thanks for looking, peria.

https://codereview.chromium.org/15035009/diff/5001/tools/deep_memory_profiler...
File tools/deep_memory_profiler/dmprof.py (right):

https://codereview.chromium.org/15035009/diff/5001/tools/deep_memory_profiler...
tools/deep_memory_profiler/dmprof.py:1063: # Translate the symbol information
from the Android device to the
On 2013/05/15 17:03:21, peria wrote:
> I'm not sure, but does this method work only for Android?
> If not, please put "if for Android ..."

This method is called even for Linux (not Android) processes.  It assumes that
it's working on Android if the binary is under "/data/app-lib".  (We have no
good way to determine the process was working on Android rather than estimating
from the path.)

Then, I'll add "If the target process is estimated to be working on Android, "
in the comment.  What do you think?

[peria, 2013-05-16 00:03:32]

On 2013/05/15 22:44:38, Dai Mikurube wrote:
> Thanks for looking, peria.
> 
>
https://codereview.chromium.org/15035009/diff/5001/tools/deep_memory_profiler...
> File tools/deep_memory_profiler/dmprof.py (right):
> 
>
https://codereview.chromium.org/15035009/diff/5001/tools/deep_memory_profiler...
> tools/deep_memory_profiler/dmprof.py:1063: # Translate the symbol information
> from the Android device to the
> On 2013/05/15 17:03:21, peria wrote:
> > I'm not sure, but does this method work only for Android?
> > If not, please put "if for Android ..."
> 
> This method is called even for Linux (not Android) processes.  It assumes that
> it's working on Android if the binary is under "/data/app-lib".  (We have no
> good way to determine the process was working on Android rather than
estimating
> from the path.)
> 
> Then, I'll add "If the target process is estimated to be working on Android, "
> in the comment.  What do you think?

SGTM.
What I wanted was to make it clear that this method had a branch based on
platforms.

[Dai Mikurube (NOT FULLTIME), 2013-05-16 10:14:38]

Maybe still a cryptic description. ;)  Could you take another look, bulach?

[peria, 2013-05-16 10:16:39]

LGTM, thanks

[Dai Mikurube (NOT FULLTIME), 2013-05-20 09:48:34]

If bulach@ is happy with it, I'll be committing it. :)

[bulach, 2013-05-20 09:52:34]

lgtm, thanks!

[Dai Mikurube (NOT FULLTIME), 2013-05-20 09:55:30]

Thanks!

[commit-bot: I haz the power, 2013-05-20 09:55:35]

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

[commit-bot: I haz the power, 2013-05-20 09:55:47]

Change committed as 201062