Add multi-architecture support in telemetry.

Add multi-architecture support in telemetry.

Cleanup README file.

BUG=422291
Committed: https://crrev.com/1d468e04dac216371b96c123b17dd5c2b80935ac
Cr-Commit-Position: refs/heads/master@{#299682}

[Fabrice (no longer in Chrome), 2014-10-10 16:23:34]

fdegans@chromium.org changed reviewers:
+ pasko@chromium.org, skyostil@chromium.org, tonyg@chromium.org

[Fabrice (no longer in Chrome), 2014-10-10 16:23:34]

PTAL
I'm not very familiar with Python but I don't think I did anything horribly
wrong.

[pasko, 2014-10-10 16:45:39]

lgtm

[tonyg, 2014-10-10 16:51:15]

https://codereview.chromium.org/643973002/diff/1/tools/telemetry/telemetry/co...
File
tools/telemetry/telemetry/core/platform/profiler/android_profiling_helper.py
(right):

https://codereview.chromium.org/643973002/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/profiler/android_profiling_helper.py:228:
perf_binary = 'perf_' + device.GetProp('ro.product.cpu.abi')
This is a general problem, not specific to the perf binary or to the android
platform.

Take a look at support_binaries.py and how it chooses the folder automatically
based on the platform. I had pictured that we'd add another level of nesting for
the architecture -- here:

def _GetBinPath(binary_name, platform_name):
  # TODO(tonyg): Add another nesting level for architecture_name.
  return os.path.join(path.GetTelemetryDir(), 'bin', platform_name, binary_name)

So then instead of renaming perf.sha1, we'd move it to
bin/android/armeabi-v7a/perf.sha1.

Would you be willing to rework this CL to generalize this?

[pasko, 2014-10-10 17:04:07]

https://codereview.chromium.org/643973002/diff/1/tools/telemetry/telemetry/co...
File
tools/telemetry/telemetry/core/platform/profiler/android_profiling_helper.py
(right):

https://codereview.chromium.org/643973002/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/profiler/android_profiling_helper.py:228:
perf_binary = 'perf_' + device.GetProp('ro.product.cpu.abi')
On 2014/10/10 16:51:14, tonyg wrote:
> This is a general problem, not specific to the perf binary or to the android
> platform.
> 
> Take a look at support_binaries.py and how it chooses the folder automatically
> based on the platform. I had pictured that we'd add another level of nesting
for
> the architecture -- here:
> 
> def _GetBinPath(binary_name, platform_name):
>   # TODO(tonyg): Add another nesting level for architecture_name.
>   return os.path.join(path.GetTelemetryDir(), 'bin', platform_name,
binary_name)
> 
> So then instead of renaming perf.sha1, we'd move it to
> bin/android/armeabi-v7a/perf.sha1.

Oops, I also recently added perfhost_precise and perfhost_trusty :)

I am slightly biased towards perf_armeabi-v7a because in the future we might
need to support multiple ABIs on the single device, which probably means
multiple perf binaries installed side by side (not 100% sure about that). We
would probably never need to run two different perf binaries concurrently
anyway, but maybe it would be easier to tell which binary is used on the device
by looking at the filename itself or the logs on the bots.

Other applications do similar things. For example, the GCC binary I use is named
as "arm-linux-androideabi-gcc".

But I am not strongly opposed to the alternative you proposed, it would work
either way.

[tonyg, 2014-10-10 17:39:30]

On 2014/10/10 17:04:07, pasko wrote:
>
https://codereview.chromium.org/643973002/diff/1/tools/telemetry/telemetry/co...
> File
> tools/telemetry/telemetry/core/platform/profiler/android_profiling_helper.py
> (right):
> 
>
https://codereview.chromium.org/643973002/diff/1/tools/telemetry/telemetry/co...
>
tools/telemetry/telemetry/core/platform/profiler/android_profiling_helper.py:228:
> perf_binary = 'perf_' + device.GetProp('ro.product.cpu.abi')
> On 2014/10/10 16:51:14, tonyg wrote:
> > This is a general problem, not specific to the perf binary or to the android
> > platform.
> > 
> > Take a look at support_binaries.py and how it chooses the folder
automatically
> > based on the platform. I had pictured that we'd add another level of nesting
> for
> > the architecture -- here:
> > 
> > def _GetBinPath(binary_name, platform_name):
> >   # TODO(tonyg): Add another nesting level for architecture_name.
> >   return os.path.join(path.GetTelemetryDir(), 'bin', platform_name,
> binary_name)
> > 
> > So then instead of renaming perf.sha1, we'd move it to
> > bin/android/armeabi-v7a/perf.sha1.
> 
> Oops, I also recently added perfhost_precise and perfhost_trusty :)
> 
> I am slightly biased towards perf_armeabi-v7a because in the future we might
> need to support multiple ABIs on the single device, which probably means
> multiple perf binaries installed side by side (not 100% sure about that). We
> would probably never need to run two different perf binaries concurrently
> anyway, but maybe it would be easier to tell which binary is used on the
device
> by looking at the filename itself or the logs on the bots.
> 
> Other applications do similar things. For example, the GCC binary I use is
named
> as "arm-linux-androideabi-gcc".
> 
> But I am not strongly opposed to the alternative you proposed, it would work
> either way.

Regardless of naming convention, my main point is that we should have one place
in code that picks the correct binary.

[pasko, 2014-10-14 14:15:42]

https://codereview.chromium.org/643973002/diff/150001/build/android/pylib/dev...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/643973002/diff/150001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:739: return
self.GetProp('ro.product.cpu.abi')
sounds good enough for now :)

https://codereview.chromium.org/643973002/diff/150001/tools/telemetry/telemet...
File tools/telemetry/telemetry/core/platform/linux_platform_backend.py (right):

https://codereview.chromium.org/643973002/diff/150001/tools/telemetry/telemet...
tools/telemetry/telemetry/core/platform/linux_platform_backend.py:130: 'ipfw',
self.GetArchName(), self.GetOSName())
I think Tony means that support_binaries should know which binary to get without
providing with the arch name. I am not sure how to do that in presence of both
host and target platforms. Tony, would you like a parameter like host=True/False
for this? Any other suggestions?

I am OK with either approach actually.

[Fabrice (no longer in Chrome), 2014-10-14 14:47:29]

fdegans@chromium.org changed reviewers:
+ jbudorick@chromium.org

[Fabrice (no longer in Chrome), 2014-10-14 14:47:30]

+ jbudorick@ for device_utils.py change.
I added architecture support for every platform. That is, if I understood tony's
comment properly :)

[jbudorick, 2014-10-14 14:51:52]

https://codereview.chromium.org/643973002/diff/170001/build/android/pylib/dev...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/643973002/diff/170001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:736: def GetABI(self):
Public functions in DeviceUtils should expose timeout & retry parameters.

https://codereview.chromium.org/643973002/diff/170001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:738: """
nit: document Returns: & Raises: (should be the same as GetProp)

[Fabrice (no longer in Chrome), 2014-10-14 16:26:03]

Thanks for the quick review!
I think that should cover it.

https://codereview.chromium.org/643973002/diff/170001/build/android/pylib/dev...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/643973002/diff/170001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:736: def GetABI(self):
On 2014/10/14 14:51:52, jbudorick wrote:
> Public functions in DeviceUtils should expose timeout & retry parameters.

Done.

https://codereview.chromium.org/643973002/diff/170001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:738: """
On 2014/10/14 14:51:52, jbudorick wrote:
> nit: document Returns: & Raises: (should be the same as GetProp)

Done.

[jbudorick, 2014-10-14 16:27:13]

device_utils.py lgtm

[tonyg, 2014-10-15 00:30:49]

lgtm assuming the unittests are happy

Thanks!

[Fabrice (no longer in Chrome), 2014-10-15 10:32:05]

The CQ bit was checked by fdegans@chromium.org

[commit-bot: I haz the power, 2014-10-15 10:33:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/643973002/190001

[commit-bot: I haz the power, 2014-10-15 11:20:27]

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

[commit-bot: I haz the power, 2014-10-15 11:20:28]

Try jobs failed on following builders:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[Fabrice (no longer in Chrome), 2014-10-15 12:02:24]

The CQ bit was checked by fdegans@chromium.org

[commit-bot: I haz the power, 2014-10-15 12:03:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/643973002/210001

[commit-bot: I haz the power, 2014-10-15 13:34:21]

Committed patchset #5 (id:210001)

[commit-bot: I haz the power, 2014-10-15 13:35:11]

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