Differential D23114 Diff 63610 helpers/perf/ftrace_marker.cpp

Changeset View

Standalone View

View Options

helpers/perf/ftrace_marker.cpp

  • This file was added.
1​/********************************************************************
2​ KWin - the KDE window manager
3​ This file is part of the KDE project.
4​
5​Copyright 2019 Roman Gilg <subdiff@gmail.com>
6​
7​This program is free software; you can redistribute it and/or modify
8​it under the terms of the GNU General Public License as published by
9​the Free Software Foundation; either version 2 of the License, or
10​(at your option) any later version.
11​
12​This program is distributed in the hope that it will be useful,
13​but WITHOUT ANY WARRANTY; without even the implied warranty of
14​MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15​GNU General Public License for more details.
16​
17​You should have received a copy of the GNU General Public License
18​along with this program. If not, see <http://www.gnu.org/licenses/>.
19​*********************************************************************/
20​#include "ftrace_marker.h"
21​
22​#include "utils.h"
23​
24​#include <QDir>
25​#include <QFileInfo>
26​
27​namespace KWin
28​{
29​namespace Perf
30​{
31​
32​void writeFunctionEnabled(QFile *file, const QString &message)
33​{
34​ file->write(message.toLatin1());
35​ file->flush();
36​}
37​
38​void writeFunctionDisabled(QFile *file, const QString &message)
39​{
40​ Q_UNUSED(file)
41​ Q_UNUSED(message)
42​}
43​
44​void (*s_writeFunction)(QFile *file, const QString &message) = writeFunctionDisabled;
45​
46​
47​KWIN_SINGLETON_FACTORY(FtraceMarker)
48​
49​FtraceMarker::FtraceMarker(QObject *parent)
50​ : QObject(parent)
51​{
52​ if (qEnvironmentVariableIsSet("KWIN_PERF_FTRACE")) {
53​ qCDebug(KWIN_PERF) << "Ftrace marking initially enabled via environment variable";
54​ setEnabled(true);
55​ }
56​}
57​
58​bool FtraceMarker::setEnabled(bool enable)
59​{
60​ if ((bool)m_file == enable) {
61​ // no change
62​ return true;
63​ }
64​ if (enable) {
65​ if (!findFile()) {
66​ qCWarning(KWIN_PERF) << "Ftrace marking not available. Try reenabling after issue is solved.";
67​ return false;
68​ }
69​ s_writeFunction = writeFunctionEnabled;
70​ } else {
71​ s_writeFunction = writeFunctionDisabled;
72​ delete m_file;
73​ m_file = nullptr;
74​ }
75​ return true;
76​}
77​
78​void FtraceMarker::print(const QString &message)
79​{
80​ (*s_writeFunction)(m_file, message);
81​}
82​
83​void FtraceMarker::print(ulong ctx, const QString &message)
84​{
85​ (*s_writeFunction)(m_file, message + QStringLiteral(" (begin_ctx=%1)").arg(ctx));
davidedmundsonUnsubmitted
Not Done

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

davidedmundson: This all seems overly complex. the presence of the file, and the function pointers are…
romanggAuthorUnsubmitted
Not Done

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.

romangg: My idea with this was that if we distribute ftrace marker calls over the code base I don't want…
86​}
87​
88​void FtraceMarker::print(const QString &message, ulong ctx)
89​{
90​ (*s_writeFunction)(m_file, message + QStringLiteral(" (end_ctx=%1)").arg(ctx));
91​}
92​
93​bool FtraceMarker::findFile()
94​{
95​ QFile mountsFile("/proc/mounts");
96​ if (!mountsFile.open(QIODevice::ReadOnly | QIODevice::Text)) {
97​ qCWarning(KWIN_PERF) << "No acces to mounts file. Can not determine trace marker file location.";
98​ return false;
99​ }
100​
101​ auto setInfo = [](const QString &line) {
102​ const int start = line.indexOf(' ') + 1;
103​ const int end = line.indexOf(' ', start);
104​ return QFileInfo(QDir(line.mid(start, end - start)),
davidedmundsonUnsubmitted
Done

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

davidedmundson: If we ever got a line we can't pass properly we would get an empty string here. QDir with an…
105​ QString::fromLatin1("trace_marker"));
106​ };
107​ QFileInfo markerFileInfo;
108​ QTextStream mountsIn(&mountsFile);
109​ QString mountsLine = mountsIn.readLine();
110​
111​ while (!mountsLine.isNull()) {
112​ if (mountsLine.startsWith("tracefs")) {
113​ markerFileInfo = setInfo(mountsLine);
114​ break;
115​ }
116​ if (mountsLine.startsWith("debugfs")) {
117​ markerFileInfo = setInfo(mountsLine);
118​ }
119​ mountsLine = mountsIn.readLine();
120​ }
121​ mountsFile.close();
122​
123​ const QString path = markerFileInfo.absoluteFilePath();
124​ m_file = new QFile(path, this);
125​ if (!m_file->open(QIODevice::WriteOnly)) {
126​ qCWarning(KWIN_PERF) << "No access to trace marker file at:" << path;
127​ delete m_file;
128​ m_file = nullptr;
129​ return false;
130​ }
131​ return true;
132​}
133​
134​}
135​}