[0/5] Build trace-cruncher as Python pakage

Message

Yordan Karadzhov Dec. 12, 2019, 9:02 a.m. UTC

This patch-set is an attempt to restructure the project and to make it
build as a native Python package. Although it looks like a complete
rewrite, this is essentially just a switching from using Cython to using
directly the C API of Python. Cython is still being used but only for
the implementation of the NumPy data wrapper. The new package has its
own stand-alone build system (very primitive for the moment) that is
completely decoupled from the existing build system used by tracecruncher.
In order to build and install the new package do:

sudo python setup.py install --record files.txt

The patch-set does not  remove the old implementation yet. This will
happen in another successive patch-set.

Please review as careful as possible!

Yordan Karadzhov (VMware) (5):
  Refactor the part of the interface that relies on libkshark
  Refactor the part of the interface that relies on libtraceevent
  Refactor NumPy based data wrapper
  Add "utils"
  Adapt the sched_wakeup.py example script to use the new tracecruncher
    module

 examples/sched_wakeup.py  |  30 ++---
 setup.py                  |  61 +++++++++
 src/common.h              |  20 +++
 src/datawrapper.pyx       | 201 ++++++++++++++++++++++++++++
 src/ftracepy.c            | 234 +++++++++++++++++++++++++++++++++
 src/ksharkpy.c            | 268 ++++++++++++++++++++++++++++++++++++++
 src/trace2matrix.c        |  29 +++++
 tracecruncher/__init__.py |   0
 tracecruncher/utils.py    |  54 ++++++++
 9 files changed, 882 insertions(+), 15 deletions(-)
 create mode 100644 setup.py
 create mode 100644 src/common.h
 create mode 100644 src/datawrapper.pyx
 create mode 100644 src/ftracepy.c
 create mode 100644 src/ksharkpy.c
 create mode 100644 src/trace2matrix.c
 create mode 100644 tracecruncher/__init__.py
 create mode 100644 tracecruncher/utils.py

Comments

Douglas RAILLARD Dec. 31, 2019, 6:37 p.m. UTC | #1

Hi Yordan,

Finally got around to trying trace-cruncher. I've rediscovered how much I hated C build
systems issues in general, so here is the outcome of the first day of battle :-)

kernelshark bits:
* kernelshark build system seems to ignore the standard CMAKE_INSTALL_PREFIX
  but uses _INSTALL_PREFIX instead, might be worth aligning with the documented
  standard stuff of CMake if that makes sense.
* _INSTALL_PREFIX is used for almost everything, except a polkit file that is copied
  to /usr/share/polkit-1 regardless of _INSTALL_PREFIX, which makes installing as
  non root fail.

Now trace-cruncher-specific bits:
* Building kernelshark gives shared objects with .so.X.Y.Z file names, but GCC
  wants to link against .so
* There are some hardcoded paths to /usr/local/ in setup.py. They don't seem
  to break user-given include paths but it would probably be a good idea to
  make that somewhat parametric.
* libkshark-input.h does not seem to exist but is used by ksharkpy.c. I'm not sure
  if I used the wrong version of something or if that's a genuine issue.

Here is the command I used to try to build trace-cruncher in a venv with
numpy and cython installed:
$ pip install . --global-option=build_ext --global-option='-Itrace-cmd/install/include' --global-option='-Ltrace-cmd/install/lib/kernelshark:trace-cmd/install/lib/trace-cmd:trace-cmd/install/lib/traceevent'

I've pushed some WIP commits on top of your series on my fork if you
want to get a look at them:

Branch: refactor
remote: https://github.com/douglas-raillard-arm/trace-cruncher.git
https://github.com/douglas-raillard-arm/trace-cruncher/commits/refactor

For the sake of reproducibility until things become stable, it might be a good idea
to maintain a WIP branch with submodules/subtrees for trace-cmd in the right version
in trace-cruncher repo, and the series applied, so we don't have to jugle with the
ML lore to get mbox plus other patches from the repo itself etc. Once we have that,
we can easily include a script that calls all the build systems involved and install all the
required bits in a venv, without polluting system-wide locations, and progressively
turn that into a standalone setup.py (or the nearest approximation of that we
manage to make).

PS: apologies for the extra new lines and lack of reply markers,
Office 365 webmail does not seem to play very nicely with plain text ...

Cheers,
Douglas

From: Yordan Karadzhov (VMware) <[e-mail]>

Sent: 12 December 2019 09:02

To: [e-mail] <[e-mail]>

Cc: [e-mail] <[e-mail]>; Valentin Schneider <[e-mail]>; Douglas Raillard <[e-mail]>; Yordan Karadzhov (VMware) <[e-mail]>

Subject: [PATCH 0/5] Build trace-cruncher as Python pakage




This patch-set is an attempt to restructure the project and to make it

build as a native Python package. Although it looks like a complete

rewrite, this is essentially just a switching from using Cython to using

directly the C API of Python. Cython is still being used but only for

the implementation of the NumPy data wrapper. The new package has its

own stand-alone build system (very primitive for the moment) that is

completely decoupled from the existing build system used by tracecruncher.

In order to build and install the new package do:



sudo python setup.py install --record files.txt



The patch-set does not  remove the old implementation yet. This will

happen in another successive patch-set.



Please review as careful as possible!



Yordan Karadzhov (VMware) (5):

  Refactor the part of the interface that relies on libkshark

  Refactor the part of the interface that relies on libtraceevent

  Refactor NumPy based data wrapper

  Add "utils"

  Adapt the sched_wakeup.py example script to use the new tracecruncher

    module



 examples/sched_wakeup.py  |  30 ++---

 setup.py                  |  61 +++++++++

 src/common.h              |  20 +++

 src/datawrapper.pyx       | 201 ++++++++++++++++++++++++++++

 src/ftracepy.c            | 234 +++++++++++++++++++++++++++++++++

 src/ksharkpy.c            | 268 ++++++++++++++++++++++++++++++++++++++

 src/trace2matrix.c        |  29 +++++

 tracecruncher/__init__.py |   0

 tracecruncher/utils.py    |  54 ++++++++

 9 files changed, 882 insertions(+), 15 deletions(-)

 create mode 100644 setup.py

 create mode 100644 src/common.h

 create mode 100644 src/datawrapper.pyx

 create mode 100644 src/ftracepy.c

 create mode 100644 src/ksharkpy.c

 create mode 100644 src/trace2matrix.c

 create mode 100644 tracecruncher/__init__.py

 create mode 100644 tracecruncher/utils.py



--

2.20.1



IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Yordan Karadzhov Jan. 7, 2020, 4:59 p.m. UTC | #2

Hi Douglas,

Happy new year!
Thanks a lot for trying trace-cruncher and providing us with useful 
feedback!

On 31.12.19 г. 20:37 ч., Douglas Raillard wrote:
> Hi Yordan,
> 
> Finally got around to trying trace-cruncher. I've rediscovered how much I hated C build
> systems issues in general, so here is the outcome of the first day of battle :-)
> 
> kernelshark bits:
> * kernelshark build system seems to ignore the standard CMAKE_INSTALL_PREFIX
>    but uses _INSTALL_PREFIX instead, might be worth aligning with the documented
>    standard stuff of CMake if that makes sense.
> * _INSTALL_PREFIX is used for almost everything, except a polkit file that is copied
>    to /usr/share/polkit-1 regardless of _INSTALL_PREFIX, which makes installing as
>    non root fail.
> 
> Now trace-cruncher-specific bits:
> * Building kernelshark gives shared objects with .so.X.Y.Z file names, but GCC
>    wants to link against .so
> * There are some hardcoded paths to /usr/local/ in setup.py. They don't seem
>    to break user-given include paths but it would probably be a good idea to
>    make that somewhat parametric.
> * libkshark-input.h does not seem to exist but is used by ksharkpy.c. I'm not sure
>    if I used the wrong version of something or if that's a genuine issue.
> 

I am going to send a second version of the patch-set that tries to 
address some of the problems you found. This second version of the 
patches goes a bit further and simplifies/automates the build process 
for the user. Now you only have to do:

 > make
 > sudo make install

and that's all. You also have "make clean" and "make uninstall". I hope 
this will fix the problems you reported above.

> Here is the command I used to try to build trace-cruncher in a venv with
> numpy and cython installed:
> $ pip install . --global-option=build_ext --global-option='-Itrace-cmd/install/include' --global-option='-Ltrace-cmd/install/lib/kernelshark:trace-cmd/install/lib/trace-cmd:trace-cmd/install/lib/traceevent'
> 
> I've pushed some WIP commits on top of your series on my fork if you
> want to get a look at them:
> 
> Branch: refactor
> remote: https://github.com/douglas-raillard-arm/trace-cruncher.git
> https://github.com/douglas-raillard-arm/trace-cruncher/commits/refactor
>

All new patches are available here:
https://github.com/vmware/trace-cruncher/tree/refactoring_WIP (branch 
"refactoring_WIP")

> For the sake of reproducibility until things become stable, it might be a good idea
> to maintain a WIP branch with submodules/subtrees for trace-cmd in the right version
> in trace-cruncher repo, and the series applied, so we don't have to jugle with the
> ML lore to get mbox plus other patches from the repo itself etc. Once we have that,
> we can easily include a script that calls all the build systems involved and install all the
> required bits in a venv, without polluting system-wide locations, and progressively
> turn that into a standalone setup.py (or the nearest approximation of that we
> manage to make).

In v2 I am trying to address this, partially by automating the patching 
and build of trace-cmd and partially by making all those patched 
libraries part of the package itself (exploiting some linker magic). 
However, your solution to use venv is probably by fare more common. It 
will be great if we can have some discussion here and decide which of 
the two solutions is the best.

Once again, thanks for helping us!
cheers,
Yordan

> 
> PS: apologies for the extra new lines and lack of reply markers,
> Office 365 webmail does not seem to play very nicely with plain text ...
> 
> Cheers,
> Douglas
> 
> From: Yordan Karadzhov (VMware) <[e-mail]>
> 
> Sent: 12 December 2019 09:02
> 
> To: [e-mail] <[e-mail]>
> 
> Cc: [e-mail] <[e-mail]>; Valentin Schneider <[e-mail]>; Douglas Raillard <[e-mail]>; Yordan Karadzhov (VMware) <[e-mail]>
> 
> Subject: [PATCH 0/5] Build trace-cruncher as Python pakage
> 
> 
> 
> 
> This patch-set is an attempt to restructure the project and to make it
> 
> build as a native Python package. Although it looks like a complete
> 
> rewrite, this is essentially just a switching from using Cython to using
> 
> directly the C API of Python. Cython is still being used but only for
> 
> the implementation of the NumPy data wrapper. The new package has its
> 
> own stand-alone build system (very primitive for the moment) that is
> 
> completely decoupled from the existing build system used by tracecruncher.
> 
> In order to build and install the new package do:
> 
> 
> 
> sudo python setup.py install --record files.txt
> 
> 
> 
> The patch-set does not  remove the old implementation yet. This will
> 
> happen in another successive patch-set.
> 
> 
> 
> Please review as careful as possible!
> 
> 
> 
> Yordan Karadzhov (VMware) (5):
> 
>    Refactor the part of the interface that relies on libkshark
> 
>    Refactor the part of the interface that relies on libtraceevent
> 
>    Refactor NumPy based data wrapper
> 
>    Add "utils"
> 
>    Adapt the sched_wakeup.py example script to use the new tracecruncher
> 
>      module
> 
> 
> 
>   examples/sched_wakeup.py  |  30 ++---
> 
>   setup.py                  |  61 +++++++++
> 
>   src/common.h              |  20 +++
> 
>   src/datawrapper.pyx       | 201 ++++++++++++++++++++++++++++
> 
>   src/ftracepy.c            | 234 +++++++++++++++++++++++++++++++++
> 
>   src/ksharkpy.c            | 268 ++++++++++++++++++++++++++++++++++++++
> 
>   src/trace2matrix.c        |  29 +++++
> 
>   tracecruncher/__init__.py |   0
> 
>   tracecruncher/utils.py    |  54 ++++++++
> 
>   9 files changed, 882 insertions(+), 15 deletions(-)
> 
>   create mode 100644 setup.py
> 
>   create mode 100644 src/common.h
> 
>   create mode 100644 src/datawrapper.pyx
> 
>   create mode 100644 src/ftracepy.c
> 
>   create mode 100644 src/ksharkpy.c
> 
>   create mode 100644 src/trace2matrix.c
> 
>   create mode 100644 tracecruncher/__init__.py
> 
>   create mode 100644 tracecruncher/utils.py
> 
> 
> 
> --
> 
> 2.20.1
> 
> 
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>