make settings accessable from the main thread only

KDAB · Nov 16, 2021 · f952582 · f952582
1 parent 1906fb4
commit f952582
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 58 deletions.
diff --git a/src/flamegraph.cpp b/src/flamegraph.cpp
@@ -396,32 +396,32 @@ FrameGraphicsItem* findItemBySymbol(const QList<QGraphicsItem*>& items, const Da
  */
 template<typename Tree>
 void toGraphicsItems(const Data::Costs& costs, int type, const QVector<Tree>& data, FrameGraphicsItem* parent,
-                     const double costThreshold, bool collapseRecursion)
+                     const double costThreshold, const Settings::ColorScheme colorScheme, bool collapseRecursion)
 {
     foreach (const auto& row, data) {
         if (collapseRecursion && !row.symbol.symbol.isEmpty() && row.symbol == parent->symbol()) {
             if (costs.cost(type, row.id) > costThreshold) {
-                toGraphicsItems(costs, type, row.children, parent, costThreshold, collapseRecursion);
+                toGraphicsItems(costs, type, row.children, parent, costThreshold, colorScheme, collapseRecursion);
             }
             continue;
         }
         auto item = findItemBySymbol(parent->childItems(), row.symbol);
         if (!item) {
             item = new FrameGraphicsItem(costs.cost(type, row.id), costs.unit(type), row.symbol, parent);
             item->setPen(parent->pen());
-            item->setBrush(brush(row.symbol, Settings::instance()->colorScheme()));
+            item->setBrush(brush(row.symbol, colorScheme));
         } else {
             item->setCost(item->cost() + costs.cost(type, row.id));
         }
         if (item->cost() > costThreshold) {
-            toGraphicsItems(costs, type, row.children, item, costThreshold, collapseRecursion);
+            toGraphicsItems(costs, type, row.children, item, costThreshold, colorScheme, collapseRecursion);
         }
     }
 }
 
 template<typename Tree>
 FrameGraphicsItem* parseData(const Data::Costs& costs, int type, const QVector<Tree>& topDownData, double costThreshold,
-                             bool collapseRecursion)
+                             const Settings::ColorScheme colorScheme, bool collapseRecursion)
 {
     const auto totalCost = costs.totalCost(type);
 
@@ -433,7 +433,7 @@ FrameGraphicsItem* parseData(const Data::Costs& costs, int type, const QVector<T
     rootItem->setBrush(scheme.background());
     rootItem->setPen(pen);
     toGraphicsItems(costs, type, topDownData, rootItem, static_cast<double>(totalCost) * costThreshold / 100.,
-                    collapseRecursion);
+                    colorScheme, collapseRecursion);
     return rootItem;
 }
 
@@ -816,16 +816,19 @@ void FlameGraph::showData()
     bool collapseRecursion = m_collapseRecursion;
     auto type = m_costSource->currentData().value<int>();
     auto threshold = m_costThreshold;
-    stream() << make_job([showBottomUpData, bottomUpData, topDownData, type, threshold, collapseRecursion, this]() {
-        FrameGraphicsItem* parsedData = nullptr;
-        if (showBottomUpData) {
-            parsedData = parseData(bottomUpData.costs, type, bottomUpData.root.children, threshold, collapseRecursion);
-        } else {
-            parsedData =
-                parseData(topDownData.inclusiveCosts, type, topDownData.root.children, threshold, collapseRecursion);
-        }
-        QMetaObject::invokeMethod(this, "setData", Qt::QueuedConnection, Q_ARG(FrameGraphicsItem*, parsedData));
-    });
+    const auto colorScheme = Settings::instance()->colorScheme();
+    stream() << make_job(
+        [showBottomUpData, bottomUpData, topDownData, type, threshold, colorScheme, collapseRecursion, this]() {
+            FrameGraphicsItem* parsedData = nullptr;
+            if (showBottomUpData) {
+                parsedData = parseData(bottomUpData.costs, type, bottomUpData.root.children, threshold, colorScheme,
+                                       collapseRecursion);
+            } else {
+                parsedData = parseData(topDownData.inclusiveCosts, type, topDownData.root.children, threshold,
+                                       colorScheme, collapseRecursion);
+            }
+            QMetaObject::invokeMethod(this, "setData", Qt::QueuedConnection, Q_ARG(FrameGraphicsItem*, parsedData));
+        });
     updateNavigationActions();
 }
 

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
@@ -374,9 +374,7 @@ void MainWindow::openFile(const QString& path, bool isReload)
     m_pageStack->setCurrentWidget(m_startPage);
 
     // TODO: support input files of different types via plugins
-    auto settings = Settings::instance();
-    m_parser->startParseFile(path, settings->sysroot(), settings->kallsyms(), settings->debugPaths(),
-                             settings->extraLibPaths(), settings->appPath(), settings->arch());
+    m_parser->startParseFile(path);
     m_reloadAction->setData(path);
     m_exportAction->setData(QUrl::fromLocalFile(file.absoluteFilePath() + QLatin1String(".perfparser")));
 

diff --git a/src/parsers/perf/perfparser.cpp b/src/parsers/perf/perfparser.cpp
@@ -1325,9 +1325,7 @@ PerfParser::PerfParser(QObject* parent)
 
 PerfParser::~PerfParser() = default;
 
-void PerfParser::startParseFile(const QString& path, const QString& sysroot, const QString& kallsyms,
-                                const QString& debugPaths, const QString& extraLibPaths, const QString& appPath,
-                                const QString& arch)
+void PerfParser::startParseFile(const QString& path)
 {
     Q_ASSERT(!m_isParsing);
 
@@ -1351,36 +1349,56 @@ void PerfParser::startParseFile(const QString& path, const QString& sysroot, con
         return;
     }
 
-    QStringList parserArgs = {QStringLiteral("--input"), decompressIfNeeded(path), QStringLiteral("--max-frames"),
-                              QStringLiteral("1024")};
-    if (!sysroot.isEmpty()) {
-        parserArgs += {QStringLiteral("--sysroot"), sysroot};
-    }
-    if (!kallsyms.isEmpty()) {
-        parserArgs += {QStringLiteral("--kallsyms"), kallsyms};
-    }
-    if (!debugPaths.isEmpty()) {
-        parserArgs += {QStringLiteral("--debug"), debugPaths};
-    }
-    if (!extraLibPaths.isEmpty()) {
-        parserArgs += {QStringLiteral("--extra"), extraLibPaths};
-    }
-    if (!appPath.isEmpty()) {
-        parserArgs += {QStringLiteral("--app"), appPath};
-    }
-    if (!arch.isEmpty()) {
-        parserArgs += {QStringLiteral("--arch"), arch};
-    }
+    auto parserArgs = [this](const QString& filename) {
+        const auto settings = Settings::instance();
+        QStringList parserArgs = {QStringLiteral("--input"), decompressIfNeeded(filename),
+                                  QStringLiteral("--max-frames"), QStringLiteral("1024")};
+        const auto sysroot = settings->sysroot();
+        if (!sysroot.isEmpty()) {
+            parserArgs += {QStringLiteral("--sysroot"), sysroot};
+        }
+        const auto kallsyms = settings->kallsyms();
+        if (!kallsyms.isEmpty()) {
+            parserArgs += {QStringLiteral("--kallsyms"), kallsyms};
+        }
+        const auto debugPaths = settings->debugPaths();
+        if (!debugPaths.isEmpty()) {
+            parserArgs += {QStringLiteral("--debug"), debugPaths};
+        }
+        const auto extraLibPaths = settings->extraLibPaths();
+        if (!extraLibPaths.isEmpty()) {
+            parserArgs += {QStringLiteral("--extra"), extraLibPaths};
+        }
+        const auto appPath = settings->appPath();
+        if (!appPath.isEmpty()) {
+            parserArgs += {QStringLiteral("--app"), appPath};
+        }
+        const auto arch = settings->arch();
+        if (!arch.isEmpty()) {
+            parserArgs += {QStringLiteral("--arch"), arch};
+        }
+        return parserArgs;
+    };
 
     // reset the data to ensure filtering will pick up the new data
-    m_parserArgs = parserArgs;
+    m_parserArgs = parserArgs(path);
     m_bottomUpResults = {};
     m_callerCalleeResults = {};
     m_events = {};
 
+    auto env = Util::appImageEnvironment();
+
+    const auto customUrls = Settings::instance()->debuginfodUrls();
+    if (!customUrls.isEmpty()) {
+        const auto envVar = QStringLiteral("DEBUGINFOD_URLS");
+        const auto defaultUrls = env.value(envVar);
+        const auto separator = QLatin1Char(' ');
+        env.insert(envVar, customUrls.join(separator) + separator + defaultUrls);
+    }
+
     emit parsingStarted();
     using namespace ThreadWeaver;
-    stream() << make_job([path, parserBinary, parserArgs, this]() {
+    stream() << make_job([path, parserBinary, parserArgs, env, this]() {
         PerfParserPrivate d;
         connect(&d, &PerfParserPrivate::progress, this, &PerfParser::progress);
         connect(this, &PerfParser::stopRequested, &d, &PerfParserPrivate::stop);
@@ -1418,15 +1436,6 @@ void PerfParser::startParseFile(const QString& path, const QString& sysroot, con
         }
 
         QProcess process;
-        auto env = Util::appImageEnvironment();
-
-        const auto customUrls = Settings::instance()->debuginfodUrls();
-        if (!customUrls.isEmpty()) {
-            const auto envVar = QStringLiteral("DEBUGINFOD_URLS");
-            const auto defaultUrls = env.value(envVar);
-            const auto separator = QLatin1Char(' ');
-            env.insert(envVar, customUrls.join(separator) + separator + defaultUrls);
-        }
 
         process.setProcessEnvironment(env);
         process.setProcessChannelMode(QProcess::ForwardedErrorChannel);
@@ -1495,7 +1504,7 @@ void PerfParser::startParseFile(const QString& path, const QString& sysroot, con
             emit parsingFailed(process.errorString());
         });
 
-        process.start(parserBinary, parserArgs);
+        process.start(parserBinary, m_parserArgs);
         if (!process.waitForStarted()) {
             emit parsingFailed(tr("Failed to start the hotspot-perfparser process"));
             return;

diff --git a/src/parsers/perf/perfparser.h b/src/parsers/perf/perfparser.h
@@ -44,8 +44,7 @@ class PerfParser : public QObject
     explicit PerfParser(QObject* parent = nullptr);
     ~PerfParser();
 
-    void startParseFile(const QString& path, const QString& sysroot, const QString& kallsyms, const QString& debugPaths,
-                        const QString& extraLibPaths, const QString& appPath, const QString& arch);
+    void startParseFile(const QString& path);
 
     void filterResults(const Data::FilterAction& filter);
 

diff --git a/src/settings.cpp b/src/settings.cpp
@@ -19,12 +19,15 @@
   along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+#include <QCoreApplication>
 #include <QDir>
+#include <QThread>
 
 #include "settings.h"
 
 Settings* Settings::instance()
 {
+    Q_ASSERT(QThread::currentThread() == QCoreApplication::instance()->thread());
     static Settings settings;
     return &settings;
 }

diff --git a/tests/integrationtests/dump_perf_data.cpp b/tests/integrationtests/dump_perf_data.cpp
@@ -50,7 +50,7 @@ int main(int argc, char** argv)
     int runningParsers = 0;
     for (const auto& arg : args) {
         auto parser = new PerfParser(&app);
-        parser->startParseFile(arg, {}, {}, {}, {}, {}, {});
+        parser->startParseFile(arg);
         ++runningParsers;
         QObject::connect(parser, &PerfParser::parsingFinished, parser, [&runningParsers, &app]() {
             --runningParsers;

diff --git a/tests/integrationtests/tst_perfparser.cpp b/tests/integrationtests/tst_perfparser.cpp
@@ -718,7 +718,7 @@ private slots:
         QSignalSpy callerCalleeDataSpy(&parser, &PerfParser::callerCalleeDataAvailable);
         QSignalSpy eventsDataSpy(&parser, &PerfParser::eventsAvailable);
 
-        parser.startParseFile(fileName, "", "", "", "", "", "");
+        parser.startParseFile(fileName);
 
         VERIFY_OR_THROW(parsingFinishedSpy.wait(6000));