From 77496df42a23d1f76b0da8ee9c630eec25f2cac0 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Mon, 18 Nov 2024 13:42:09 +0100 Subject: [PATCH 01/13] feat: remove tracepoints from timeaxisheaderview The tracepoints are getting a better visual representation on the timelinewidget so this is no longer necessary. --- src/models/timeaxisheaderview.cpp | 49 ------------------------------- src/models/timeaxisheaderview.h | 3 -- src/timelinewidget.cpp | 3 -- 3 files changed, 55 deletions(-) diff --git a/src/models/timeaxisheaderview.cpp b/src/models/timeaxisheaderview.cpp index 576032fe..4d8f245f 100644 --- a/src/models/timeaxisheaderview.cpp +++ b/src/models/timeaxisheaderview.cpp @@ -61,42 +61,6 @@ void TimeAxisHeaderView::emitHeaderDataChanged() headerDataChanged(this->orientation(), EventModel::EventsColumn, EventModel::EventsColumn); } -bool TimeAxisHeaderView::event(QEvent* event) -{ - if (event->type() == QEvent::ToolTip) { - auto helpEvent = static_cast(event); - - auto zoomTime = m_filterAndZoomStack->zoom().time; - if (!zoomTime.isValid()) - zoomTime = m_timeRange; // full - - const auto xForTime = xForTimeFactory(m_timeRange, zoomTime, sectionSize(EventModel::EventsColumn), - sectionPosition(EventModel::EventsColumn)); - - const auto oneNanoSecond = 1e-9; - for (const auto& tracepoint : std::as_const(m_tracepoints.tracepoints)) { - if (zoomTime.contains(tracepoint.time)) { - if (helpEvent->pos().x() == xForTime((tracepoint.time - m_timeRange.start) * oneNanoSecond)) { - QToolTip::showText(helpEvent->globalPos(), tracepoint.name, this); - return true; - } - } - } - - QToolTip::hideText(); - event->ignore(); - - return true; - } - return QHeaderView::event(event); -} - -void TimeAxisHeaderView::setTracepoints(const Data::TracepointResults& tracepoints) -{ - m_tracepoints = tracepoints; - update(); -} - void TimeAxisHeaderView::paintSection(QPainter* painter, const QRect& rect, int logicalIndex) const { if (painter == nullptr) @@ -134,19 +98,6 @@ void TimeAxisHeaderView::paintSection(QPainter* painter, const QRect& rect, int const QColor tickColor = palette().windowText().color(); const QColor prefixedColor = palette().highlight().color(); - if (!m_tracepoints.tracepoints.isEmpty()) { - const auto scheme = KColorScheme(palette().currentColorGroup()); - const auto tracepointPen = QPen(scheme.foreground(KColorScheme::LinkText), 1); - painter->setPen(tracepointPen); - - for (const auto& tracepoint : m_tracepoints.tracepoints) { - if (!zoomTime.contains(tracepoint.time)) - continue; - const auto x = xForTime((tracepoint.time - m_timeRange.start) * oneNanoSecond); - painter->drawLine(x, rect.height() / 2, x, rect.height()); - } - } - // Draw the long prefix tick and its label if (pfl.hasPrefix()) { diff --git a/src/models/timeaxisheaderview.h b/src/models/timeaxisheaderview.h index 4cc5e238..fe3a6857 100644 --- a/src/models/timeaxisheaderview.h +++ b/src/models/timeaxisheaderview.h @@ -24,15 +24,12 @@ class TimeAxisHeaderView : public QHeaderView public: void setTimeRange(Data::TimeRange timeRange); - void setTracepoints(const Data::TracepointResults& tracepoints); protected slots: void emitHeaderDataChanged(); - bool event(QEvent* event) override; private: Data::TimeRange m_timeRange; - Data::TracepointResults m_tracepoints; const FilterAndZoomStack* m_filterAndZoomStack = nullptr; protected: diff --git a/src/timelinewidget.cpp b/src/timelinewidget.cpp index 3c2e124b..e3ed1691 100644 --- a/src/timelinewidget.cpp +++ b/src/timelinewidget.cpp @@ -106,9 +106,6 @@ TimeLineWidget::TimeLineWidget(PerfParser* parser, QMenu* filterMenu, FilterAndZ connect(m_parser, &PerfParser::summaryDataAvailable, this, [eventModel](const Data::Summary& summary) { eventModel->setApplicationTime(summary.applicationTime); }); - connect(m_parser, &PerfParser::tracepointDataAvailable, this, - [this](const Data::TracepointResults& data) { m_timeAxisHeaderView->setTracepoints(data); }); - connect(ui->timeLineEventSource, static_cast(&QComboBox::currentIndexChanged), this, [this](int index) { const auto typeId = ui->timeLineEventSource->itemData(index).toInt(); From e3084ea5cae8653036a4dd1c7fec199e00da632a Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Wed, 20 Sep 2023 13:53:10 +0200 Subject: [PATCH 02/13] feat: show tracepoints in the timelinewidget Move the tracepoints from TimeAxisHeaderView to TimeLineWidget so that we can use the header for cpu usage. This also improves usability since the tracepoints are no longer bundles in one line. They now each have their own line. --- src/models/data.h | 34 ++-- src/models/eventmodel.cpp | 256 +++++++++++++++++++++++++----- src/models/timeaxisheaderview.cpp | 1 - src/models/timelinedelegate.cpp | 4 +- src/parsers/perf/perfparser.cpp | 67 +++++--- src/parsers/perf/perfparser.h | 2 - 6 files changed, 284 insertions(+), 80 deletions(-) diff --git a/src/models/data.h b/src/models/data.h index 56ea1401..fdb7187a 100644 --- a/src/models/data.h +++ b/src/models/data.h @@ -952,36 +952,37 @@ struct ThreadNames QHash> names; }; +struct TracepointEvents +{ + QString name; + Events events; + bool operator==(const TracepointEvents& rhs) const + { + return std::tie(name, events) == std::tie(rhs.name, rhs.events); + } +}; + struct EventResults { QVector threads; QVector cpus; + QVector tracepoints; QVector> stacks; QVector totalCosts; qint32 offCpuTimeCostId = -1; qint32 lostEventCostId = -1; + qint32 tracepointEventCostId = -1; ThreadEvents* findThread(qint32 pid, qint32 tid); const ThreadEvents* findThread(qint32 pid, qint32 tid) const; bool operator==(const EventResults& rhs) const { - return std::tie(threads, cpus, stacks, totalCosts, offCpuTimeCostId) - == std::tie(rhs.threads, rhs.cpus, rhs.stacks, rhs.totalCosts, rhs.offCpuTimeCostId); + return std::tie(threads, cpus, tracepoints, stacks, totalCosts, offCpuTimeCostId) + == std::tie(rhs.threads, rhs.cpus, rhs.tracepoints, rhs.stacks, rhs.totalCosts, rhs.offCpuTimeCostId); } }; -struct Tracepoint -{ - quint64 time = 0; - QString name; -}; - -struct TracepointResults -{ - QVector tracepoints; -}; - struct FilterAction { TimeRange time; @@ -1094,11 +1095,8 @@ Q_DECLARE_TYPEINFO(Data::ThreadNames, Q_MOVABLE_TYPE); Q_DECLARE_METATYPE(Data::EventResults) Q_DECLARE_TYPEINFO(Data::EventResults, Q_MOVABLE_TYPE); -Q_DECLARE_METATYPE(Data::Tracepoint) -Q_DECLARE_TYPEINFO(Data::Tracepoint, Q_MOVABLE_TYPE); - -Q_DECLARE_METATYPE(Data::TracepointResults) -Q_DECLARE_TYPEINFO(Data::TracepointResults, Q_MOVABLE_TYPE); +Q_DECLARE_METATYPE(Data::TracepointEvents) +Q_DECLARE_TYPEINFO(Data::TracepointEvents, Q_MOVABLE_TYPE); Q_DECLARE_METATYPE(Data::TimeRange) Q_DECLARE_TYPEINFO(Data::TimeRange, Q_MOVABLE_TYPE); diff --git a/src/models/eventmodel.cpp b/src/models/eventmodel.cpp index c41c5479..39c26346 100644 --- a/src/models/eventmodel.cpp +++ b/src/models/eventmodel.cpp @@ -9,7 +9,6 @@ #include "../util.h" -#include #include namespace { @@ -22,9 +21,20 @@ enum class Tag : quint8 Overview = 2, Cpus = 3, Processes = 4, - Threads = 5 + Threads = 5, + Tracepoints = 6, }; +enum OverviewRow : quint8 +{ + CpuRow, + ProcessRow, + TracepointRow, +}; +constexpr auto numRows = TracepointRow + 1; + +constexpr auto LAST_TAG = Tag::Tracepoints; + const auto DATATAG_SHIFT = sizeof(Tag) * 8; const auto DATATAG_UNSHIFT = (sizeof(quintptr) - sizeof(Tag)) * 8; @@ -36,7 +46,7 @@ quintptr combineDataTag(Tag tag, quintptr data) Tag dataTag(quintptr internalId) { auto ret = (internalId << DATATAG_UNSHIFT) >> DATATAG_UNSHIFT; - if (ret > static_cast(Tag::Threads)) + if (ret > static_cast(LAST_TAG)) return Tag::Invalid; return static_cast(ret); } @@ -76,15 +86,23 @@ int EventModel::rowCount(const QModelIndex& parent) const case Tag::Invalid: case Tag::Cpus: case Tag::Threads: - break; + case Tag::Tracepoints: + return 0; case Tag::Processes: return m_processes.value(parent.row()).threads.size(); case Tag::Overview: - return (parent.row() == 0) ? m_data.cpus.size() : m_processes.size(); + switch (static_cast(parent.row())) { + case OverviewRow::CpuRow: + return m_data.cpus.size(); + case OverviewRow::ProcessRow: + return m_processes.size(); + case OverviewRow::TracepointRow: + return m_data.tracepoints.size(); + } + Q_UNREACHABLE(); case Tag::Root: - return 2; - }; - + return numRows; + } return 0; } @@ -138,17 +156,29 @@ QVariant EventModel::data(const QModelIndex& index, int role) const auto tag = dataTag(index); + Q_ASSERT(static_cast(tag) <= static_cast(LAST_TAG)); + if (tag == Tag::Invalid || tag == Tag::Root) { return {}; } else if (tag == Tag::Overview) { if (role == Qt::DisplayRole) { - return index.row() == 0 ? tr("CPUs") : tr("Processes"); + switch (static_cast(index.row())) { + case OverviewRow::CpuRow: + return tr("CPUs"); + case OverviewRow::ProcessRow: + return tr("Processes"); + case OverviewRow::TracepointRow: + return tr("Tracepoints"); + } } else if (role == Qt::ToolTipRole) { - if (index.row() == 0) { + switch (static_cast(index.row())) { + case OverviewRow::CpuRow: return tr("Event timelines for all CPUs. This shows you which, and how many CPUs where leveraged." "Note that this feature relies on perf data files recorded with --sample-cpu."); - } else { + case OverviewRow::ProcessRow: return tr("Event timelines for the individual threads and processes."); + case OverviewRow::TracepointRow: + return tr("Event timelines for tracepoints"); } } else if (role == SortRole) { return index.row(); @@ -205,47 +235,163 @@ QVariant EventModel::data(const QModelIndex& index, int role) const const Data::ThreadEvents* thread = nullptr; const Data::CpuEvents* cpu = nullptr; + const Data::TracepointEvents* tracepoint = nullptr; if (tag == Tag::Cpus) { cpu = &m_data.cpus[index.row()]; - } else { - Q_ASSERT(tag == Tag::Threads); + } else if (tag == Tag::Threads) { const auto process = m_processes.value(tagData(index.internalId())); const auto tid = process.threads.value(index.row()); thread = m_data.findThread(process.pid, tid); Q_ASSERT(thread); + } else if (tag == Tag::Tracepoints) { + tracepoint = &m_data.tracepoints[index.row()]; } if (role == ThreadStartRole) { - return thread ? thread->time.start : m_time.start; + switch (tag) { + case Tag::Threads: + return thread->time.start; + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Cpus: + case Tag::Processes: + case Tag::Tracepoints: + return m_time.start; + } } else if (role == ThreadEndRole) { - return thread ? thread->time.end : m_time.end; + switch (tag) { + case Tag::Threads: + return thread->time.end; + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Cpus: + case Tag::Processes: + case Tag::Tracepoints: + return m_time.end; + } } else if (role == ThreadNameRole) { - return thread ? thread->name : tr("CPU #%1").arg(cpu->cpuId); + switch (tag) { + case Tag::Threads: + return thread->name; + case Tag::Cpus: + return tr("CPU #%1").arg(cpu->cpuId); + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Processes: + case Tag::Tracepoints: + return {}; + } } else if (role == ThreadIdRole) { - return thread ? thread->tid : Data::INVALID_TID; + switch (tag) { + case Tag::Threads: + return thread->tid; + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Cpus: + case Tag::Processes: + case Tag::Tracepoints: + return Data::INVALID_TID; + } } else if (role == ProcessIdRole) { - return thread ? thread->pid : Data::INVALID_PID; + switch (tag) { + case Tag::Threads: + return thread->pid; + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Cpus: + case Tag::Processes: + case Tag::Tracepoints: + return Data::INVALID_PID; + } } else if (role == CpuIdRole) { - return cpu ? cpu->cpuId : Data::INVALID_CPU_ID; + switch (tag) { + case Tag::Cpus: + return cpu->cpuId; + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Processes: + case Tag::Threads: + case Tag::Tracepoints: + return Data::INVALID_CPU_ID; + } } else if (role == EventsRole) { - return QVariant::fromValue(thread ? thread->events : (cpu ? cpu->events : Data::Events())); + switch (tag) { + case Tag::Threads: + return QVariant::fromValue(thread->events); + case Tag::Cpus: + return QVariant::fromValue(cpu->events); + case Tag::Tracepoints: + return QVariant::fromValue(tracepoint->events); + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Processes: + return {}; + } } else if (role == SortRole) { - if (index.column() == ThreadColumn) - return thread ? thread->tid : cpu->cpuId; - else - return thread ? thread->events.size() : cpu->events.size(); + if (index.column() == ThreadColumn) { + switch (tag) { + case Tag::Threads: + return thread->tid; + case Tag::Cpus: + return cpu->cpuId; + case Tag::Tracepoints: + return tracepoint->name; + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Processes: + return {}; + } + } else { + switch (tag) { + case Tag::Threads: + return thread->events.size(); + case Tag::Cpus: + return cpu->events.size(); + case Tag::Tracepoints: + return tracepoint->events.size(); + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Processes: + return {}; + } + } } switch (static_cast(index.column())) { case ThreadColumn: if (role == Qt::DisplayRole) { - return cpu ? tr("CPU #%1").arg(cpu->cpuId) : tr("%1 (#%2)").arg(thread->name, QString::number(thread->tid)); + switch (tag) { + case Tag::Cpus: + return tr("CPU #%1").arg(cpu->cpuId); + case Tag::Threads: + return tr("%1 (#%2)").arg(thread->name, QString::number(thread->tid)); + case Tag::Tracepoints: + return tracepoint->name; + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Processes: + return {}; + } } else if (role == Qt::ToolTipRole) { - QString tooltip = cpu ? tr("CPU #%1\n").arg(cpu->cpuId) - : tr("Thread %1, tid = %2, pid = %3\n") - .arg(thread->name, QString::number(thread->tid), QString::number(thread->pid)); - if (thread) { + QString tooltip; + int numEvents = 0; + + switch (tag) { + case Tag::Threads: { + tooltip = tr("Thread %1, tid = %2, pid = %3\n") + .arg(thread->name, QString::number(thread->tid), QString::number(thread->pid)); + const auto runtime = thread->time.delta(); const auto totalRuntime = m_time.delta(); tooltip += tr("Runtime: %1 (%2% of total runtime)\n") @@ -260,16 +406,45 @@ QVariant EventModel::data(const QModelIndex& index, int role) const Util::formatCostRelative(thread->offCpuTime, runtime), Util::formatCostRelative(thread->offCpuTime, m_totalOffCpuTime)); } + numEvents = thread->events.size(); + break; } - const auto numEvents = thread ? thread->events.size() : cpu->events.size(); + case Tag::Cpus: + tooltip = tr("CPU #%1\n").arg(cpu->cpuId); + numEvents = cpu->events.size(); + break; + case Tag::Tracepoints: + tooltip = tracepoint->name; + numEvents = tracepoint->events.size(); + break; + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Processes: + return {}; + } + tooltip += tr("Number of Events: %1 (%2% of the total)") .arg(QString::number(numEvents), Util::formatCostRelative(numEvents, m_totalEvents)); return tooltip; } break; case EventsColumn: - if (role == Qt::DisplayRole) - return thread ? thread->events.size() : cpu->events.size(); + if (role == Qt::DisplayRole) { + switch (tag) { + case Tag::Threads: + return thread->events.size(); + case Tag::Cpus: + return cpu->events.size(); + case Tag::Tracepoints: + return tracepoint->events.size(); + case Tag::Invalid: + case Tag::Root: + case Tag::Overview: + case Tag::Processes: + return {}; + } + } break; case NUM_COLUMNS: // nothing @@ -322,6 +497,7 @@ void EventModel::setData(const Data::EventResults& data) [](const Data::CpuEvents& cpuEvents) { return cpuEvents.events.isEmpty(); }); m_data.cpus.erase(it, m_data.cpus.end()); } + endResetModel(); } @@ -344,15 +520,21 @@ QModelIndex EventModel::index(int row, int column, const QModelIndex& parent) co switch (dataTag(parent)) { case Tag::Invalid: // leaf / invalid -> no children case Tag::Cpus: + case Tag::Tracepoints: case Tag::Threads: break; case Tag::Root: // root has the 1st level children: Overview return createIndex(row, column, static_cast(Tag::Overview)); case Tag::Overview: // 2nd level children: Cpus and the Processes - if (parent.row() == 0) + switch (static_cast(parent.row())) { + case OverviewRow::CpuRow: return createIndex(row, column, static_cast(Tag::Cpus)); - else + case OverviewRow::ProcessRow: return createIndex(row, column, static_cast(Tag::Processes)); + case OverviewRow::TracepointRow: + return createIndex(row, column, static_cast(Tag::Tracepoints)); + } + Q_UNREACHABLE(); case Tag::Processes: // 3rd level children: Threads return createIndex(row, column, combineDataTag(Tag::Threads, parent.row())); } @@ -368,9 +550,11 @@ QModelIndex EventModel::parent(const QModelIndex& child) const case Tag::Overview: break; case Tag::Cpus: - return createIndex(0, 0, static_cast(Tag::Overview)); + return createIndex(OverviewRow::CpuRow, 0, static_cast(Tag::Overview)); case Tag::Processes: - return createIndex(1, 0, static_cast(Tag::Overview)); + return createIndex(OverviewRow::ProcessRow, 0, static_cast(Tag::Overview)); + case Tag::Tracepoints: + return createIndex(OverviewRow::TracepointRow, 0, static_cast(Tag::Overview)); case Tag::Threads: { const auto parentRow = tagData(child.internalId()); return createIndex(parentRow, 0, static_cast(Tag::Processes)); diff --git a/src/models/timeaxisheaderview.cpp b/src/models/timeaxisheaderview.cpp index 4d8f245f..6c6666a8 100644 --- a/src/models/timeaxisheaderview.cpp +++ b/src/models/timeaxisheaderview.cpp @@ -14,7 +14,6 @@ #include -#include "../util.h" #include "eventmodel.h" #include "filterandzoomstack.h" diff --git a/src/models/timelinedelegate.cpp b/src/models/timelinedelegate.cpp index ddb010a9..56871e87 100644 --- a/src/models/timelinedelegate.cpp +++ b/src/models/timelinedelegate.cpp @@ -160,6 +160,7 @@ void TimeLineDelegate::paint(QPainter* painter, const QStyleOptionViewItem& opti const auto results = index.data(EventModel::EventResultsRole).value(); const auto offCpuCostId = results.offCpuTimeCostId; const auto lostEventCostId = results.lostEventCostId; + const auto tracepointEventCostId = results.tracepointEventCostId; const bool is_alternate = option.features & QStyleOptionViewItem::Alternate; const auto& palette = option.palette; @@ -229,7 +230,8 @@ void TimeLineDelegate::paint(QPainter* painter, const QStyleOptionViewItem& opti // see also: https://www.spinics.net/lists/linux-perf-users/msg03486.html for (const auto& event : data.events) { const auto isLostEvent = event.type == lostEventCostId; - if (event.type != m_eventType && !isLostEvent) { + const auto isTracepointEvent = event.type == tracepointEventCostId; + if (event.type != m_eventType && !isLostEvent && !isTracepointEvent) { continue; } diff --git a/src/parsers/perf/perfparser.cpp b/src/parsers/perf/perfparser.cpp index c4b94daf..97fa4a61 100644 --- a/src/parsers/perf/perfparser.cpp +++ b/src/parsers/perf/perfparser.cpp @@ -554,7 +554,7 @@ QDataStream& operator>>(QDataStream& stream, TracePointFormat& format) return stream; } -QDebug operator<<(QDebug stream, TracePointFormat format) +QDebug operator<<(QDebug stream, const TracePointFormat& format) { stream.noquote().nospace() << "TracePointFormat{" << "systemId=" << format.systemId << ", " @@ -958,6 +958,28 @@ class PerfParserPrivate : public QObject buildPerLibraryResult(); buildCallerCalleeResult(); + eventResult.tracepoints.reserve(tracepoints.size()); + for (auto it = tracepoints.cbegin(), end = tracepoints.cend(); it != end; it++) { + eventResult.tracepoints.push_back({strings[it.key()], {it.value()}}); + } + + eventResult.tracePointData.reserve(tracepointData.size()); + std::transform(tracepointData.cbegin(), tracepointData.cend(), std::back_inserter(eventResult.tracePointData), + [this](const TracePointData& data) -> Data::TracePointData { + QHash tracepointData; + + for (auto it = data.data.cbegin(), end = data.data.cend(); it != end; it++) { + tracepointData[strings.value(it.key())] = it.value(); + } + + return tracepointData; + }); + + for (auto it = tracepointFormat.cbegin(), end = tracepointFormat.cend(); it != end; it++) { + eventResult.tracePointFormats[it.key()] = {strings.value(it->systemId.id), strings.value(it->nameId.id), + it->flags, strings.value(it->format.id)}; + } + for (auto& thread : eventResult.threads) { thread.time.start = std::max(thread.time.start, applicationTime.start); thread.time.end = std::min(thread.time.end, applicationTime.end); @@ -1179,12 +1201,14 @@ class PerfParserPrivate : public QObject const auto attribute = attributes.value(event.type); if (attribute.type == static_cast(AttributesDefinition::Type::Tracepoint)) { - Data::Tracepoint tracepoint; - tracepoint.time = event.time; - tracepoint.name = strings.value(attribute.name.id); - if (tracepoint.name != QLatin1String("sched:sched_switch")) { - // sched_switch events are handled separately already - tracepointResult.tracepoints.push_back(tracepoint); + if (eventResult.tracepointEventCostId == -1) { + eventResult.tracepointEventCostId = + addCostType(QStringLiteral("Tracepoint"), Data::Costs::Unit::Tracepoint); + } + + if (attribute.name.id != m_schedSwitchId) { + auto& tracepointList = tracepoints[attribute.name.id]; + tracepointList.push_back({event.time, 0, eventResult.tracepointEventCostId}); } } } @@ -1201,6 +1225,10 @@ class PerfParserPrivate : public QObject { Q_ASSERT(string.id == strings.size()); strings.push_back(QString::fromUtf8(string.string)); + + if (string.string == "sched:sched_switch") { + m_schedSwitchId = string.id; + } } void addSampleToBottomUp(const Sample& sample) @@ -1471,7 +1499,7 @@ class PerfParserPrivate : public QObject Data::CallerCalleeResults callerCalleeResult; Data::ByFileResults byFileResult; Data::EventResults eventResult; - Data::TracepointResults tracepointResult; + QHash tracepoints; Data::FrequencyResults frequencyResult; Data::ThreadNames commands; std::unique_ptr perfScriptOutput; @@ -1483,6 +1511,7 @@ class PerfParserPrivate : public QObject QHash attributeNameToCostIds; qint32 m_nextCostId = 0; qint32 m_schedSwitchCostId = -1; + qint32 m_schedSwitchId = -1; QHash m_lastSampleTimePerCore; Settings::CostAggregation costAggregation; bool perfMapFileExists = false; @@ -1519,7 +1548,7 @@ PerfParser::PerfParser(QObject* parent) qRegisterMetaType(); qRegisterMetaType(); qRegisterMetaType(); - qRegisterMetaType(); + qRegisterMetaType(); qRegisterMetaType(); qRegisterMetaType(); @@ -1549,11 +1578,6 @@ PerfParser::PerfParser(QObject* parent) m_events = data; } }); - connect(this, &PerfParser::tracepointDataAvailable, this, [this](const Data::TracepointResults& data) { - if (m_tracepointResults.tracepoints.isEmpty()) { - m_tracepointResults = data; - } - }); connect(this, &PerfParser::threadNamesAvailable, this, [this](const Data::ThreadNames& threadNames) { m_threadNames = threadNames; }); connect(this, &PerfParser::parsingStarted, this, [this]() { @@ -1672,7 +1696,6 @@ void PerfParser::startParseFile(const QString& path) m_bottomUpResults = {}; m_callerCalleeResults = {}; m_byFileResults = {}; - m_tracepointResults = {}; m_events = {}; m_frequencyResults = {}; @@ -1696,7 +1719,6 @@ void PerfParser::startParseFile(const QString& path) emit summaryDataAvailable(d.summaryResult); emit callerCalleeDataAvailable(d.callerCalleeResult); emit byFileDataAvailable(d.byFileResult); - emit tracepointDataAvailable(d.tracepointResult); emit eventsAvailable(d.eventResult); emit frequencyDataAvailable(d.frequencyResult); emit threadNamesAvailable(d.commands); @@ -1825,7 +1847,6 @@ void PerfParser::filterResults(const Data::FilterAction& filter) Data::EventResults events = m_events; Data::CallerCalleeResults callerCallee; Data::ByFileResults byFile; - Data::TracepointResults tracepointResults = m_tracepointResults; auto frequencyResults = m_frequencyResults; const bool filterByTime = filter.time.isValid(); const bool filterByCpu = filter.cpuId != std::numeric_limits::max(); @@ -1907,10 +1928,13 @@ void PerfParser::filterResults(const Data::FilterAction& filter) } if (filterByTime) { - auto it = std::remove_if( - tracepointResults.tracepoints.begin(), tracepointResults.tracepoints.end(), - [filter](const Data::Tracepoint& tracepoint) { return !filter.time.contains(tracepoint.time); }); - tracepointResults.tracepoints.erase(it, tracepointResults.tracepoints.end()); + // TODO: parallelize + for (auto& tracepoints : events.tracepoints) { + auto it = std::remove_if( + tracepoints.events.begin(), tracepoints.events.end(), + [filter](const Data::Event& event) { return !filter.time.contains(event.time); }); + tracepoints.events.erase(it, tracepoints.events.end()); + } for (auto& core : frequencyResults.cores) { for (auto& costType : core.costs) { @@ -2025,7 +2049,6 @@ void PerfParser::filterResults(const Data::FilterAction& filter) emit callerCalleeDataAvailable(callerCallee); emit byFileDataAvailable(byFile); emit frequencyDataAvailable(frequencyResults); - emit tracepointDataAvailable(tracepointResults); emit eventsAvailable(events); emit parsingFinished(); }); diff --git a/src/parsers/perf/perfparser.h b/src/parsers/perf/perfparser.h index 7133f49b..988bdb3e 100644 --- a/src/parsers/perf/perfparser.h +++ b/src/parsers/perf/perfparser.h @@ -60,7 +60,6 @@ class PerfParser : public QObject void perLibraryDataAvailable(const Data::PerLibraryResults& data); void callerCalleeDataAvailable(const Data::CallerCalleeResults& data); void byFileDataAvailable(const Data::ByFileResults& data); - void tracepointDataAvailable(const Data::TracepointResults& data); void frequencyDataAvailable(const Data::FrequencyResults& data); void eventsAvailable(const Data::EventResults& events); void threadNamesAvailable(const Data::ThreadNames& threadNames); @@ -87,7 +86,6 @@ class PerfParser : public QObject Data::BottomUpResults m_bottomUpResults; Data::CallerCalleeResults m_callerCalleeResults; Data::ByFileResults m_byFileResults; - Data::TracepointResults m_tracepointResults; Data::EventResults m_events; Data::FrequencyResults m_frequencyResults; std::atomic m_isParsing; From c3d0d9472c532f4f3d43854a578b46cfcf23fd29 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Thu, 21 Sep 2023 11:27:21 +0200 Subject: [PATCH 03/13] feat: simplify tag enum Enum automatically counts up so there is no need to manually set these values. --- src/models/eventmodel.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/models/eventmodel.cpp b/src/models/eventmodel.cpp index 39c26346..1f6387a1 100644 --- a/src/models/eventmodel.cpp +++ b/src/models/eventmodel.cpp @@ -17,12 +17,12 @@ constexpr auto orderProcessByPid = [](const EventModel::Process& process, qint32 enum class Tag : quint8 { Invalid = 0, - Root = 1, - Overview = 2, - Cpus = 3, - Processes = 4, - Threads = 5, - Tracepoints = 6, + Root, + Overview, + Cpus, + Processes, + Threads, + Tracepoints, }; enum OverviewRow : quint8 From d45cea4aaeeed9b7c5d228a82128568a79f0f0df Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Thu, 21 Sep 2023 11:32:58 +0200 Subject: [PATCH 04/13] feat: add Favourites to TimeLineWidget this allows the user to group important timelines together so that he can compare them better --- src/models/eventmodel.cpp | 102 ++++++++++++++++++++++- src/models/eventmodel.h | 6 ++ src/models/timelinedelegate.cpp | 15 ++++ src/models/timelinedelegate.h | 2 + src/timelinewidget.cpp | 5 ++ tests/modeltests/tst_models.cpp | 142 +++++++++++++++++++------------- 6 files changed, 213 insertions(+), 59 deletions(-) diff --git a/src/models/eventmodel.cpp b/src/models/eventmodel.cpp index 1f6387a1..9096c2d2 100644 --- a/src/models/eventmodel.cpp +++ b/src/models/eventmodel.cpp @@ -23,6 +23,7 @@ enum class Tag : quint8 Processes, Threads, Tracepoints, + Favorites, }; enum OverviewRow : quint8 @@ -30,10 +31,11 @@ enum OverviewRow : quint8 CpuRow, ProcessRow, TracepointRow, + FavoriteRow, }; -constexpr auto numRows = TracepointRow + 1; +constexpr auto numRows = FavoriteRow + 1; -constexpr auto LAST_TAG = Tag::Tracepoints; +constexpr auto LAST_TAG = Tag::Favorites; const auto DATATAG_SHIFT = sizeof(Tag) * 8; const auto DATATAG_UNSHIFT = (sizeof(quintptr) - sizeof(Tag)) * 8; @@ -87,6 +89,7 @@ int EventModel::rowCount(const QModelIndex& parent) const case Tag::Cpus: case Tag::Threads: case Tag::Tracepoints: + case Tag::Favorites: return 0; case Tag::Processes: return m_processes.value(parent.row()).threads.size(); @@ -98,6 +101,8 @@ int EventModel::rowCount(const QModelIndex& parent) const return m_processes.size(); case OverviewRow::TracepointRow: return m_data.tracepoints.size(); + case OverviewRow::FavoriteRow: + return m_favourites.size(); } Q_UNREACHABLE(); case Tag::Root: @@ -169,6 +174,8 @@ QVariant EventModel::data(const QModelIndex& index, int role) const return tr("Processes"); case OverviewRow::TracepointRow: return tr("Tracepoints"); + case OverviewRow::FavoriteRow: + return tr("Favorites"); } } else if (role == Qt::ToolTipRole) { switch (static_cast(index.row())) { @@ -179,6 +186,8 @@ QVariant EventModel::data(const QModelIndex& index, int role) const return tr("Event timelines for the individual threads and processes."); case OverviewRow::TracepointRow: return tr("Event timelines for tracepoints"); + case OverviewRow::FavoriteRow: + return tr("A list of favourites to group important events"); } } else if (role == SortRole) { return index.row(); @@ -246,6 +255,13 @@ QVariant EventModel::data(const QModelIndex& index, int role) const Q_ASSERT(thread); } else if (tag == Tag::Tracepoints) { tracepoint = &m_data.tracepoints[index.row()]; + } else if (tag == Tag::Favorites) { + if (role == IsFavoriteRole) { + return true; + } + + const auto& favourite = m_favourites[index.row()]; + return data(favourite.siblingAtColumn(index.column()), role); } if (role == ThreadStartRole) { @@ -259,6 +275,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Processes: case Tag::Tracepoints: return m_time.start; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } else if (role == ThreadEndRole) { switch (tag) { @@ -271,6 +290,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Processes: case Tag::Tracepoints: return m_time.end; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } else if (role == ThreadNameRole) { switch (tag) { @@ -284,6 +306,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Processes: case Tag::Tracepoints: return {}; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } else if (role == ThreadIdRole) { switch (tag) { @@ -296,6 +321,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Processes: case Tag::Tracepoints: return Data::INVALID_TID; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } else if (role == ProcessIdRole) { switch (tag) { @@ -308,6 +336,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Processes: case Tag::Tracepoints: return Data::INVALID_PID; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } else if (role == CpuIdRole) { switch (tag) { @@ -320,6 +351,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Threads: case Tag::Tracepoints: return Data::INVALID_CPU_ID; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } else if (role == EventsRole) { switch (tag) { @@ -334,6 +368,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Overview: case Tag::Processes: return {}; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } else if (role == SortRole) { if (index.column() == ThreadColumn) { @@ -349,6 +386,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Overview: case Tag::Processes: return {}; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } else { switch (tag) { @@ -363,8 +403,13 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Overview: case Tag::Processes: return {}; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } + } else if (role == IsFavoriteRole) { + return false; } switch (static_cast(index.column())) { @@ -382,6 +427,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Overview: case Tag::Processes: return {}; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } else if (role == Qt::ToolTipRole) { QString tooltip; @@ -422,6 +470,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Overview: case Tag::Processes: return {}; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } tooltip += tr("Number of Events: %1 (%2% of the total)") @@ -443,6 +494,9 @@ QVariant EventModel::data(const QModelIndex& index, int role) const case Tag::Overview: case Tag::Processes: return {}; + case Tag::Favorites: + // they are handled elsewhere + Q_UNREACHABLE(); } } break; @@ -457,6 +511,8 @@ QVariant EventModel::data(const QModelIndex& index, int role) const void EventModel::setData(const Data::EventResults& data) { beginResetModel(); + m_favourites.clear(); + m_data = data; m_totalEvents = 0; m_maxCost = 0; @@ -522,6 +578,7 @@ QModelIndex EventModel::index(int row, int column, const QModelIndex& parent) co case Tag::Cpus: case Tag::Tracepoints: case Tag::Threads: + case Tag::Favorites: break; case Tag::Root: // root has the 1st level children: Overview return createIndex(row, column, static_cast(Tag::Overview)); @@ -533,6 +590,8 @@ QModelIndex EventModel::index(int row, int column, const QModelIndex& parent) co return createIndex(row, column, static_cast(Tag::Processes)); case OverviewRow::TracepointRow: return createIndex(row, column, static_cast(Tag::Tracepoints)); + case OverviewRow::FavoriteRow: + return createIndex(row, column, static_cast(Tag::Favorites)); } Q_UNREACHABLE(); case Tag::Processes: // 3rd level children: Threads @@ -554,7 +613,9 @@ QModelIndex EventModel::parent(const QModelIndex& child) const case Tag::Processes: return createIndex(OverviewRow::ProcessRow, 0, static_cast(Tag::Overview)); case Tag::Tracepoints: - return createIndex(OverviewRow::TracepointRow, 0, static_cast(Tag::Overview)); + return createIndex(OverviewRow::TracepointRow, 0, static_cast(Tag::Overview)); + case Tag::Favorites: + return createIndex(OverviewRow::FavoriteRow, 0, static_cast(Tag::Overview)); case Tag::Threads: { const auto parentRow = tagData(child.internalId()); return createIndex(parentRow, 0, static_cast(Tag::Processes)); @@ -563,3 +624,38 @@ QModelIndex EventModel::parent(const QModelIndex& child) const return {}; } + +void EventModel::addToFavorites(const QModelIndex& index) +{ + Q_ASSERT(index.model() == this); + + if (index.column() != 0) { + // we only want one index per row, so we force it to be column zero + // this way we can easily check if we have duplicate rows + addToFavorites(index.siblingAtColumn(0)); + return; + } + + if (m_favourites.contains(index)) { + return; + } + + const auto row = m_favourites.size(); + + beginInsertRows(createIndex(FavoriteRow, 0, static_cast(Tag::Overview)), row, row); + m_favourites.push_back(index); + endInsertRows(); +} + +void EventModel::removeFromFavorites(const QModelIndex& index) +{ + Q_ASSERT(index.model() == this); + Q_ASSERT(dataTag(index) == Tag::Favorites); + + const auto row = index.row(); + Q_ASSERT(row >= 0 && row < m_favourites.size()); + + beginRemoveRows(createIndex(FavoriteRow, 0, static_cast(Tag::Overview)), row, row); + m_favourites.remove(row); + endRemoveRows(); +} diff --git a/src/models/eventmodel.h b/src/models/eventmodel.h index 60f0ac78..6c0d782e 100644 --- a/src/models/eventmodel.h +++ b/src/models/eventmodel.h @@ -44,6 +44,7 @@ class EventModel : public QAbstractItemModel SortRole, TotalCostsRole, EventResultsRole, + IsFavoriteRole, }; int rowCount(const QModelIndex& parent = {}) const override; @@ -74,9 +75,14 @@ class EventModel : public QAbstractItemModel QString name; }; +public: + void addToFavorites(const QModelIndex& index); + void removeFromFavorites(const QModelIndex& index); + private: Data::EventResults m_data; QVector m_processes; + QVector m_favourites; Data::TimeRange m_time; Data::TimeRange m_applicationTime; quint64 m_totalOnCpuTime = 0; diff --git a/src/models/timelinedelegate.cpp b/src/models/timelinedelegate.cpp index 56871e87..fa89f61f 100644 --- a/src/models/timelinedelegate.cpp +++ b/src/models/timelinedelegate.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "../util.h" @@ -463,6 +464,20 @@ bool TimeLineDelegate::eventFilter(QObject* watched, QEvent* event) const auto isMainThread = threadStartTime == minTime && threadEndTime == maxTime; const auto cpuId = index.data(EventModel::CpuIdRole).value(); const auto numCpus = index.data(EventModel::NumCpusRole).value(); + const auto isFavorite = index.data(EventModel::IsFavoriteRole).value(); + + contextMenu->addAction(QIcon::fromTheme(QStringLiteral("favorite")), + isFavorite ? tr("Remove from favorites") : tr("Add to favorites"), this, + [this, index, isFavorite] { + auto model = qobject_cast(index.model()); + Q_ASSERT(model); + if (isFavorite) { + emit removeFromFavorites(model->mapToSource(index)); + } else { + emit addToFavorites(model->mapToSource(index)); + } + }); + if (isTimeSpanSelected && (minTime != timeSlice.start || maxTime != timeSlice.end)) { contextMenu->addAction(QIcon::fromTheme(QStringLiteral("zoom-in")), tr("Zoom In On Selection"), this, [this, timeSlice]() { m_filterAndZoomStack->zoomIn(timeSlice); }); diff --git a/src/models/timelinedelegate.h b/src/models/timelinedelegate.h index 4d38a811..69759a0a 100644 --- a/src/models/timelinedelegate.h +++ b/src/models/timelinedelegate.h @@ -64,6 +64,8 @@ class TimeLineDelegate : public QStyledItemDelegate signals: void stacksHovered(const QSet& stacks); + void addToFavorites(const QModelIndex& index); + void removeFromFavorites(const QModelIndex& index); protected: bool eventFilter(QObject* watched, QEvent* event) override; diff --git a/src/timelinewidget.cpp b/src/timelinewidget.cpp index e3ed1691..6bfa5d84 100644 --- a/src/timelinewidget.cpp +++ b/src/timelinewidget.cpp @@ -112,6 +112,11 @@ TimeLineWidget::TimeLineWidget(PerfParser* parser, QMenu* filterMenu, FilterAndZ m_timeLineDelegate->setEventType(typeId); }); + connect(m_timeLineDelegate, &TimeLineDelegate::addToFavorites, this, + [eventModel](const QModelIndex& index) { eventModel->addToFavorites(index); }); + connect(m_timeLineDelegate, &TimeLineDelegate::removeFromFavorites, this, + [eventModel](const QModelIndex& index) { eventModel->removeFromFavorites(index); }); + connect(m_timeLineDelegate, &TimeLineDelegate::stacksHovered, this, [this](const QSet& stackIds) { if (stackIds.isEmpty()) { ++m_currentHoverStacksJobId; diff --git a/tests/modeltests/tst_models.cpp b/tests/modeltests/tst_models.cpp index 3da662a1..965e7722 100644 --- a/tests/modeltests/tst_models.cpp +++ b/tests/modeltests/tst_models.cpp @@ -559,72 +559,18 @@ private slots: void testEventModel() { - Data::EventResults events; - events.cpus.resize(3); - events.cpus[0].cpuId = 0; - events.cpus[1].cpuId = 1; // empty - events.cpus[2].cpuId = 2; + const auto events = createEventModelTestData(); const int nonEmptyCpus = 2; const int processes = 2; const quint64 endTime = 1000; - const quint64 deltaTime = 10; - events.threads.resize(4); - auto& thread1 = events.threads[0]; - { - thread1.pid = 1234; - thread1.tid = 1234; - thread1.time = {0, endTime}; - thread1.name = QStringLiteral("foobar"); - } - auto& thread2 = events.threads[1]; - { - thread2.pid = 1234; - thread2.tid = 1235; - thread2.time = {deltaTime, endTime - deltaTime}; - thread2.name = QStringLiteral("asdf"); - } - auto& thread3 = events.threads[2]; - { - thread3.pid = 5678; - thread3.tid = 5678; - thread3.time = {0, endTime}; - thread3.name = QStringLiteral("barfoo"); - } - auto& thread4 = events.threads[3]; - { - thread4.pid = 5678; - thread4.tid = 5679; - thread4.time = {endTime - deltaTime, endTime}; - thread4.name = QStringLiteral("blub"); - } - - Data::CostSummary costSummary(QStringLiteral("cycles"), 0, 0, Data::Costs::Unit::Unknown); - auto generateEvent = [&costSummary, &events](quint64 time, quint32 cpuId) -> Data::Event { - Data::Event event; - event.cost = 10; - event.cpuId = cpuId; - event.type = 0; - event.time = time; - ++costSummary.sampleCount; - costSummary.totalPeriod += event.cost; - events.cpus[cpuId].events << event; - return event; - }; - for (quint64 time = 0; time < endTime; time += deltaTime) { - thread1.events << generateEvent(time, 0); - if (thread2.time.contains(time)) { - thread2.events << generateEvent(time, 2); - } - } - events.totalCosts = {costSummary}; EventModel model; QAbstractItemModelTester tester(&model); model.setData(events); QCOMPARE(model.columnCount(), static_cast(EventModel::NUM_COLUMNS)); - QCOMPARE(model.rowCount(), 2); + QCOMPARE(model.rowCount(), 4); auto simplifiedEvents = events; simplifiedEvents.cpus.remove(1); @@ -706,6 +652,27 @@ private slots: } } + void testEventModelFavorites() + { + const auto events = createEventModelTestData(); + EventModel model; + QAbstractItemModelTester tester(&model); + model.setData(events); + + const auto favoritesIndex = model.index(3, 0); + const auto processesIndex = model.index(1, 0); + + QCOMPARE(model.rowCount(favoritesIndex), 0); + QCOMPARE(model.data(model.index(0, 0, processesIndex)).toString(), QLatin1String("foobar (#1234)")); + + model.addToFavorites(model.index(0, 0, processesIndex)); + QCOMPARE(model.rowCount(favoritesIndex), 1); + QCOMPARE(model.data(model.index(0, 0, favoritesIndex)).toString(), QLatin1String("foobar (#1234)")); + + model.removeFromFavorites(model.index(0, 0, favoritesIndex)); + QCOMPARE(model.rowCount(favoritesIndex), 0); + } + void testPrettySymbol_data() { QTest::addColumn("prettySymbol"); @@ -948,6 +915,69 @@ private slots: font.setPixelSize(10); return QFontMetrics(font); } + + Data::EventResults createEventModelTestData() + { + Data::EventResults events; + events.cpus.resize(3); + events.cpus[0].cpuId = 0; + events.cpus[1].cpuId = 1; // empty + events.cpus[2].cpuId = 2; + + const quint64 endTime = 1000; + const quint64 deltaTime = 10; + events.threads.resize(4); + auto& thread1 = events.threads[0]; + { + thread1.pid = 1234; + thread1.tid = 1234; + thread1.time = {0, endTime}; + thread1.name = QStringLiteral("foobar"); + } + auto& thread2 = events.threads[1]; + { + thread2.pid = 1234; + thread2.tid = 1235; + thread2.time = {deltaTime, endTime - deltaTime}; + thread2.name = QStringLiteral("asdf"); + } + auto& thread3 = events.threads[2]; + { + thread3.pid = 5678; + thread3.tid = 5678; + thread3.time = {0, endTime}; + thread3.name = QStringLiteral("barfoo"); + } + auto& thread4 = events.threads[3]; + { + thread4.pid = 5678; + thread4.tid = 5679; + thread4.time = {endTime - deltaTime, endTime}; + thread4.name = QStringLiteral("blub"); + } + + Data::CostSummary costSummary(QStringLiteral("cycles"), 0, 0, Data::Costs::Unit::Unknown); + auto generateEvent = [&costSummary, &events](quint64 time, quint32 cpuId) -> Data::Event { + Data::Event event; + event.cost = 10; + event.cpuId = cpuId; + event.type = 0; + event.time = time; + ++costSummary.sampleCount; + costSummary.totalPeriod += event.cost; + events.cpus[cpuId].events << event; + return event; + }; + for (quint64 time = 0; time < endTime; time += deltaTime) { + thread1.events << generateEvent(time, 0); + if (thread2.time.contains(time)) { + thread2.events << generateEvent(time, 2); + } + } + events.totalCosts = {costSummary}; + + return events; + } }; HOTSPOT_GUITEST_MAIN(TestModels) From 568e935a262da3ce52b08929b7abbcc4b4ec6041 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Wed, 4 Oct 2023 12:28:55 +0200 Subject: [PATCH 05/13] feat: add QSFP to hide empty rows in eventmodel The favourites and tracepoint patches include some rows in the model that may be empty. To keep the code simple an readable all rows will be shown. Then a proxy model is put ontop to remove empty rows. --- src/models/CMakeLists.txt | 1 + src/models/eventmodelproxy.cpp | 43 +++++++++++++++++++++++++++++++++ src/models/eventmodelproxy.h | 21 ++++++++++++++++ src/timelinewidget.cpp | 7 ++---- tests/modeltests/tst_models.cpp | 33 +++++++++++++++++++++++++ 5 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 src/models/eventmodelproxy.cpp create mode 100644 src/models/eventmodelproxy.h diff --git a/src/models/CMakeLists.txt b/src/models/CMakeLists.txt index b1754778..16846410 100644 --- a/src/models/CMakeLists.txt +++ b/src/models/CMakeLists.txt @@ -11,6 +11,7 @@ add_library( disassemblymodel.cpp disassemblyoutput.cpp eventmodel.cpp + eventmodelproxy.cpp filterandzoomstack.cpp formattingutils.cpp frequencymodel.cpp diff --git a/src/models/eventmodelproxy.cpp b/src/models/eventmodelproxy.cpp new file mode 100644 index 00000000..12a547ba --- /dev/null +++ b/src/models/eventmodelproxy.cpp @@ -0,0 +1,43 @@ +/* + SPDX-FileCopyrightText: Lieven Hey + SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#include "eventmodelproxy.h" +#include "eventmodel.h" + +EventModelProxy::EventModelProxy(QObject* parent) + : QSortFilterProxyModel(parent) +{ + setDynamicSortFilter(true); + setRecursiveFilteringEnabled(true); + setSortRole(EventModel::SortRole); + setFilterKeyColumn(EventModel::ThreadColumn); + setFilterRole(Qt::DisplayRole); +} + +EventModelProxy::~EventModelProxy() = default; + +bool EventModelProxy::filterAcceptsRow(int source_row, const QModelIndex& source_parent) const +{ + // index is invalid -> we are at the root node + // hide categories that have no children (e.g. favorites, tracepoints) + if (!source_parent.isValid()) { + const auto model = sourceModel(); + if (!model->hasChildren(model->index(source_row, 0))) + return false; + } + + auto data = sourceModel() + ->index(source_row, EventModel::EventsColumn, source_parent) + .data(EventModel::EventsRole) + .value(); + + if (data.empty()) { + return false; + } + + return QSortFilterProxyModel::filterAcceptsRow(source_row, source_parent); +} diff --git a/src/models/eventmodelproxy.h b/src/models/eventmodelproxy.h new file mode 100644 index 00000000..a720fd58 --- /dev/null +++ b/src/models/eventmodelproxy.h @@ -0,0 +1,21 @@ +/* + SPDX-FileCopyrightText: Lieven Hey + SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#pragma once + +#include + +class EventModelProxy : public QSortFilterProxyModel +{ + Q_OBJECT +public: + explicit EventModelProxy(QObject* parent = nullptr); + ~EventModelProxy() override; + +protected: + bool filterAcceptsRow(int source_row, const QModelIndex& source_parent) const override; +}; diff --git a/src/timelinewidget.cpp b/src/timelinewidget.cpp index 6bfa5d84..8c40ea40 100644 --- a/src/timelinewidget.cpp +++ b/src/timelinewidget.cpp @@ -9,6 +9,7 @@ #include "filterandzoomstack.h" #include "models/eventmodel.h" +#include "models/eventmodelproxy.h" #include "resultsutil.h" #include "timelinedelegate.h" @@ -61,12 +62,8 @@ TimeLineWidget::TimeLineWidget(PerfParser* parser, QMenu* filterMenu, FilterAndZ ui->setupUi(this); auto* eventModel = new EventModel(this); - auto* timeLineProxy = new QSortFilterProxyModel(this); - timeLineProxy->setRecursiveFilteringEnabled(true); + auto* timeLineProxy = new EventModelProxy(this); timeLineProxy->setSourceModel(eventModel); - timeLineProxy->setSortRole(EventModel::SortRole); - timeLineProxy->setFilterKeyColumn(EventModel::ThreadColumn); - timeLineProxy->setFilterRole(Qt::DisplayRole); ResultsUtil::connectFilter(ui->timeLineSearch, timeLineProxy, ui->regexCheckBox); ui->timeLineView->setModel(timeLineProxy); ui->timeLineView->setSortingEnabled(true); diff --git a/tests/modeltests/tst_models.cpp b/tests/modeltests/tst_models.cpp index 965e7722..d88a28d4 100644 --- a/tests/modeltests/tst_models.cpp +++ b/tests/modeltests/tst_models.cpp @@ -21,6 +21,7 @@ #include #include +#include #include namespace { @@ -673,6 +674,38 @@ private slots: QCOMPARE(model.rowCount(favoritesIndex), 0); } + void testEventModelProxy() + { + const auto events = createEventModelTestData(); + EventModel model; + QAbstractItemModelTester tester(&model); + model.setData(events); + + EventModelProxy proxy; + proxy.setSourceModel(&model); + + const auto favoritesIndex = model.index(3, 0); + const auto processesIndex = model.index(1, 0); + + QCOMPARE(model.rowCount(), 4); + QCOMPARE(proxy.rowCount(), 2); + + proxy.setFilterRegularExpression(QStringLiteral("this does not match")); + QCOMPARE(proxy.rowCount(), 0); + proxy.setFilterRegularExpression(QString()); + QCOMPARE(proxy.rowCount(), 2); + + // add the first data trace to favourites + // adding the whole process doesn't work currently + auto firstProcess = model.index(0, 0, processesIndex); + model.addToFavorites(model.index(0, 0, firstProcess)); + + QCOMPARE(proxy.rowCount(), 3); + + model.removeFromFavorites(model.index(0, 0, favoritesIndex)); + QCOMPARE(proxy.rowCount(), 2); + } + void testPrettySymbol_data() { QTest::addColumn("prettySymbol"); From 07794493da44775964359a16cfdafd1ec03873aa Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Wed, 4 Oct 2023 22:06:05 +0200 Subject: [PATCH 06/13] feat: Always put the favorite contents on the top of the view This way we can more easily find them and changing the sort order doesn't move them to the bottom. --- src/models/eventmodel.cpp | 2 ++ src/models/eventmodel.h | 1 + src/models/eventmodelproxy.cpp | 16 ++++++++++++++++ src/models/eventmodelproxy.h | 1 + tests/modeltests/tst_models.cpp | 31 +++++++++++++++++++++++++------ 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/models/eventmodel.cpp b/src/models/eventmodel.cpp index 9096c2d2..cf8184c5 100644 --- a/src/models/eventmodel.cpp +++ b/src/models/eventmodel.cpp @@ -191,6 +191,8 @@ QVariant EventModel::data(const QModelIndex& index, int role) const } } else if (role == SortRole) { return index.row(); + } else if (role == IsFavoritesSectionRole) { + return index.row() == OverviewRow::FavoriteRow; } return {}; } else if (tag == Tag::Processes) { diff --git a/src/models/eventmodel.h b/src/models/eventmodel.h index 6c0d782e..f6a471c3 100644 --- a/src/models/eventmodel.h +++ b/src/models/eventmodel.h @@ -45,6 +45,7 @@ class EventModel : public QAbstractItemModel TotalCostsRole, EventResultsRole, IsFavoriteRole, + IsFavoritesSectionRole, }; int rowCount(const QModelIndex& parent = {}) const override; diff --git a/src/models/eventmodelproxy.cpp b/src/models/eventmodelproxy.cpp index 12a547ba..19e96a81 100644 --- a/src/models/eventmodelproxy.cpp +++ b/src/models/eventmodelproxy.cpp @@ -16,6 +16,7 @@ EventModelProxy::EventModelProxy(QObject* parent) setSortRole(EventModel::SortRole); setFilterKeyColumn(EventModel::ThreadColumn); setFilterRole(Qt::DisplayRole); + sort(0); } EventModelProxy::~EventModelProxy() = default; @@ -41,3 +42,18 @@ bool EventModelProxy::filterAcceptsRow(int source_row, const QModelIndex& source return QSortFilterProxyModel::filterAcceptsRow(source_row, source_parent); } + +bool EventModelProxy::lessThan(const QModelIndex& source_left, const QModelIndex& source_right) const +{ + const auto lhsIsFavoritesSection = source_left.data(EventModel::IsFavoritesSectionRole).toBool(); + const auto rhsIsFavoritesSection = source_right.data(EventModel::IsFavoritesSectionRole).toBool(); + if (lhsIsFavoritesSection != rhsIsFavoritesSection) { + // always put the favorites section on the top + if (sortOrder() == Qt::AscendingOrder) + return lhsIsFavoritesSection > rhsIsFavoritesSection; + else + return lhsIsFavoritesSection < rhsIsFavoritesSection; + } + + return QSortFilterProxyModel::lessThan(source_left, source_right); +} diff --git a/src/models/eventmodelproxy.h b/src/models/eventmodelproxy.h index a720fd58..eaedf37a 100644 --- a/src/models/eventmodelproxy.h +++ b/src/models/eventmodelproxy.h @@ -18,4 +18,5 @@ class EventModelProxy : public QSortFilterProxyModel protected: bool filterAcceptsRow(int source_row, const QModelIndex& source_parent) const override; + bool lessThan(const QModelIndex& source_left, const QModelIndex& source_right) const override; }; diff --git a/tests/modeltests/tst_models.cpp b/tests/modeltests/tst_models.cpp index d88a28d4..40bf9dbb 100644 --- a/tests/modeltests/tst_models.cpp +++ b/tests/modeltests/tst_models.cpp @@ -702,7 +702,26 @@ private slots: QCOMPARE(proxy.rowCount(), 3); + { + // verify that favorites remain at the top + QCOMPARE(proxy.sortOrder(), Qt::AscendingOrder); + QCOMPARE(proxy.sortColumn(), 0); + + // favorites on top + QVERIFY(proxy.index(0, 0, proxy.index(0, 0)).data(EventModel::IsFavoriteRole).toBool()); + // followed by CPUs + QCOMPARE(proxy.index(0, 0, proxy.index(1, 0)).data(EventModel::CpuIdRole).value(), 1); + + proxy.sort(0, Qt::DescendingOrder); + + // favorites are still on top + QVERIFY(proxy.index(0, 0, proxy.index(0, 0)).data(EventModel::IsFavoriteRole).toBool()); + // followed by processes + QCOMPARE(proxy.index(0, 0, proxy.index(1, 0)).data(EventModel::ProcessIdRole).value(), 1234); + } + model.removeFromFavorites(model.index(0, 0, favoritesIndex)); + QCOMPARE(proxy.rowCount(), 2); } @@ -953,9 +972,9 @@ private slots: { Data::EventResults events; events.cpus.resize(3); - events.cpus[0].cpuId = 0; - events.cpus[1].cpuId = 1; // empty - events.cpus[2].cpuId = 2; + events.cpus[0].cpuId = 1; + events.cpus[1].cpuId = 2; // empty + events.cpus[2].cpuId = 3; const quint64 endTime = 1000; const quint64 deltaTime = 10; @@ -998,13 +1017,13 @@ private slots: event.time = time; ++costSummary.sampleCount; costSummary.totalPeriod += event.cost; - events.cpus[cpuId].events << event; + events.cpus[cpuId - 1].events << event; return event; }; for (quint64 time = 0; time < endTime; time += deltaTime) { - thread1.events << generateEvent(time, 0); + thread1.events << generateEvent(time, 1); if (thread2.time.contains(time)) { - thread2.events << generateEvent(time, 2); + thread2.events << generateEvent(time, 3); } } events.totalCosts = {costSummary}; From a08c2a538bd88c2a4cd258f0eb59f98a45853e73 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Mon, 15 Jan 2024 15:56:54 +0100 Subject: [PATCH 07/13] feat: Show multiple costs in timelinewidget Showing only one cost is fine if we only show a hardware event, but since we now support tracepoints and some come in an enter/exit pair it requires us to rework the timeline delegate. This patch makes the event source combobox multi select and allows to select multiple event sources. --- src/models/eventmodelproxy.cpp | 14 ++++++++++++- src/models/eventmodelproxy.h | 7 +++++++ src/models/timelinedelegate.cpp | 35 ++++++++++++++++++++------------- src/models/timelinedelegate.h | 2 +- src/resultsutil.cpp | 32 ++++++++++++++++++++++++++++++ src/resultsutil.h | 1 + src/timelinewidget.cpp | 29 ++++++++++++++++++--------- 7 files changed, 95 insertions(+), 25 deletions(-) diff --git a/src/models/eventmodelproxy.cpp b/src/models/eventmodelproxy.cpp index 19e96a81..721d86d6 100644 --- a/src/models/eventmodelproxy.cpp +++ b/src/models/eventmodelproxy.cpp @@ -21,6 +21,18 @@ EventModelProxy::EventModelProxy(QObject* parent) EventModelProxy::~EventModelProxy() = default; +void EventModelProxy::showCostId(qint32 costId) +{ + m_hiddenCostIds.remove(costId); + invalidate(); +} + +void EventModelProxy::hideCostId(qint32 costId) +{ + m_hiddenCostIds.insert(costId); + invalidate(); +} + bool EventModelProxy::filterAcceptsRow(int source_row, const QModelIndex& source_parent) const { // index is invalid -> we are at the root node @@ -36,7 +48,7 @@ bool EventModelProxy::filterAcceptsRow(int source_row, const QModelIndex& source .data(EventModel::EventsRole) .value(); - if (data.empty()) { + if (data.empty() || m_hiddenCostIds.contains(data[0].type)) { return false; } diff --git a/src/models/eventmodelproxy.h b/src/models/eventmodelproxy.h index eaedf37a..a382aa3c 100644 --- a/src/models/eventmodelproxy.h +++ b/src/models/eventmodelproxy.h @@ -7,6 +7,7 @@ #pragma once +#include #include class EventModelProxy : public QSortFilterProxyModel @@ -16,7 +17,13 @@ class EventModelProxy : public QSortFilterProxyModel explicit EventModelProxy(QObject* parent = nullptr); ~EventModelProxy() override; + void showCostId(qint32 costId); + void hideCostId(qint32 costId); + protected: bool filterAcceptsRow(int source_row, const QModelIndex& source_parent) const override; bool lessThan(const QModelIndex& source_left, const QModelIndex& source_right) const override; + +private: + QSet m_hiddenCostIds; }; diff --git a/src/models/timelinedelegate.cpp b/src/models/timelinedelegate.cpp index fa89f61f..4c2be5bf 100644 --- a/src/models/timelinedelegate.cpp +++ b/src/models/timelinedelegate.cpp @@ -161,7 +161,6 @@ void TimeLineDelegate::paint(QPainter* painter, const QStyleOptionViewItem& opti const auto results = index.data(EventModel::EventResultsRole).value(); const auto offCpuCostId = results.offCpuTimeCostId; const auto lostEventCostId = results.lostEventCostId; - const auto tracepointEventCostId = results.tracepointEventCostId; const bool is_alternate = option.features & QStyleOptionViewItem::Alternate; const auto& palette = option.palette; @@ -231,10 +230,6 @@ void TimeLineDelegate::paint(QPainter* painter, const QStyleOptionViewItem& opti // see also: https://www.spinics.net/lists/linux-perf-users/msg03486.html for (const auto& event : data.events) { const auto isLostEvent = event.type == lostEventCostId; - const auto isTracepointEvent = event.type == tracepointEventCostId; - if (event.type != m_eventType && !isLostEvent && !isTracepointEvent) { - continue; - } const auto x = data.mapTimeToX(event.time); if (x < TimeLineData::padding || x >= data.w) { @@ -336,16 +331,22 @@ bool TimeLineDelegate::helpEvent(QHelpEvent* event, QAbstractItemView* view, con Util::formatTimeString(found.totalCost), Util::formatTimeString(found.maxCost)), view); } else if (found.numSamples > 0) { - QToolTip::showText(event->globalPos(), - tr("time: %1\n%5 samples: %2\ntotal sample cost: %3\nmax sample cost: %4") - .arg(formattedTime, QString::number(found.numSamples), - Util::formatCost(found.totalCost), Util::formatCost(found.maxCost), - totalCosts.value(found.type).label), - view); + if (m_eventType == results.tracepointEventCostId) { + // currently tracepoint cost is saying nothig, so don't show it + QToolTip::showText( + event->globalPos(), + tr("time: %1\n%3 samples: %2") + .arg(formattedTime, QString::number(found.numSamples), results.tracepoints[index.row()].name)); + + } else { + QToolTip::showText(event->globalPos(), + tr("time: %1\n%5 samples: %2\ntotal sample cost: %3\nmax sample cost: %4") + .arg(formattedTime, QString::number(found.numSamples), + Util::formatCost(found.totalCost), Util::formatCost(found.maxCost), + totalCosts.value(found.type).label)); + } } else { - QToolTip::showText(event->globalPos(), - tr("time: %1 (no %2 samples)").arg(formattedTime, totalCosts.value(m_eventType).label), - view); + QToolTip::showText(event->globalPos(), tr("time: %1 (no samples)").arg(formattedTime)); } return true; } @@ -394,6 +395,12 @@ bool TimeLineDelegate::eventFilter(QObject* watched, QEvent* event) const auto time = data.mapXToTime(pos.x() - visualRect.left() - TimeLineData::padding); const auto start = findEvent(data.events.constBegin(), data.events.constEnd(), time); + + // we can show multiple events in one row so we need to dynamically figure out which costId is needed + auto hoveringEntry = std::find_if(start, data.events.cend(), + [time](const Data::Event& event) { return event.time >= time; }); + setEventType(hoveringEntry != data.events.cend() ? hoveringEntry->type : 0); + auto findSamples = [&](int costType, bool contains) { bool foundAny = false; data.findSamples(hoverX, costType, results.lostEventCostId, contains, start, diff --git a/src/models/timelinedelegate.h b/src/models/timelinedelegate.h index 69759a0a..7477c091 100644 --- a/src/models/timelinedelegate.h +++ b/src/models/timelinedelegate.h @@ -59,7 +59,6 @@ class TimeLineDelegate : public QStyledItemDelegate bool helpEvent(QHelpEvent* event, QAbstractItemView* view, const QStyleOptionViewItem& option, const QModelIndex& index) override; - void setEventType(int type); void setSelectedStacks(const QSet& selectedStacks); signals: @@ -71,6 +70,7 @@ class TimeLineDelegate : public QStyledItemDelegate bool eventFilter(QObject* watched, QEvent* event) override; private: + void setEventType(int type); void updateView(); void updateZoomState(); diff --git a/src/resultsutil.cpp b/src/resultsutil.cpp index 6fac24f7..7699b229 100644 --- a/src/resultsutil.cpp +++ b/src/resultsutil.cpp @@ -19,6 +19,8 @@ #include #include +#include + #include "models/costdelegate.h" #include "models/data.h" #include "models/filterandzoomstack.h" @@ -219,6 +221,36 @@ void fillEventSourceComboBox(QComboBox* combo, const Data::Costs& costs, const Q } } +void fillEventSourceComboBoxMultiSelect(QComboBox* combo, const Data::Costs& costs, const QString& /*tooltipTemplate*/) +{ + // restore selection if possible + const auto oldData = combo->currentData(); + + combo->clear(); + + auto model = new QStandardItemModel(costs.numTypes(), 1, combo); + int itemCounter = 0; + for (int costId = 0, c = costs.numTypes(); costId < c; costId++) { + if (!costs.totalCost(costId)) { + continue; + } + + auto item = new QStandardItem(costs.typeName(costId)); + item->setFlags(Qt::ItemIsUserCheckable | Qt::ItemIsEnabled); + item->setData(Qt::Checked, Qt::CheckStateRole); + item->setData(costId, Qt::UserRole + 1); + model->setItem(itemCounter, item); + itemCounter++; + } + model->setRowCount(itemCounter); + combo->setModel(model); + + const auto index = combo->findData(oldData); + if (index != -1) { + combo->setCurrentIndex(index); + } +} + void setupResultsAggregation(QComboBox* costAggregationComboBox) { struct AggregationType diff --git a/src/resultsutil.h b/src/resultsutil.h index 568d8870..0551a007 100644 --- a/src/resultsutil.h +++ b/src/resultsutil.h @@ -100,6 +100,7 @@ void hideEmptyColumns(const Data::Costs& costs, QTreeView* view, int numBaseColu void hideTracepointColumns(const Data::Costs& costs, QTreeView* view, int numBaseColumns); void fillEventSourceComboBox(QComboBox* combo, const Data::Costs& costs, const QString& tooltipTemplate); +void fillEventSourceComboBoxMultiSelect(QComboBox* combo, const Data::Costs& costs, const QString& tooltipTemplate); void setupResultsAggregation(QComboBox* costAggregationComboBox); } diff --git a/src/timelinewidget.cpp b/src/timelinewidget.cpp index 8c40ea40..3096db8a 100644 --- a/src/timelinewidget.cpp +++ b/src/timelinewidget.cpp @@ -17,9 +17,11 @@ #include "parsers/perf/perfparser.h" #include +#include #include #include #include +#include #include #include @@ -82,9 +84,24 @@ TimeLineWidget::TimeLineWidget(PerfParser* parser, QMenu* filterMenu, FilterAndZ connect(timeLineProxy, &QAbstractItemModel::rowsInserted, this, [this]() { ui->timeLineView->expandToDepth(1); }); connect(timeLineProxy, &QAbstractItemModel::modelReset, this, [this]() { ui->timeLineView->expandToDepth(1); }); - connect(m_parser, &PerfParser::bottomUpDataAvailable, this, [this](const Data::BottomUpResults& data) { - ResultsUtil::fillEventSourceComboBox(ui->timeLineEventSource, data.costs, tr("Show timeline for %1 events.")); - }); + connect(m_parser, &PerfParser::bottomUpDataAvailable, this, + [this, timeLineProxy](const Data::BottomUpResults& data) { + ResultsUtil::fillEventSourceComboBoxMultiSelect(ui->timeLineEventSource, data.costs, + tr("Show timeline for %1 events.")); + + auto model = qobject_cast(ui->timeLineEventSource->model()); + connect(ui->timeLineEventSource->model(), &QStandardItemModel::dataChanged, model, + [timeLineProxy](const QModelIndex& topLeft, const QModelIndex& /*bottomRight*/, + const QVector& /*roles*/) { + auto checkState = topLeft.data(Qt::CheckStateRole).value(); + + if (checkState == Qt::CheckState::Checked) { + timeLineProxy->showCostId(topLeft.data(Qt::UserRole + 1).toUInt()); + } else { + timeLineProxy->hideCostId(topLeft.data(Qt::UserRole + 1).toUInt()); + } + }); + }); connect(m_parser, &PerfParser::eventsAvailable, this, [this, eventModel](const Data::EventResults& data) { eventModel->setData(data); @@ -103,12 +120,6 @@ TimeLineWidget::TimeLineWidget(PerfParser* parser, QMenu* filterMenu, FilterAndZ connect(m_parser, &PerfParser::summaryDataAvailable, this, [eventModel](const Data::Summary& summary) { eventModel->setApplicationTime(summary.applicationTime); }); - connect(ui->timeLineEventSource, static_cast(&QComboBox::currentIndexChanged), this, - [this](int index) { - const auto typeId = ui->timeLineEventSource->itemData(index).toInt(); - m_timeLineDelegate->setEventType(typeId); - }); - connect(m_timeLineDelegate, &TimeLineDelegate::addToFavorites, this, [eventModel](const QModelIndex& index) { eventModel->addToFavorites(index); }); connect(m_timeLineDelegate, &TimeLineDelegate::removeFromFavorites, this, From 2583d516ffb3f2745515ea5dd1da46d158046da3 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Mon, 18 Nov 2024 11:04:30 +0100 Subject: [PATCH 08/13] feat: tracepoint data on hover --- src/models/CMakeLists.txt | 1 + src/models/data.h | 23 ++++++++++++++++++++-- src/models/timelinedelegate.cpp | 35 +++++++++++++++++++++++++-------- src/parsers/perf/perfparser.cpp | 32 ++++++++++++++++++++---------- tests/modeltests/CMakeLists.txt | 14 ++++++++++++- 5 files changed, 84 insertions(+), 21 deletions(-) diff --git a/src/models/CMakeLists.txt b/src/models/CMakeLists.txt index 16846410..780b1dbf 100644 --- a/src/models/CMakeLists.txt +++ b/src/models/CMakeLists.txt @@ -23,6 +23,7 @@ add_library( timeaxisheaderview.cpp timelinedelegate.cpp topproxy.cpp + tracepointformat.cpp treemodel.cpp ) diff --git a/src/models/data.h b/src/models/data.h index fdb7187a..8af1d050 100644 --- a/src/models/data.h +++ b/src/models/data.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include "../util.h" @@ -787,6 +788,9 @@ const constexpr auto INVALID_CPU_ID = std::numeric_limits::max(); const constexpr int INVALID_TID = -1; const constexpr int INVALID_PID = -1; +const constexpr auto INVALID_TRACEPOINTFORMAT = std::numeric_limits::max(); +const constexpr auto INVALID_TRACEPOINTDATA = std::numeric_limits::max(); + struct Event { quint64 time = 0; @@ -794,11 +798,13 @@ struct Event qint32 type = -1; qint32 stackId = -1; quint32 cpuId = INVALID_CPU_ID; + quint32 tracepointFormat = INVALID_TRACEPOINTFORMAT; + quint32 tracepointData = INVALID_TRACEPOINTDATA; bool operator==(const Event& rhs) const { - return std::tie(time, cost, type, stackId, cpuId) - == std::tie(rhs.time, rhs.cost, rhs.type, rhs.stackId, rhs.cpuId); + return std::tie(time, cost, type, stackId, cpuId, tracepointFormat, tracepointData) + == std::tie(rhs.time, rhs.cost, rhs.type, rhs.stackId, rhs.cpuId, rhs.tracepointFormat, rhs.tracepointData); } }; @@ -962,6 +968,17 @@ struct TracepointEvents } }; +struct TracePointFormat +{ + QString systemId; + QString nameId; + quint32 flags; + QString format; +}; + +#include +using TracePointData = QHash; + struct EventResults { QVector threads; @@ -969,6 +986,8 @@ struct EventResults QVector tracepoints; QVector> stacks; QVector totalCosts; + QHash tracePointFormats; + QVector tracePointData; qint32 offCpuTimeCostId = -1; qint32 lostEventCostId = -1; qint32 tracepointEventCostId = -1; diff --git a/src/models/timelinedelegate.cpp b/src/models/timelinedelegate.cpp index 4c2be5bf..a542f7e4 100644 --- a/src/models/timelinedelegate.cpp +++ b/src/models/timelinedelegate.cpp @@ -19,6 +19,7 @@ #include "../util.h" #include "eventmodel.h" #include "filterandzoomstack.h" +#include "tracepointformat.h" #include @@ -314,9 +315,7 @@ bool TimeLineDelegate::helpEvent(QHelpEvent* event, QAbstractItemView* view, con // check whether we are hovering an off-CPU area found = findSamples(results.offCpuTimeCostId, true); } - - const auto appStartTime = index.data(EventModel::ApplicationStartTimeRole).value(); - const auto formattedTime = Util::formatTimeString(time - appStartTime); + const auto formattedTime = Util::formatTimeString(time - data.time.start); const auto totalCosts = index.data(EventModel::TotalCostsRole).value>(); if (found.numLost > 0) { QToolTip::showText( @@ -332,11 +331,31 @@ bool TimeLineDelegate::helpEvent(QHelpEvent* event, QAbstractItemView* view, con view); } else if (found.numSamples > 0) { if (m_eventType == results.tracepointEventCostId) { - // currently tracepoint cost is saying nothig, so don't show it - QToolTip::showText( - event->globalPos(), - tr("time: %1\n%3 samples: %2") - .arg(formattedTime, QString::number(found.numSamples), results.tracepoints[index.row()].name)); + if (found.numSamples != 1) { + QToolTip::showText(event->globalPos(), + tr("time: %1\n%3 samples: %2") + .arg(formattedTime, QString::number(found.numSamples), + results.tracepoints[index.row()].name)); + } else { + // we only hover over one tracepoint, find it + Data::Event tracepoint; + data.findSamples(mappedX, m_eventType, results.lostEventCostId, false, start, + [&tracepoint](const Data::Event& event, bool isLost) { + Q_UNUSED(isLost); + tracepoint = event; + }); + + const auto format = results.tracePointFormats[tracepoint.tracepointFormat]; + qDebug() << format.systemId << format.nameId << format.format; + qDebug() << results.tracePointData[tracepoint.tracepointData]; + + TracePointFormatter formatter(format.format); + + QToolTip::showText(event->globalPos(), + tr("time: %1\n%2:\n%3") + .arg(formattedTime, results.tracepoints[index.row()].name, + formatter.format(results.tracePointData[tracepoint.tracepointData]))); + } } else { QToolTip::showText(event->globalPos(), diff --git a/src/parsers/perf/perfparser.cpp b/src/parsers/perf/perfparser.cpp index 97fa4a61..7dbf593b 100644 --- a/src/parsers/perf/perfparser.cpp +++ b/src/parsers/perf/perfparser.cpp @@ -564,13 +564,24 @@ QDebug operator<<(QDebug stream, const TracePointFormat& format) return stream; } -using TracePointData = QHash; +struct TracePointData +{ + quint32 formatId; + QHash data; +}; + +QDataStream& operator>>(QDataStream& stream, TracePointData& traceData) +{ + stream >> traceData.formatId >> traceData.data; + return stream; +} QDebug operator<<(QDebug stream, const TracePointData& traceData) { auto s = stream.noquote().nospace(); s << "TracePointData{"; - for (auto it = traceData.cbegin(), end = traceData.cend(); it != end; it++) { + s << "eventId=" << traceData.formatId << ", "; + for (auto it = traceData.data.cbegin(), end = traceData.data.cend(); it != end; it++) { s << it.key() << "=" << it.value() << ", "; } s << "}"; @@ -799,12 +810,11 @@ class PerfParserPrivate : public QObject } if (static_cast(eventType) == EventType::TracePointSample) { - quint32 eventFormatId; TracePointData traceData; - stream >> eventFormatId >> traceData; - tracepointData[eventFormatId].push_back(traceData); + stream >> traceData; + tracepointData.push_back(traceData); qCDebug(LOG_PERFPARSER) << "parsed:" << traceData; - sample.tracePointFormat = eventFormatId; + sample.tracePointFormat = traceData.formatId; sample.tracePointData = tracepointData.size() - 1; } @@ -925,7 +935,7 @@ class PerfParserPrivate : public QObject case EventType::TracePointFormat: { qint32 id; TracePointFormat format; - stream >> id >> format; + stream >> id >> format; // id is the tracepoint id, see /sys/kernel/tracing/system/tracepoint qCDebug(LOG_PERFPARSER) << "parsed:" << format; tracepointFormat[id] = format; break; @@ -1196,6 +1206,8 @@ class PerfParserPrivate : public QObject event.type = attributeIdsToCostIds.value(sampleCost.attributeId, -1); event.stackId = internStack(sample.frames); event.cpuId = sample.cpu; + event.tracepointFormat = sample.tracePointFormat; + event.tracepointData = sample.tracePointData; thread->events.push_back(event); cpu.events.push_back(event); @@ -1208,7 +1220,8 @@ class PerfParserPrivate : public QObject if (attribute.name.id != m_schedSwitchId) { auto& tracepointList = tracepoints[attribute.name.id]; - tracepointList.push_back({event.time, 0, eventResult.tracepointEventCostId}); + event.type = eventResult.tracepointEventCostId; + tracepointList.push_back(event); } } } @@ -1516,7 +1529,7 @@ class PerfParserPrivate : public QObject Settings::CostAggregation costAggregation; bool perfMapFileExists = false; QHash tracepointFormat; - QHash> tracepointData; + QVector tracepointData; // samples recorded without --call-graph have only one frame int m_numSamplesWithMoreThanOneFrame = 0; @@ -1548,7 +1561,6 @@ PerfParser::PerfParser(QObject* parent) qRegisterMetaType(); qRegisterMetaType(); qRegisterMetaType(); - qRegisterMetaType(); qRegisterMetaType(); qRegisterMetaType(); diff --git a/tests/modeltests/CMakeLists.txt b/tests/modeltests/CMakeLists.txt index a8af4476..a4bb59ad 100644 --- a/tests/modeltests/CMakeLists.txt +++ b/tests/modeltests/CMakeLists.txt @@ -39,6 +39,7 @@ set_target_properties( ecm_add_test( tst_disassemblyoutput.cpp + ../../src/settings.cpp LINK_LIBRARIES Qt::Core Qt::Test @@ -46,7 +47,6 @@ ecm_add_test( PrefixTickLabels TEST_NAME tst_disassemblyoutput - ../../src/settings.cpp ) set_target_properties( @@ -95,3 +95,15 @@ ecm_add_test( tst_formatting ) set_target_properties(tst_formatting PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/${KDE_INSTALL_BINDIR}") + +ecm_add_test( + tst_tracepointformat.cpp + LINK_LIBRARIES + Qt::Test + models + TEST_NAME + tst_tracepointformat +) +set_target_properties( + tst_tracepointformat PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/${KDE_INSTALL_BINDIR}" +) From 7b417d5c2de2ec8b0b77a990d9c0ea61fb197fe9 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Mon, 18 Nov 2024 13:35:56 +0100 Subject: [PATCH 09/13] feat: add tracepoint formatter --- src/models/tracepointformat.cpp | 249 ++++++++++++++++++++++ src/models/tracepointformat.h | 35 +++ tests/modeltests/tst_tracepointformat.cpp | 109 ++++++++++ 3 files changed, 393 insertions(+) create mode 100644 src/models/tracepointformat.cpp create mode 100644 src/models/tracepointformat.h create mode 100644 tests/modeltests/tst_tracepointformat.cpp diff --git a/src/models/tracepointformat.cpp b/src/models/tracepointformat.cpp new file mode 100644 index 00000000..735e57d4 --- /dev/null +++ b/src/models/tracepointformat.cpp @@ -0,0 +1,249 @@ +/* + SPDX-FileCopyrightText: Lieven Hey + SPDX-FileCopyrightText: 2024 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#include "tracepointformat.h" + +#include + +extern "C" { +#include +#include +} + +namespace { +Q_LOGGING_CATEGORY(FormatParser, "hotspot.formatparser"); + +auto formatUnsignedNumber(const FormatConversion& format, int base, const QVariant& value) +{ + switch (format.len) { + case FormatConversion::Length::Char: + return QStringLiteral("%1").arg(static_cast(value.toULongLong()), format.width, base, + QLatin1Char('0')); + case FormatConversion::Length::Short: + return QStringLiteral("%1").arg(static_cast(value.toULongLong()), format.width, base, + QLatin1Char('0')); + case FormatConversion::Length::Long: + return QStringLiteral("%1").arg(static_cast(value.toULongLong()), format.width, base, + QLatin1Char('0')); + case FormatConversion::Length::Size: + case FormatConversion::Length::LongLong: + return QStringLiteral("%1").arg(value.toULongLong(), format.width, base, QLatin1Char('0')); + } + Q_UNREACHABLE(); +} + +auto formatSignedNumber(const FormatConversion& format, int base, const QVariant& value) +{ + switch (format.len) { + case FormatConversion::Length::Char: + return QStringLiteral("%1").arg(value.toLongLong() & 0xff, format.width, base, QLatin1Char('0')); + case FormatConversion::Length::Short: + return QStringLiteral("%1").arg(value.toLongLong() & 0xffff, format.width, base, QLatin1Char('0')); + case FormatConversion::Length::Long: + return QStringLiteral("%1").arg(value.toLongLong() & 0xffffffff, format.width, base, QLatin1Char('0')); + case FormatConversion::Length::Size: + case FormatConversion::Length::LongLong: + return QStringLiteral("%1").arg(value.toLongLong(), format.width, base, QLatin1Char('0')); + } + Q_UNREACHABLE(); +} +} + +FormatData parseFormatString(const QString& format) +{ + // try to parse the format string + // if it fails or we encounter unknown flags bail out + auto latin = format.toLatin1().toStdString(); + + const char* str = latin.c_str(); + + fmt_status rc; + fmt_spec spec; + + QVector formats; + QVector qtFormatString; + int formatCounter = 1; + + do { + fmt_spec_init(&spec); + rc = fmt_read_one(&str, &spec); + if (rc == FMT_EOK) { + fmt_spec_print(&spec, stdout); + printf("\n"); + FormatConversion format; + + if (spec.kind == FMT_SPEC_KIND_STRING) { + qtFormatString.append(QByteArray {spec.str_start, static_cast(spec.str_end - spec.str_start)}); + } else { + qtFormatString.append(QStringLiteral("%%1").arg(formatCounter++).toLatin1()); + + switch (static_cast(spec.len)) { + case FMT_SPEC_LEN_hh: + format.len = FormatConversion::Length::Char; + break; + case FMT_SPEC_LEN_h: + format.len = FormatConversion::Length::Short; + break; + case FMT_SPEC_LEN_L: + case FMT_SPEC_LEN_l: + format.len = FormatConversion::Length::Long; + break; + case FMT_SPEC_LEN_ll: + format.len = FormatConversion::Length::LongLong; + break; + case FMT_SPEC_LEN_z: + format.len = FormatConversion::Length::Size; + break; + case FMT_SPEC_LEN_UNKNOWN: + // no length given + break; + default: + qCWarning(FormatParser) << "Failed to parse fmt_spec_len" << spec.len; + return {}; + } + + switch (static_cast(spec.type)) { + case FMT_SPEC_TYPE_X: + format.format = FormatConversion::Format::UpperHex; + break; + case FMT_SPEC_TYPE_x: + format.format = FormatConversion::Format::Hex; + break; + case FMT_SPEC_TYPE_o: + format.format = FormatConversion::Format::Octal; + break; + case FMT_SPEC_TYPE_d: + case FMT_SPEC_TYPE_i: + format.format = FormatConversion::Format::Signed; + break; + case FMT_SPEC_TYPE_u: + format.format = FormatConversion::Format::Unsigned; + break; + case FMT_SPEC_TYPE_c: + format.format = FormatConversion::Format::Char; + break; + case FMT_SPEC_TYPE_p: + format.format = FormatConversion::Format::Pointer; + break; + case FMT_SPEC_TYPE_s: + format.format = FormatConversion::Format::String; + break; + default: + qCWarning(FormatParser) << "Failed to parse fmt_spec_type" << spec.type; + return {}; + } + + if (spec.flags.prepend_zero) { + format.padZeros = true; + format.width = spec.width; + + if (spec.width == FMT_VALUE_OUT_OF_LINE) { + return {}; + } + } + + formats.push_back(format); + } + } + } while (fmt_read_is_ok(rc)); + + return {formats, QString::fromLatin1(qtFormatString.join())}; +} + +QString format(const FormatConversion& format, const QVariant& value) +{ + switch (format.format) { + case FormatConversion::Format::Signed: + return formatSignedNumber(format, 10, value); + case FormatConversion::Format::Unsigned: + return formatUnsignedNumber(format, 10, value); + case FormatConversion::Format::Char: + return value.toChar(); + case FormatConversion::Format::String: + return value.toString(); + case FormatConversion::Format::Pointer: + return QStringLiteral("0x%1").arg(QString::number(value.toULongLong(), 16)); + case FormatConversion::Format::Hex: + return formatUnsignedNumber(format, 16, value); + case FormatConversion::Format::UpperHex: + return formatUnsignedNumber(format, 16, value).toUpper(); + case FormatConversion::Format::Octal: + return formatUnsignedNumber(format, 8, value); + } + return {}; +} + +TracePointFormatter::TracePointFormatter(const QString& format) +{ + // ignore empty format strings + if (format.isEmpty()) { + return; + } + + // the format string are the arguments to a printf call, therefor the format will always be in quotes and then + // follows a list of arguments + auto endOfFormatString = format.indexOf(QLatin1Char('\"'), 1); + + auto formatStringExtracted = format.mid(1, endOfFormatString - 1); + auto formats = parseFormatString(formatStringExtracted); + + const auto args = format.mid(endOfFormatString + 2).split(QLatin1Char(',')); + + // we successfully parsed the string + if (formats.format.size() == args.size()) { + m_formatString = formats.formatString; + for (int i = 0; i < formats.format.size(); i++) { + const auto& arg = args[i]; + auto rec = arg.indexOf(QLatin1String("REC->")); + + if (rec == -1) { + return; + } + + auto closingBracket = arg.indexOf(QLatin1Char(')'), rec); + if (closingBracket == -1) { + closingBracket = arg.length() - 1; + } + // TODO: safeguard this + rec += 5; + m_args.push_back({formats.format[i], arg.mid(rec, closingBracket - rec)}); + } + } +} + +QString TracePointFormatter::format(const Data::TracePointData& data) const +{ + QString result; + + // if m_formatString is empty, we couldn't parse it, just dump out the information + if (m_formatString.isEmpty()) { + for (auto it = data.cbegin(), end = data.cend(); it != end; it++) { + result += QLatin1String("%1: %2\n").arg(it.key(), QString::number(it->toULongLong())); + } + return result.trimmed(); + } + + result = m_formatString; + for (const auto& arg : m_args) { + result = result.arg(::format(arg.format, data.value(arg.name))); + } + + return result; +} + +QString formatTracepoint(const Data::TracePointFormat& format, const Data::TracePointData& data) +{ + static QHash formatterCache; + + const auto name = QStringLiteral("%1:%2").arg(format.systemId, format.nameId); + auto formatter = formatterCache.find(name); + if (formatter == formatterCache.end()) { + formatter = formatterCache.emplace(name, format.format); + } + + return QStringLiteral("%1:\n%2").arg(name, formatter->format(data)); +} diff --git a/src/models/tracepointformat.h b/src/models/tracepointformat.h new file mode 100644 index 00000000..fb7c71a6 --- /dev/null +++ b/src/models/tracepointformat.h @@ -0,0 +1,35 @@ +/* + SPDX-FileCopyrightText: Lieven Hey + SPDX-FileCopyrightText: 2024 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#pragma once + +#include + +#include "data.h" + +class TracePointFormatter +{ +public: + TracePointFormatter(const QString& format); + + QString format(const Data::TracePointData& data) const; + + QString formatString() const + { + return m_formatString; + } + QStringList args() const + { + return m_args; + } + +private: + QString m_formatString; + QStringList m_args; +}; + +QString formatTracepoint(const Data::TracePointFormat& format, const Data::TracePointData& data); diff --git a/tests/modeltests/tst_tracepointformat.cpp b/tests/modeltests/tst_tracepointformat.cpp new file mode 100644 index 00000000..3a8d3c0a --- /dev/null +++ b/tests/modeltests/tst_tracepointformat.cpp @@ -0,0 +1,109 @@ +/* + SPDX-FileCopyrightText: Lieven Hey + SPDX-FileCopyrightText: 2024 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#include + +#if QT_VERSION >= QT_VERSION_CHECK(6, 2, 0) +#include +#endif // QT_VERSION < QT_VERSION_CHECK(6, 2, 0) + +#include + +class TestTracepointFormat : public QObject +{ + Q_OBJECT +private slots: + void initTestCase() + { +#if QT_VERSION < QT_VERSION_CHECK(6, 2, 0) + qSetGlobalQHashSeed(0); +#else + QHashSeed::setDeterministicGlobalSeed(); +#endif // QT_VERSION < QT_VERSION_CHECK(6, 2, 0) + } + + void testFormatString() + { + // taken from /sys/kernel/tracing/events/syscalls/sys_enter_openat/format + auto format = QStringLiteral( + "\"dfd: 0x%08lx, filename: 0x%08lx, flags: 0x%08lx, mode: 0x%08lx\", ((unsigned long)(REC->dfd)), " + "((unsigned long)(REC->filename)), ((unsigned long)(REC->flags)), ((unsigned long)(REC->mode))"); + + TracePointFormatter formatter(format); + + QCOMPARE(formatter.formatString(), + QStringLiteral("dfd: 0x%08lx, filename: 0x%08lx, flags: 0x%08lx, mode: 0x%08lx")); + QCOMPARE(formatter.args(), + (QStringList {{QStringLiteral("dfd")}, + {QStringLiteral("filename")}, + {QStringLiteral("flags")}, + {QStringLiteral("mode")}})); + } + + void testSyscallEnterOpenat() + { + Data::TracePointData tracepointData = {{QStringLiteral("filename"), QVariant(140732347873408ull)}, + {QStringLiteral("dfd"), QVariant(4294967196ull)}, + {QStringLiteral("__syscall_nr"), QVariant(257)}, + {QStringLiteral("flags"), QVariant(0ull)}, + {QStringLiteral("mode"), QVariant(0)}}; + + const Data::TracePointFormat format = { + QStringLiteral("syscalls"), QStringLiteral("syscall_enter_openat"), 0, + QStringLiteral( + "\"dfd: 0x%08lx, filename: 0x%08lx, flags: 0x%08lx, mode: 0x%08lx\", ((unsigned long)(REC->dfd)), " + "((unsigned long)(REC->filename)), ((unsigned long)(REC->flags)), ((unsigned long)(REC->mode))")}; + + TracePointFormatter formatter(format.format); + } + + void testInvalidFormatString_data() + { + QTest::addColumn("format"); + QTest::addRow("Too complex format") << QStringLiteral( + "\"%d,%d %s (%s) %llu + %u %s,%u,%u [%d]\", ((unsigned int) ((REC->dev) >> 20)), ((unsigned int) " + "((REC->dev) & ((1U << 20) - 1))), REC->rwbs, __get_str(cmd), (unsigned long long)REC->sector, " + "REC->nr_sector, __print_symbolic((((REC->ioprio) >> 13) & (8 - 1)), { IOPRIO_CLASS_NONE, \"none\" }, " + "{IOPRIO_CLASS_RT, \"rt\"}, {IOPRIO_CLASS_BE, \"be\"}, {IOPRIO_CLASS_IDLE, \"idle\"}, " + "{IOPRIO_CLASS_INVALID, \"invalid\"}), (((REC->ioprio) >> 3) & ((1 << 10) - 1)), ((REC->ioprio) & ((1 << " + "3) - 1)), REC->error "); + + QTest::addRow("Invalid format string") << QStringLiteral("abc123%s"); + QTest::addRow("Emptry format string") << QString {}; + } + void testInvalidFormatString() + { + QFETCH(QString, format); + + Data::TracePointData data = {{QStringLiteral("ioprio"), QVariant(0)}, + {QStringLiteral("sector"), QVariant(18446744073709551615ull)}, + {QStringLiteral("nr_sector"), QVariant(0u)}, + {QStringLiteral("rwbs"), QVariant(QByteArray("N\x00\x00\x00\x00\x00\x00\x00"))}, + {QStringLiteral("dev"), QVariant(8388624u)}, + {QStringLiteral("cmd"), QVariant(65584u)}, + {QStringLiteral("error"), QVariant(-5)}}; + + TracePointFormatter formatter(format); + QVERIFY(formatter.formatString().isEmpty()); + + // if the format string cannot be decoded then for formatter will just concat the tracepoint data + // Qt5 and Qt6 use different hashing functions so we need two different outputs +#if QT_VERSION < QT_VERSION_CHECK(6, 2, 0) + auto output = QLatin1String("dev: 8388624\ncmd: 65584\nnr_sector: 0\nrwbs: 0\nioprio: 0\nerror: " + "18446744073709551611\nsector: 18446744073709551615"); +#else + auto output = QLatin1String("cmd: 65584\nioprio: 0\nnr_sector: 0\nrwbs: 0\nsector: 18446744073709551615\ndev: " + "8388624\nerror: 18446744073709551611"); +#endif // QT_VERSION < QT_VERSION_CHECK(6, 2, 0) + + QCOMPARE(formatter.format(data), output); + } +}; + +QTEST_GUILESS_MAIN(TestTracepointFormat) + +#include "tst_tracepointformat.moc" From 861311b2d014b111ae6d707f64b1582b092f624b Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Mon, 2 Dec 2024 13:01:58 +0100 Subject: [PATCH 10/13] feat: correctly parse format string from tracepoint definition This patch allows hotspot to correctly parse most format string from the tracepoint definition. If that fails every entry in the tracepoint will be printed unformatted. In this case it is possible to add a custom formatter. --- .gitmodules | 3 + 3rdparty/CMakeLists.txt | 1 + 3rdparty/fmtparser | 1 + src/models/CMakeLists.txt | 1 + src/models/timelinedelegate.cpp | 12 +- src/models/tracepointformat.cpp | 14 +-- src/models/tracepointformat.h | 54 ++++++++- tests/modeltests/tst_tracepointformat.cpp | 128 ++++++++++++++++++++-- 8 files changed, 190 insertions(+), 24 deletions(-) create mode 160000 3rdparty/fmtparser diff --git a/.gitmodules b/.gitmodules index ef967ea1..fd8e191d 100644 --- a/.gitmodules +++ b/.gitmodules @@ -5,3 +5,6 @@ [submodule "3rdparty/PrefixTickLabels"] path = 3rdparty/PrefixTickLabels url = https://github.com/koenpoppe/PrefixTickLabels +[submodule "3rdparty/fmtparser"] + path = 3rdparty/fmtparser + url = https://github.com/fmtparser/fmtparser diff --git a/3rdparty/CMakeLists.txt b/3rdparty/CMakeLists.txt index 5587fbb8..b9bbf051 100644 --- a/3rdparty/CMakeLists.txt +++ b/3rdparty/CMakeLists.txt @@ -1,2 +1,3 @@ include(perfparser.cmake) include(PrefixTickLabels.cmake) +add_subdirectory(fmtparser) diff --git a/3rdparty/fmtparser b/3rdparty/fmtparser new file mode 160000 index 00000000..c10a48ce --- /dev/null +++ b/3rdparty/fmtparser @@ -0,0 +1 @@ +Subproject commit c10a48ce42819e78c68548c6b91845141e3a3082 diff --git a/src/models/CMakeLists.txt b/src/models/CMakeLists.txt index 780b1dbf..5b4fd935 100644 --- a/src/models/CMakeLists.txt +++ b/src/models/CMakeLists.txt @@ -29,6 +29,7 @@ add_library( target_link_libraries( models + fmt_parser Qt::Core Qt::Widgets KF${QT_MAJOR_VERSION}::ItemModels diff --git a/src/models/timelinedelegate.cpp b/src/models/timelinedelegate.cpp index a542f7e4..13a3477d 100644 --- a/src/models/timelinedelegate.cpp +++ b/src/models/timelinedelegate.cpp @@ -8,7 +8,6 @@ #include "timelinedelegate.h" #include -#include #include #include #include @@ -346,15 +345,12 @@ bool TimeLineDelegate::helpEvent(QHelpEvent* event, QAbstractItemView* view, con }); const auto format = results.tracePointFormats[tracepoint.tracepointFormat]; - qDebug() << format.systemId << format.nameId << format.format; - qDebug() << results.tracePointData[tracepoint.tracepointData]; - TracePointFormatter formatter(format.format); + auto tracepointFormatted = format.nameId.isEmpty() + ? QStringLiteral("PerfParser does not support tracepoints") + : formatTracepoint(format, results.tracePointData[tracepoint.tracepointData]); - QToolTip::showText(event->globalPos(), - tr("time: %1\n%2:\n%3") - .arg(formattedTime, results.tracepoints[index.row()].name, - formatter.format(results.tracePointData[tracepoint.tracepointData]))); + QToolTip::showText(event->globalPos(), tr("time: %1\n%2").arg(formattedTime, tracepointFormatted)); } } else { diff --git a/src/models/tracepointformat.cpp b/src/models/tracepointformat.cpp index 735e57d4..45cd591f 100644 --- a/src/models/tracepointformat.cpp +++ b/src/models/tracepointformat.cpp @@ -17,7 +17,7 @@ extern "C" { namespace { Q_LOGGING_CATEGORY(FormatParser, "hotspot.formatparser"); -auto formatUnsignedNumber(const FormatConversion& format, int base, const QVariant& value) +auto formatUnsignedNumber(FormatConversion format, int base, const QVariant& value) { switch (format.len) { case FormatConversion::Length::Char: @@ -36,15 +36,16 @@ auto formatUnsignedNumber(const FormatConversion& format, int base, const QVaria Q_UNREACHABLE(); } -auto formatSignedNumber(const FormatConversion& format, int base, const QVariant& value) +auto formatSignedNumber(FormatConversion format, int base, const QVariant& value) { switch (format.len) { case FormatConversion::Length::Char: - return QStringLiteral("%1").arg(value.toLongLong() & 0xff, format.width, base, QLatin1Char('0')); + return QStringLiteral("%1").arg(static_cast(value.toLongLong() & 0xff), format.width, base, + QLatin1Char('0')); case FormatConversion::Length::Short: - return QStringLiteral("%1").arg(value.toLongLong() & 0xffff, format.width, base, QLatin1Char('0')); + return QStringLiteral("%1").arg(static_cast(value.toLongLong()), format.width, base, QLatin1Char('0')); case FormatConversion::Length::Long: - return QStringLiteral("%1").arg(value.toLongLong() & 0xffffffff, format.width, base, QLatin1Char('0')); + return QStringLiteral("%1").arg(static_cast(value.toLongLong()), format.width, base, QLatin1Char('0')); case FormatConversion::Length::Size: case FormatConversion::Length::LongLong: return QStringLiteral("%1").arg(value.toLongLong(), format.width, base, QLatin1Char('0')); @@ -154,7 +155,7 @@ FormatData parseFormatString(const QString& format) return {formats, QString::fromLatin1(qtFormatString.join())}; } -QString format(const FormatConversion& format, const QVariant& value) +QString format(FormatConversion format, const QVariant& value) { switch (format.format) { case FormatConversion::Format::Signed: @@ -231,7 +232,6 @@ QString TracePointFormatter::format(const Data::TracePointData& data) const for (const auto& arg : m_args) { result = result.arg(::format(arg.format, data.value(arg.name))); } - return result; } diff --git a/src/models/tracepointformat.h b/src/models/tracepointformat.h index fb7c71a6..b12a3cdb 100644 --- a/src/models/tracepointformat.h +++ b/src/models/tracepointformat.h @@ -11,6 +11,48 @@ #include "data.h" +struct FormatConversion +{ + Q_GADGET +public: + enum class Length + { + Char, + Short, + Long, + LongLong, + Size + }; + Q_ENUM(Length) + Length len = Length::Long; + + enum class Format + { + Signed, + Unsigned, + Char, + String, + Pointer, + Hex, + UpperHex, + Octal, + }; + Q_ENUM(Format); + Format format = Format::Signed; + + bool padZeros = false; + int width = 0; +}; + +struct FormatData +{ + QVector format; + QString formatString; +}; + +FormatData parseFormatString(const QString& format); +QString format(FormatConversion format, const QVariant& value); + class TracePointFormatter { public: @@ -22,14 +64,22 @@ class TracePointFormatter { return m_formatString; } - QStringList args() const + + struct Arg + { + FormatConversion format; + QString name; + }; + using Arglist = QVector; + + Arglist args() const { return m_args; } private: QString m_formatString; - QStringList m_args; + Arglist m_args; }; QString formatTracepoint(const Data::TracePointFormat& format, const Data::TracePointData& data); diff --git a/tests/modeltests/tst_tracepointformat.cpp b/tests/modeltests/tst_tracepointformat.cpp index 3a8d3c0a..fe0cbb2a 100644 --- a/tests/modeltests/tst_tracepointformat.cpp +++ b/tests/modeltests/tst_tracepointformat.cpp @@ -13,6 +13,17 @@ #include +bool operator==(const FormatConversion& lhs, const FormatConversion& rhs) +{ + return std::tie(lhs.format, lhs.len, lhs.padZeros, lhs.width) + == std::tie(rhs.format, rhs.len, rhs.padZeros, rhs.width); +} + +bool operator==(const TracePointFormatter::Arg& lhs, const TracePointFormatter::Arg& rhs) +{ + return std::tie(lhs.format, lhs.name) == std::tie(rhs.format, rhs.name); +} + class TestTracepointFormat : public QObject { Q_OBJECT @@ -26,6 +37,106 @@ private slots: #endif // QT_VERSION < QT_VERSION_CHECK(6, 2, 0) } + void testFormatStringParser() + { + auto check = [](const QString& format, const FormatConversion& exspected) { + // parseFormatString exspects the raw format string from the tracepoint format + auto conversion = parseFormatString(format).format; + QVERIFY(!conversion.isEmpty()); + QCOMPARE(conversion[0], exspected); + }; + { + FormatConversion format; + format.format = FormatConversion::Format::Hex; + check(QStringLiteral("%x"), format); + } + { + FormatConversion format; + format.format = FormatConversion::Format::UpperHex; + check(QStringLiteral("%X"), format); + } + { + FormatConversion format; + format.format = FormatConversion::Format::Octal; + check(QStringLiteral("%o"), format); + } + { + FormatConversion format; + format.format = FormatConversion::Format::Signed; + check(QStringLiteral("%d"), format); + check(QStringLiteral("%i"), format); + } + { + FormatConversion format; + format.format = FormatConversion::Format::Char; + check(QStringLiteral("%c"), format); + } + { + FormatConversion format; + format.format = FormatConversion::Format::Pointer; + check(QStringLiteral("%p"), format); + } + { + FormatConversion format; + format.format = FormatConversion::Format::String; + check(QStringLiteral("%s"), format); + } + + { + FormatConversion format; + format.format = FormatConversion::Format::UpperHex; + format.len = FormatConversion::Length::LongLong; + check(QStringLiteral("%llX"), format); + } + + { + FormatConversion format; + format.format = FormatConversion::Format::Signed; + format.len = FormatConversion::Length::Long; + check(QStringLiteral("%ld"), format); + } + + { + FormatConversion format; + format.format = FormatConversion::Format::Unsigned; + format.len = FormatConversion::Length::Short; + check(QStringLiteral("%hu"), format); + } + } + + void testFormatting() + { + auto test = [](const QString& formatString, auto value) { + auto formats = parseFormatString(formatString).format; + QVERIFY(!formats.isEmpty()); + QCOMPARE(format(formats[0], QVariant::fromValue(value)), + QString::asprintf(formatString.toLatin1().data(), value)); + }; + + test(QStringLiteral("%x"), 16); + test(QStringLiteral("%X"), 255); + test(QStringLiteral("%hhX"), 255); + test(QStringLiteral("%o"), 255); + test(QStringLiteral("%c"), 'a'); + test(QStringLiteral("%i"), -10); + test(QStringLiteral("%i"), LONG_LONG_MAX); + test(QStringLiteral("%u"), LONG_LONG_MAX); + + int x = 0; + // we get pointers as a quint64 + test(QStringLiteral("%p"), reinterpret_cast(&x)); + + test(QStringLiteral("%04u"), 5); + test(QStringLiteral("%04i"), -5); + } + + void testNotParsable() + { + // some tracepoint format strings cant be parsed trivial + QVERIFY(parseFormatString(QStringLiteral("%0*llx")).format.isEmpty()); + QVERIFY(parseFormatString(QStringLiteral("%+05")).format.isEmpty()); + } + void testFormatString() { // taken from /sys/kernel/tracing/events/syscalls/sys_enter_openat/format @@ -35,13 +146,16 @@ private slots: TracePointFormatter formatter(format); - QCOMPARE(formatter.formatString(), - QStringLiteral("dfd: 0x%08lx, filename: 0x%08lx, flags: 0x%08lx, mode: 0x%08lx")); + QCOMPARE(formatter.formatString(), QStringLiteral("dfd: 0x%1, filename: 0x%2, flags: 0x%3, mode: 0x%4")); + + const auto formatDefinition = + FormatConversion {FormatConversion::Length::Long, FormatConversion::Format::Hex, true, 8}; + QCOMPARE(formatter.args(), - (QStringList {{QStringLiteral("dfd")}, - {QStringLiteral("filename")}, - {QStringLiteral("flags")}, - {QStringLiteral("mode")}})); + (TracePointFormatter::Arglist {{formatDefinition, QStringLiteral("dfd")}, + {formatDefinition, QStringLiteral("filename")}, + {formatDefinition, QStringLiteral("flags")}, + {formatDefinition, QStringLiteral("mode")}})); } void testSyscallEnterOpenat() @@ -72,7 +186,7 @@ private slots: "{IOPRIO_CLASS_INVALID, \"invalid\"}), (((REC->ioprio) >> 3) & ((1 << 10) - 1)), ((REC->ioprio) & ((1 << " "3) - 1)), REC->error "); - QTest::addRow("Invalid format string") << QStringLiteral("abc123%s"); + QTest::addRow("Invalid format string") << QStringLiteral("abc%123k"); QTest::addRow("Emptry format string") << QString {}; } void testInvalidFormatString() From 41583429454c5106259e04bf12f1c56c86f2fa82 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Thu, 27 Nov 2025 14:23:03 +0100 Subject: [PATCH 11/13] fix: silence warnings from submodule --- 3rdparty/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/3rdparty/CMakeLists.txt b/3rdparty/CMakeLists.txt index b9bbf051..1bf1ea2b 100644 --- a/3rdparty/CMakeLists.txt +++ b/3rdparty/CMakeLists.txt @@ -1,3 +1,7 @@ include(perfparser.cmake) include(PrefixTickLabels.cmake) add_subdirectory(fmtparser) + +set(CMAKE_CSTANDARD 11) +set(CMAKE_C_FLAGS "-Wno-error") +target_compile_options(fmt_parser PRIVATE -Wno-pedantic -Wno-address -Wno-unused-variable -Wno-error) From c9349c5d9cb67f2784ef2b050f863c9b6ac5a3fd Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Thu, 27 Nov 2025 15:48:57 +0100 Subject: [PATCH 12/13] fix: errors in fmtparser --- .gitmodules | 2 +- 3rdparty/fmtparser | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitmodules b/.gitmodules index fd8e191d..4f85bb78 100644 --- a/.gitmodules +++ b/.gitmodules @@ -7,4 +7,4 @@ url = https://github.com/koenpoppe/PrefixTickLabels [submodule "3rdparty/fmtparser"] path = 3rdparty/fmtparser - url = https://github.com/fmtparser/fmtparser + url = git@github.com:lievenhey/fmtparser.git diff --git a/3rdparty/fmtparser b/3rdparty/fmtparser index c10a48ce..6e2e6a25 160000 --- a/3rdparty/fmtparser +++ b/3rdparty/fmtparser @@ -1 +1 @@ -Subproject commit c10a48ce42819e78c68548c6b91845141e3a3082 +Subproject commit 6e2e6a2513bbb4c6449eac2a84dd5c7b029d9164 From a7c66cce41cd5c6073908b247d2ce7fdd6807bbc Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Thu, 27 Nov 2025 17:08:46 +0100 Subject: [PATCH 13/13] fix: add --- .github/workflows/compile-and-test.yml | 4 ++-- 3rdparty/fmtparser | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/compile-and-test.yml b/.github/workflows/compile-and-test.yml index e8502656..ff5d1e0c 100644 --- a/.github/workflows/compile-and-test.yml +++ b/.github/workflows/compile-and-test.yml @@ -98,10 +98,10 @@ jobs: submodules: recursive - name: Configure - run: cmake --preset dev-clazy-qt6 + run: cmake --trace-expand --preset dev-clazy-qt6 - name: Build - run: cmake --build --preset dev-clazy-qt6 + run: cmake --trace-expand --build --preset dev-clazy-qt6 - name: Test run: ctest --preset dev-clazy-qt6 diff --git a/3rdparty/fmtparser b/3rdparty/fmtparser index 6e2e6a25..36df3332 160000 --- a/3rdparty/fmtparser +++ b/3rdparty/fmtparser @@ -1 +1 @@ -Subproject commit 6e2e6a2513bbb4c6449eac2a84dd5c7b029d9164 +Subproject commit 36df3332ae1a7bba656f29a7c27633df8e5db602