adb_profile_chrome: Check in perf-to-tracing conversion script

adb_profile_chrome: Check in perf-to-tracing conversion script

Check in perf-to-tracing conversion script. This lets the --perf command
line switch actually produce trace files that can be loaded by Trace
Viewer.

The conversion script was originally written by Victor Miura
<vmiura@chromium.org>; in https://codereview.chromium.org/296623002/.

TEST=PerfControllerTest
BUG=375754
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285022

[Sami, 2014-07-17 18:37:54]

Hey Victor, I swiped this from your patch. With this "adb_profile_chrome --perf"
will give us nice sunbursts in the result.

[vmiura, 2014-07-17 22:51:17]

LGTM with comments.

What version does this patch apply on?  On ToT there's a missing "import signal"
in perf_controller.py

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
File tools/android/adb_profile_chrome/perf_to_tracing.py (right):

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:4: # Licensed under the
terms of the GNU GPL License version 2
Should we replace this with Chrome's license?

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:63: return name
#orign_module
nit: remove comment

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:65: return name #
orign_module
nit: remove comment

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:149: #samp_pid =
param_dict['pid']
nit: Remove commented line

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:218: # Try to reduce
misplaced stack leaves by mobing them up into sibling nodes.
nit: mobing -> moving

[Sami, 2014-07-18 11:19:13]

Thanks! All comments addressed.

> What version does this patch apply on?  On ToT there's a 
> missing "import signal" in perf_controller.py

Ah, looks like that came in recently. Fixed now.

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
File tools/android/adb_profile_chrome/perf_to_tracing.py (right):

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:4: # Licensed under the
terms of the GNU GPL License version 2
On 2014/07/17 22:51:17, vmiura wrote:
> Should we replace this with Chrome's license?

I'm not sure, but I think we need to keep the license from the generated file.

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:63: return name
#orign_module
On 2014/07/17 22:51:17, vmiura wrote:
> nit: remove comment

Done.

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:65: return name #
orign_module
On 2014/07/17 22:51:17, vmiura wrote:
> nit: remove comment

Done.

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:149: #samp_pid =
param_dict['pid']
On 2014/07/17 22:51:17, vmiura wrote:
> nit: Remove commented line

Done.

https://codereview.chromium.org/404543003/diff/1/tools/android/adb_profile_ch...
tools/android/adb_profile_chrome/perf_to_tracing.py:218: # Try to reduce
misplaced stack leaves by mobing them up into sibling nodes.
On 2014/07/17 22:51:17, vmiura wrote:
> nit: mobing -> moving

Done.

[Sami, 2014-07-18 11:19:57]

The CQ bit was checked by skyostil@chromium.org

[commit-bot: I haz the power, 2014-07-18 11:20:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skyostil@chromium.org/404543003/20001

[commit-bot: I haz the power, 2014-07-18 15:33:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-07-18 15:36:35]

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

[commit-bot: I haz the power, 2014-07-18 15:36:36]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[Sami, 2014-07-18 15:45:44]

Looks like the license checker wasn't happy with this.

Danny, are you the right person to okay the exception I added to
tools/licenses.py? FWIW, the tool in that directory isn't shipped with Chrome
and is only used by Chrome developers.

[chromium-reviews, 2014-07-18 16:13:07]

This has no real copyright info. Who owns it? Where did it come from?
(This is one of the reasons why README.chromium exists
Unless there is an amazingly great reason to do so, it should go in
third_party.


On Fri, Jul 18, 2014 at 8:45 AM,  <skyostil@chromium.org> wrote:
> Looks like the license checker wasn't happy with this.
>
> Danny, are you the right person to okay the exception I added to
> tools/licenses.py? FWIW, the tool in that directory isn't shipped with
> Chrome
> and is only used by Chrome developers.
>
> https://codereview.chromium.org/404543003/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Sami, 2014-07-18 16:21:39]

On 2014/07/18 16:13:07, chromium-reviews wrote:
> This has no real copyright info. Who owns it? Where did it come from?
> (This is one of the reasons why README.chromium exists
> Unless there is an amazingly great reason to do so, it should go in
> third_party.

The file was generated by running "perf script -g python", which produces this
output (including the license header): http://pastebin.com/pbDMKtY1. That was
then edited to what you see here. "perf" itself is a tool that lives in the
Linux kernel repository.

Should I add a Chromium copyright at the top?

[vmiura, 2014-07-18 17:49:25]

On 2014/07/18 16:21:39, Sami wrote:
> On 2014/07/18 16:13:07, chromium-reviews wrote:
> > This has no real copyright info. Who owns it? Where did it come from?
> > (This is one of the reasons why README.chromium exists
> > Unless there is an amazingly great reason to do so, it should go in
> > third_party.
> 
> The file was generated by running "perf script -g python", which produces this
> output (including the license header): http://pastebin.com/pbDMKtY1. That was
> then edited to what you see here. "perf" itself is a tool that lives in the
> Linux kernel repository.
> 
> Should I add a Chromium copyright at the top?

Do we really need the GLP license when only the following lines are from the
generated code, which we can just as easily write without generating them?

"def trace_begin():"
"def trace_end():"

[chromium-reviews, 2014-07-18 17:52:05]

On Fri, Jul 18, 2014 at 9:21 AM,  <skyostil@chromium.org> wrote:
> On 2014/07/18 16:13:07, chromium-reviews wrote:
>>
>> This has no real copyright info. Who owns it? Where did it come from?
>> (This is one of the reasons why README.chromium exists
>> Unless there is an amazingly great reason to do so, it should go in
>> third_party.
>
>
> The file was generated by running "perf script -g python", which produces
> this
> output (including the license header): http://pastebin.com/pbDMKtY1. That
> was
> then edited to what you see here. "perf" itself is a tool that lives in the
> Linux kernel repository.

Ah.
>
> Should I add a Chromium copyright at the top?
>
No.
You should, however, at the very least, add a README.chromium file as per
http://www.chromium.org/developers/adding-3rd-party-libraries

> https://codereview.chromium.org/404543003/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Sami, 2014-07-21 10:24:44]

On 2014/07/18 17:52:05, chromium-reviews wrote:
> You should, however, at the very least, add a README.chromium file as per
> http://www.chromium.org/developers/adding-3rd-party-libraries

Thanks, I did this and reorganized the code a bit. PTAL.

[Sami, 2014-07-23 14:25:26]

Danny, ping?

[chromium-reviews, 2014-07-23 15:34:12]

LGTM

On Wed, Jul 23, 2014 at 7:25 AM,  <skyostil@chromium.org> wrote:
> Danny, ping?
>
> https://codereview.chromium.org/404543003/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Sami, 2014-07-23 15:36:57]

The CQ bit was checked by skyostil@chromium.org

[commit-bot: I haz the power, 2014-07-23 15:38:15]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skyostil@chromium.org/404543003/60001

[commit-bot: I haz the power, 2014-07-23 16:50:39]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-07-23 17:11:33]

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

[commit-bot: I haz the power, 2014-07-23 17:11:34]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[Sami, 2014-07-23 17:16:33]

The CQ bit was checked by skyostil@chromium.org

[commit-bot: I haz the power, 2014-07-23 17:17:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skyostil@chromium.org/404543003/60001

[commit-bot: I haz the power, 2014-07-23 18:29:30]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-07-23 18:49:37]

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

[commit-bot: I haz the power, 2014-07-23 18:49:39]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[Sami, 2014-07-23 18:53:06]

The CQ bit was checked by skyostil@chromium.org

[commit-bot: I haz the power, 2014-07-23 18:54:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skyostil@chromium.org/404543003/60001

[commit-bot: I haz the power, 2014-07-23 20:03:06]

Change committed as 285022