Add converter from Linux 'perf record'
Needs ReviewPublic

Authored by lunakl on May 20 2019, 1:42 PM.

Details

Reviewers
weidendo
Summary

Originally from
https://github.com/milianw/linux/blob/milian/perf/tools/perf/scripts/python/callgrind.py
I only have made the addr2line usage faster.
After mailing with Milian, for him the script was just POC, he's developed
app called Hotspot aimed specifically at perf and he's not
interested in submitting it upstream. But I find the script useful
(and I really like KCachegrind's call graph), and it does the job for me,
so with his consent I'm submitting it.

Diff Detail

Repository
R49 KCacheGrind
Lint
Lint Skipped
Unit
Unit Tests Skipped
lunakl requested review of this revision.May 20 2019, 1:42 PM
lunakl created this revision.
mwolff added a subscriber: mwolff.May 20 2019, 3:22 PM

FTR: I did give my consent, so thanks @lunakl!

Hey,

I'm not a KDE developer so feel free to ignore my comments, but there are two issues for python 3:

  1. There is invalid whitespace in this file (python3 throws TabError for line 76ff).
  2. dict.iteritems() does not work in python 3, see https://python-future.org/compatible_idioms.html#iterating-through-dict-keys-values-items. I replaced iteritems() by items().

Cheers
Bastian

How do you test it with python3? The script must be run using perf and here on openSUSE 15.0 that uses python2.

I'm on Arch Linux and '/usr/bin/python' is version 3.7.3. I don't know how the python version is determined in scripts run through 'perf script', unfortunately. Could be that they invoke /usr/bin/python (through #!/usr/bin/python?).

Anyway, after fixing the two issues mentioned above your script runs fine with python 3 here...

I'm on Arch Linux and '/usr/bin/python' is version 3.7.3. I don't know how the python version is determined in scripts run through 'perf script', unfortunately. Could be that they invoke /usr/bin/python (through #!/usr/bin/python?).

It doesn't change anything if I change here /usr/bin/python to point to python3, so I guess I can't change that. Can you simply post a fixed version in some form (I don't know if Phabricator lets you edit my patch)?

Here's my modified file.

lunakl updated this revision to Diff 58582.May 24 2019, 8:43 AM

Thank you. I've checked and this works with Python2 too without problems, so I've updated the patch to incorporate these fixes.

pino added a subscriber: pino.May 24 2019, 8:51 AM

A couple of notes:

  • can you please remove the .py extension? the other scripts do not have it
  • what about installing it, just like the other scripts?
  • since it is a new Python script, what about formatting it according to PEP5? (so 4 spaces indentation, 80 chars limit per line, etc)