[perf] Introduce ftrace marker
Needs ReviewPublic

Authored by romangg on Aug 12 2019, 3:08 PM.

Details

Reviewers
None
Group Reviewers
KWin
Summary

Adds a class for ftrace marking program flow of KWin. This used in conjunction
with a tool like GPUvis can be very powerful.

I use it at the moment for analyzing the compositing behavior of KWin on X11.
For this I also apply a patch to the XServer adding the same capability[1].

Besides the class also some marking points have been added. Ftracing can be,
if it has been built into the binary, activated via D-Bus or per env var
KWIN_PERF_FTRACE directly from the start.

[1] https://gitlab.freedesktop.org/xorg/xserver/merge_requests/209


Example ftrace in GPUvis

Diff Detail

Repository
R108 KWin
Branch
ftraceMarker
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 15025
Build 15043: arc lint + arc unit
romangg created this revision.Aug 12 2019, 3:08 PM
Restricted Application added a project: KWin. · View Herald TranscriptAug 12 2019, 3:08 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Aug 12 2019, 3:08 PM
romangg edited the summary of this revision. (Show Details)Aug 12 2019, 3:12 PM
romangg edited the summary of this revision. (Show Details)Aug 12 2019, 3:17 PM
zzag added a subscriber: zzag.Aug 12 2019, 8:44 PM

I think you could just add FtraceMarker class to KWin core and use it directly.

CMakeLists.txt
302

Not sure whether it has to be on by default. This is kind of development thing.

composite.cpp
28

Wrap it in #ifdef KWIN_BUILD_PERF

apol added a subscriber: apol.Aug 14 2019, 10:42 PM

It would be really cool to have a blog post on how to use this once it's in. 👍

CMakeLists.txt
302

If disabled is not a big penalty. I'd say it's good that it's on, in case you need to test an already existing binary.

dbusinterface.cpp
203

Add a warning?

I'm looking forward to using this in other projects potentially.
Seems a good tool to make use of.

composite.cpp
28

it's fine here.

perf is the public API wrapper around ftracemarker

helpers/perf/ftrace_marker.cpp
85

This all seems overly complex.

the presence of the file, and the function pointers are denoting the same thing

This code is basically just

void FtraceMarker::print()
{
   if (!m_file) return;
   m_file->write(message.toLatin1());
   m_file->flush();
}

void FtraceMarker::print(ulong ctx, const QString &message)
{
   print(message+ QStringLiteral(" (begin_ctx%").arg(ctx);
}

void FtraceMarker::print(const QString &message, ulong ctx )
{
   print(message+ QStringLiteral(" (end_ctx%").arg(ctx);
}

and you can strip half this class

104

If we ever got a line we can't pass properly we would get an empty string here.
QDir with an empty string returns the PWD, which would be a legitmate path you can write into filling up a user homedir

helpers/perf/ftrace_marker.h
42–43

Using overloads to denote different actions (begin/end) is a bit unusual...especially if a future person wants to add a real overload

org.kde.KWin.xml
51

FYI (if you develop something not just for dev purposes) a DBus method implicitly returns whether it succeeded or failed.

You can have a success reply, or an error reply which contains the reason why a call failed in a codified string and human readable form.