[perf] Introduce ftrace marker
Needs ReviewPublic

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


Group Reviewers

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

R108 KWin
Lint OK
No Unit Test Coverage
Build Status
Buildable 18785
Build 18803: 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.


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


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. πŸ‘


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.


Add a warning?

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


it's fine here.

perf is the public API wrapper around ftracemarker


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;

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


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


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


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.

romangg updated this revision to Diff 69729.Nov 14 2019, 9:14 AM

Rebase on master.

romangg updated this revision to Diff 69732.Nov 14 2019, 11:00 AM
romangg marked 3 inline comments as done.
  • Use DBus error message
  • No overloaded API functions
  • Check on empty dir name
romangg added inline comments.Nov 14 2019, 11:01 AM

I used what David proposed (DBus error message).


My idea with this was that if we distribute ftrace marker calls over the code base I don't want to have an additional if conditional checking at every place if tracing is active or not but instead set function pointers that simply return in case tracing is off.

I assumed such a function call would cost less than evaluating the if-conditional. But my research later showed that this does not need to be the case.

And I know it's kind of mini optimizing anyway, so I guess we can simplify the code anyway.

zzag added a comment.Nov 18 2019, 8:01 AM

This still looks slightly over-engineered. You could put everything in one class and add a few convenience methods to make it a bit easier to use the new class.


I don't think that directory helpers/ is the right place for the FtraceMarker class since it's not an internal utility. Directory helpers/ contains libexec stuff.

apol added a comment.Apr 2 2020, 11:22 PM

Don't we want this anymore? :/ it looked promising