Cleanup code and build graphics items in the background thread.

This prevents ~1s of GUI freezes on common data files.

diff --git a/gui/flamegraph.cpp b/gui/flamegraph.cpp
index 7f22f4bbc44097d70278ba7a751cd23bfd56b3ab..c41387300302a1bc2b557975cb10d462cf625e27 100644 (file)
--- a/gui/flamegraph.cpp
+++ b/gui/flamegraph.cpp
@@ -36,75 +36,88 @@
 #include <KLocalizedString>
 #include <KColorScheme>
 
-class FrameGraphicsItem : public QGraphicsRectItem
-{
-public:
-    FrameGraphicsItem(const QRectF& rect, const quint64 cost, const QString& function, FrameGraphicsItem* parent = nullptr)
-        : QGraphicsRectItem(rect, parent)
-    {
-        static const QString emptyLabel = QStringLiteral("???");
-
-        m_label = i18nc("%1: number of allocations, %2: function label",
-                        "%2: %1",
-                        cost,
-                        function.isEmpty() ? emptyLabel : function);
-        setToolTip(m_label);
-    }
+FrameGraphicsItem::FrameGraphicsItem(const QRectF& rect, const quint64 cost, const QString& function, FrameGraphicsItem* parent)
+    : QGraphicsRectItem(rect, parent)
+{
+    static const QString emptyLabel = QStringLiteral("???");
 
-    static const QFont font()
-    {
-        static const QFont font(QStringLiteral("monospace"), 10);
-        return font;
-    }
+    m_label = i18nc("%1: number of allocations, %2: function label",
+                    "%2: %1",
+                    cost,
+                    function.isEmpty() ? emptyLabel : function);
+    setToolTip(m_label);
+}
 
-    static const QFontMetrics fontMetrics()
-    {
-        static const QFontMetrics metrics(font());
-        return metrics;
-    }
+QFont FrameGraphicsItem::font()
+{
+    static const QFont font(QStringLiteral("monospace"), 10);
+    return font;
+}
 
-    static int margin()
-    {
-        return 5;
-    }
+QFontMetrics FrameGraphicsItem::fontMetrics()
+{
+    static const QFontMetrics metrics(font());
+    return metrics;
+}
+
+int FrameGraphicsItem::margin()
+{
+    return 5;
+}
 
-    int preferredWidth() const
-    {
-        return fontMetrics().width(m_label) + 2 * margin();
+int FrameGraphicsItem::preferredWidth() const
+{
+    return fontMetrics().width(m_label) + 2 * margin();
+}
+
+void FrameGraphicsItem::paint(QPainter* painter, const QStyleOptionGraphicsItem* option, QWidget* widget)
+{
+    const int width = rect().width() - 2 * margin();
+    if (width < 2) {
+        // don't try to paint tiny items
+        return;
     }
 
-    virtual void paint(QPainter* painter, const QStyleOptionGraphicsItem* option, QWidget* widget = 0)
-    {
-        const int width = rect().width() - 2 * margin();
-        if (width < 2) {
-            // don't try to paint tiny items
-            return;
-        }
+    QGraphicsRectItem::paint(painter, option, widget);
+
+    // TODO: text should always be displayed in a constant size and not zoomed
+    // TODO: items should only be scaled horizontally, not vertically
+    // TODO: items should "fit" into the view width
+    if (width < fontMetrics().averageCharWidth() * 6) {
+        // text is too wide for the current LOD, don't paint it
+        return;
+    }
 
-        QGraphicsRectItem::paint(painter, option, widget);
+    const int height = rect().height();
 
-        // TODO: text should always be displayed in a constant size and not zoomed
-        // TODO: items should only be scaled horizontally, not vertically
-        // TODO: items should "fit" into the view width
-        if (width < fontMetrics().averageCharWidth() * 6) {
-            // text is too wide for the current LOD, don't paint it
-            return;
-        }
+    const QFont oldFont = painter->font();
+    painter->setFont(font());
+    painter->drawText(margin() + rect().x(), rect().y(), width, height, Qt::AlignVCenter | Qt::AlignLeft | Qt::TextSingleLine,
+                        fontMetrics().elidedText(m_label, Qt::ElideRight, width));
+    painter->setFont(oldFont);
+}
 
-        const int height = rect().height();
+namespace {
 
-        const QFont oldFont = painter->font();
-        painter->setFont(font());
-        painter->drawText(margin() + rect().x(), rect().y(), width, height, Qt::AlignVCenter | Qt::AlignLeft | Qt::TextSingleLine,
-                          fontMetrics().elidedText(m_label, Qt::ElideRight, width));
-        painter->setFont(oldFont);
+void scaleItems(FrameGraphicsItem *item, qreal scaleFactor)
+{
+    auto rect = item->rect();
+    rect.moveLeft(rect.left() * scaleFactor);
+    rect.setWidth(rect.width() * scaleFactor);
+    item->setRect(rect);
+    foreach (auto child, item->childItems()) {
+        if (auto frameChild = dynamic_cast<FrameGraphicsItem*>(child)) {
+            scaleItems(frameChild, scaleFactor);
+        }
     }
+}
 
-private:
-    QString m_label;
+struct Frame {
+    quint64 cost = 0;
+    using Stack = QMap<QString, Frame>;
+    Stack children;
 };
-
-namespace {
+using Stack = Frame::Stack;
 
 QColor color(quint64 cost, quint64 maxCost)
 {
@@ -113,12 +126,10 @@ QColor color(quint64 cost, quint64 maxCost)
 }
 
 const qreal h = 25.;
-const qreal x_margin = 0.;
 const qreal y_margin = 2.;
 const qreal minRootWidth = 800.;
 
-// TODO: what is the right value for maxWidth here?
-void toGraphicsItems(const FlameGraphData::Stack& data, qreal totalCostForColor,
+void toGraphicsItems(const Stack& data, qreal totalCostForColor,
                      qreal parentCost, FrameGraphicsItem *parent)
 {
     auto pos = parent->rect().topLeft();
@@ -133,19 +144,43 @@ void toGraphicsItems(const FlameGraphData::Stack& data, qreal totalCostForColor,
         item->setPen(parent->pen());
         item->setBrush(color(it.value().cost, totalCostForColor));
         toGraphicsItems(it.value().children, totalCostForColor, it.value().cost, item);
-        x += w + x_margin;
+        x += w;
     }
 }
 
-void scaleItems(FrameGraphicsItem *item, qreal scaleFactor)
+FrameGraphicsItem* buildGraphicsItems(const Stack& stack)
 {
-    auto rect = item->rect();
-    rect.moveLeft(rect.left() * scaleFactor);
-    rect.setWidth(rect.width() * scaleFactor);
-    item->setRect(rect);
-    foreach (auto child, item->childItems()) {
-        if (auto frameChild = dynamic_cast<FrameGraphicsItem*>(child)) {
-            scaleItems(frameChild, scaleFactor);
+    double totalCost = 0;
+    foreach(const auto& frame, stack) {
+        totalCost += frame.cost;
+    }
+
+    const QPen pen(KColorScheme(QPalette::Active).foreground().color());
+
+    auto rootItem = new FrameGraphicsItem(QRectF(0, 0, minRootWidth, h), totalCost, i18n("total allocations"));
+    rootItem->setPen(pen);
+    toGraphicsItems(stack, totalCost, totalCost, rootItem);
+    return rootItem;
+}
+
+static void buildFlameGraph(const QVector<RowData>& mergedAllocations, Stack* topStack)
+{
+    foreach (const auto& row, mergedAllocations) {
+        if (row.children.isEmpty()) {
+            // leaf node found, bubble up the parent chain to build a top-down tree
+            auto node = &row;
+            auto stack = topStack;
+            while (node) {
+                auto& data = (*stack)[node->location.function];
+                // always use the leaf node's cost and propagate that one up the chain
+                // otherwise we'd count the cost of some nodes multiple times
+                data.cost += row.allocations;
+                stack = &data.children;
+                node = node->parent;
+            }
+        } else {
+            // recurse to find a leaf
+            buildFlameGraph(row.children, topStack);
         }
     }
 }
@@ -158,8 +193,6 @@ FlameGraph::FlameGraph(QWidget* parent, Qt::WindowFlags flags)
     , m_view(new QGraphicsView(this))
     , m_rootItem(nullptr)
 {
-    qRegisterMetaType<FlameGraphData>();
-
     setLayout(new QVBoxLayout);
 
     m_view->setScene(m_scene);
@@ -207,25 +240,16 @@ bool FlameGraph::eventFilter(QObject* object, QEvent* event)
     return QObject::eventFilter(object, event);
 }
 
-void FlameGraph::setData(const FlameGraphData& data)
+void FlameGraph::setData(FrameGraphicsItem* rootItem)
 {
-    m_data = data;
     m_scene->clear();
+    m_rootItem = rootItem;
+    m_scene->addItem(rootItem);
+}
 
-    qDebug() << "Evaluating flame graph";
-    QElapsedTimer t; t.start();
-
-    double totalCost = 0;
-    foreach(const auto& frame, data.stack) {
-        totalCost += frame.cost;
-    }
-
-    const QPen pen(KColorScheme(QPalette::Active).foreground().color());
-
-    m_rootItem = new FrameGraphicsItem(QRectF(0, 0, minRootWidth, h), totalCost, i18n("total allocations"));
-    m_rootItem->setPen(pen);
-    toGraphicsItems(data.stack, totalCost, totalCost, m_rootItem);
-    m_scene->addItem(m_rootItem);
-
-    qDebug() << "took me: " << t.elapsed();
+FrameGraphicsItem * FlameGraph::parseData(const QVector<RowData>& data)
+{
+    Stack stack;
+    buildFlameGraph(data, &stack);
+    return buildGraphicsItems(stack);
 }

diff --git a/gui/flamegraph.h b/gui/flamegraph.h
index 787b227fff4a7c7ad11eedc108fd2982cea2ee0f..2b09358899538e06089df206cf940f4e16c698c8 100644 (file)
--- a/gui/flamegraph.h
+++ b/gui/flamegraph.h
@@ -21,42 +21,47 @@
 #define FLAMEGRAPH_H
 
 #include <QWidget>
-#include <QMap>
+#include <QGraphicsRectItem>
+#include "bottomupmodel.h"
 
 class QGraphicsScene;
 class QGraphicsView;
-class FrameGraphicsItem;
 
-struct FlameGraphData
+class FrameGraphicsItem : public QGraphicsRectItem
 {
-    struct Frame {
-        quint64 cost = 0;
-        using Stack = QMap<QString, Frame>;
-        Stack children;
-    };
-    using Stack = Frame::Stack;
-
-    Stack stack;
-};
+public:
+    FrameGraphicsItem(const QRectF& rect, const quint64 cost, const QString& function, FrameGraphicsItem* parent = nullptr);
+
+    static QFont font();
+    static QFontMetrics fontMetrics();
+
+    static int margin();
 
-Q_DECLARE_METATYPE(FlameGraphData);
+    int preferredWidth() const;
+
+    void paint(QPainter* painter, const QStyleOptionGraphicsItem* option, QWidget* widget = nullptr) override;
+
+private:
+    QString m_label;
+};
 
 class FlameGraph : public QWidget
 {
     Q_OBJECT
 public:
-    FlameGraph(QWidget* parent = 0, Qt::WindowFlags flags = 0);
+    FlameGraph(QWidget* parent = nullptr, Qt::WindowFlags flags = 0);
     ~FlameGraph();
 
-    void setData(const FlameGraphData& data);
+    void setData(FrameGraphicsItem* rootFrame);
+    // called from background thread
+    static FrameGraphicsItem* parseData(const QVector<RowData>& data);
 
 protected:
-    virtual bool eventFilter(QObject* object, QEvent* event);
+    bool eventFilter(QObject* object, QEvent* event) override;
 
 private:
     QGraphicsScene* m_scene;
     QGraphicsView* m_view;
-    FlameGraphData m_data;
     FrameGraphicsItem* m_rootItem;
 };
 

diff --git a/gui/parser.cpp b/gui/parser.cpp
index 7739c260373ef6eb31dce5a5861db633fb79d03e..4bc5288318234780fe93fae61c9c6a3b0d36de04 100644 (file)
--- a/gui/parser.cpp
+++ b/gui/parser.cpp
@@ -27,6 +27,7 @@
 #include <QDebug>
 
 #include "../accumulatedtracedata.h"
+#include "flamegraph.h"
 
 #include <vector>
 
@@ -183,28 +184,6 @@ Parser::Parser(QObject* parent)
 
 Parser::~Parser() = default;
 
-static void buildFrameGraph(const QVector<RowData>& mergedAllocations, FlameGraphData::Stack* topStack)
-{
-    foreach (const auto& row, mergedAllocations) {
-        if (row.children.isEmpty()) {
-            // leaf node found, bubble up the parent chain to build a top-down tree
-            auto node = &row;
-            auto stack = topStack;
-            while (node) {
-                auto& data = (*stack)[node->location.function];
-                // always use the leaf node's cost and propagate that one up the chain
-                // otherwise we'd count the cost of some nodes multiple times
-                data.cost += row.allocations;
-                stack = &data.children;
-                node = node->parent;
-            }
-        } else {
-            // recurse to find a leaf
-            buildFrameGraph(row.children, topStack);
-        }
-    }
-}
-
 void Parser::parse(const QString& path)
 {
     using namespace ThreadWeaver;
@@ -215,9 +194,7 @@ void Parser::parse(const QString& path)
         const auto mergedAllocations = mergeAllocations(data);
         emit bottomUpDataAvailable(mergedAllocations);
         emit chartDataAvailable(data.chartData);
-        FlameGraphData::Stack stack;
-        buildFrameGraph(mergedAllocations, &stack);
-        emit flameGraphDataAvailable({stack});
+        emit flameGraphDataAvailable(FlameGraph::parseData(mergedAllocations));
         emit finished();
     });
 }

diff --git a/gui/parser.h b/gui/parser.h
index 3bd4a1f8cb6a1e53ee5a2bd4388f3e7d600653eb..35f343dce50b41909227f0274b7e42d0693923e9 100644 (file)
--- a/gui/parser.h
+++ b/gui/parser.h
@@ -24,7 +24,8 @@
 
 #include "bottomupmodel.h"
 #include "chartmodel.h"
-#include "flamegraph.h"
+
+class FrameGraphicsItem;
 
 class Parser : public QObject
 {
@@ -40,7 +41,7 @@ signals:
     void summaryAvailable(const QString& summary);
     void bottomUpDataAvailable(const BottomUpData& data);
     void chartDataAvailable(const ChartData& data);
-    void flameGraphDataAvailable(const FlameGraphData& data);
+    void flameGraphDataAvailable(FrameGraphicsItem* rootItem);
     void finished();
 };