From 58d3fc18db6fac921150c42be68e46bda2d1a9e1 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Fri, 4 Oct 2024 13:35:25 +0200 Subject: [PATCH] Valgrind: Dismantle Parser class Replace it with parseDataFile() function. Change-Id: I21ee1e5ffa2dba94501c7d8e258ac2c5478af633 Reviewed-by: hjk --- .../valgrind/callgrind/callgrindparser.cpp | 60 ++++++------------- .../valgrind/callgrind/callgrindparser.h | 22 +------ src/plugins/valgrind/callgrindengine.cpp | 15 +---- src/plugins/valgrind/callgrindengine.h | 5 +- src/plugins/valgrind/callgrindtool.cpp | 12 +--- .../callgrind/callgrindparsertests.cpp | 12 +--- 6 files changed, 28 insertions(+), 98 deletions(-) diff --git a/src/plugins/valgrind/callgrind/callgrindparser.cpp b/src/plugins/valgrind/callgrind/callgrindparser.cpp index 764b0104674..a9977f76cdc 100644 --- a/src/plugins/valgrind/callgrind/callgrindparser.cpp +++ b/src/plugins/valgrind/callgrind/callgrindparser.cpp @@ -110,17 +110,10 @@ static int parseNameShorthand(const char **current, const char *end) } -class Parser::Private +class ParserPrivate { - Parser *const q; public: - - explicit Private(Parser *qq) - : q(qq) - { - } - - void parse(const FilePath &filePath); + ParseDataPtr parse(const FilePath &filePath); void parseHeader(QIODevice *device); using NamePair = QPair; @@ -167,7 +160,7 @@ public: QSet recursiveFunctions; }; -void Parser::Private::parse(const FilePath &filePath) +ParseDataPtr ParserPrivate::parse(const FilePath &filePath) { const QString path = filePath.path(); // FIXME: Works only accidentally for docker QFile file(path); @@ -233,8 +226,7 @@ void Parser::Private::parse(const FilePath &filePath) // now accumulate callees for (Function *func : std::as_const(pendingFunctions)) func->finalize(); - - emit q->parserDataReady(); + return data; } inline QString getValue(const QByteArray &line, const int prefixLength) @@ -244,7 +236,7 @@ inline QString getValue(const QByteArray &line, const int prefixLength) return QString::fromLatin1(line.mid(prefixLength, line.length() - 1 - prefixLength).constData()); } -void Parser::Private::parseHeader(QIODevice *device) +void ParserPrivate::parseHeader(QIODevice *device) { QTC_ASSERT(device->isOpen(), return); QTC_ASSERT(device->isReadable(), return); @@ -301,7 +293,7 @@ void Parser::Private::parseHeader(QIODevice *device) } } -Parser::Private::NamePair Parser::Private::parseName(const char *begin, const char *end) +ParserPrivate::NamePair ParserPrivate::parseName(const char *begin, const char *end) { const char *current = begin; qint64 nameShorthand = -1; @@ -325,7 +317,7 @@ Parser::Private::NamePair Parser::Private::parseName(const char *begin, const ch * cfn means called function */ -void Parser::Private::dispatchLine(const QByteArray &line) +void ParserPrivate::dispatchLine(const QByteArray &line) { int lineEnding = line.endsWith("\r\n") ? 2 : 1; const char *const begin = line.constData(); @@ -408,7 +400,7 @@ void Parser::Private::dispatchLine(const QByteArray &line) } } -void Parser::Private::parseCostItem(const char *begin, const char *end) +void ParserPrivate::parseCostItem(const char *begin, const char *end) { QTC_ASSERT(currentFunction, return); @@ -504,7 +496,7 @@ void Parser::Private::parseCostItem(const char *begin, const char *end) currentFunction->addCostItem(costItem); } -void Parser::Private::parseSourceFile(const char *begin, const char *end) +void ParserPrivate::parseSourceFile(const char *begin, const char *end) { NamePair name = parseName(begin, end); @@ -518,7 +510,7 @@ void Parser::Private::parseSourceFile(const char *begin, const char *end) currentDifferingFile = -1; } -void Parser::Private::parseFunction(const char *begin, const char *end) +void ParserPrivate::parseFunction(const char *begin, const char *end) { currentFunction = new Function(data.get()); currentFunction->setFile(lastFile); @@ -534,7 +526,7 @@ void Parser::Private::parseFunction(const char *begin, const char *end) currentFunction->setName(name.first); } -void Parser::Private::parseDifferingSourceFile(const char *begin, const char *end) +void ParserPrivate::parseDifferingSourceFile(const char *begin, const char *end) { NamePair name = parseName(begin, end); @@ -550,7 +542,7 @@ void Parser::Private::parseDifferingSourceFile(const char *begin, const char *en currentDifferingFile = name.first; } -void Parser::Private::parseObjectFile(const char *begin, const char *end) +void ParserPrivate::parseObjectFile(const char *begin, const char *end) { NamePair name = parseName(begin, end); if (!name.second.isEmpty()) @@ -559,7 +551,7 @@ void Parser::Private::parseObjectFile(const char *begin, const char *end) lastObject = name.first; } -void Parser::Private::parseCalls(const char *begin, const char *end) +void ParserPrivate::parseCalls(const char *begin, const char *end) { const char *current = begin; bool ok; @@ -577,7 +569,7 @@ void Parser::Private::parseCalls(const char *begin, const char *end) isParsingFunctionCall = true; } -void Parser::Private::parseCalledFunction(const char *begin, const char *end) +void ParserPrivate::parseCalledFunction(const char *begin, const char *end) { NamePair name = parseName(begin, end); if (!name.second.isEmpty()) @@ -586,7 +578,7 @@ void Parser::Private::parseCalledFunction(const char *begin, const char *end) currentCallData.calledFunction = name.first; } -void Parser::Private::parseCalledSourceFile(const char *begin, const char *end) +void ParserPrivate::parseCalledSourceFile(const char *begin, const char *end) { NamePair name = parseName(begin, end); if (!name.second.isEmpty()) { @@ -598,7 +590,7 @@ void Parser::Private::parseCalledSourceFile(const char *begin, const char *end) currentCallData.calledFile = name.first; } -void Parser::Private::parseCalledObjectFile(const char *begin, const char *end) +void ParserPrivate::parseCalledObjectFile(const char *begin, const char *end) { NamePair name = parseName(begin, end); if (!name.second.isEmpty()) @@ -609,24 +601,10 @@ void Parser::Private::parseCalledObjectFile(const char *begin, const char *end) //BEGIN Parser -void Parser::parse(const Utils::FilePath &filePath) +ParseDataPtr parseDataFile(const Utils::FilePath &filePath) { - d->parse(filePath); -} - -Parser::Parser() - : d(new Private(this)) -{ -} - -Parser::~Parser() -{ - delete d; -} - -ParseDataPtr Parser::parserData() const -{ - return d->data; + ParserPrivate parser; + return parser.parse(filePath); } } // namespace Valgrind::Callgrind diff --git a/src/plugins/valgrind/callgrind/callgrindparser.h b/src/plugins/valgrind/callgrind/callgrindparser.h index f6211a9aae0..43058827047 100644 --- a/src/plugins/valgrind/callgrind/callgrindparser.h +++ b/src/plugins/valgrind/callgrind/callgrindparser.h @@ -11,8 +11,6 @@ namespace Utils { class FilePath; } namespace Valgrind::Callgrind { -class ParseData; - /** * Parser for Valgrind --tool=callgrind output * most of the format is documented at http://kcachegrind.sourceforge.net/html/CallgrindFormat.html @@ -22,25 +20,7 @@ class ParseData; * the rest is assumed to be zero." * */ -class Parser : public QObject -{ - Q_OBJECT -public: - Parser(); - ~Parser() override; - - // get and take ownership of the parsing results. If this function is not called the repository - // will be destroyed when the parser is destroyed. Subsequent calls return null. - ParseDataPtr parserData() const; - void parse(const Utils::FilePath &filePath); - -signals: - void parserDataReady(); - -private: - class Private; - Private *const d; -}; +ParseDataPtr parseDataFile(const Utils::FilePath &filePath); } // namespace Valgrind::Callgrind diff --git a/src/plugins/valgrind/callgrindengine.cpp b/src/plugins/valgrind/callgrindengine.cpp index d0fa47853cb..4d6c911df60 100644 --- a/src/plugins/valgrind/callgrindengine.cpp +++ b/src/plugins/valgrind/callgrindengine.cpp @@ -34,13 +34,7 @@ CallgrindToolRunner::CallgrindToolRunner(RunControl *runControl) connect(&m_runner, &ValgrindProcess::valgrindStarted, this, [this](qint64 pid) { m_pid = pid; }); - connect(&m_runner, &ValgrindProcess::done, this, [this] { - triggerParse(); - emit parserDataReady(this); - }); - connect(&m_parser, &Callgrind::Parser::parserDataReady, this, [this] { - emit parserDataReady(this); - }); + connect(&m_runner, &ValgrindProcess::done, this, &CallgrindToolRunner::triggerParse); m_valgrindRunnable = runControl->runnable(); @@ -117,11 +111,6 @@ void CallgrindToolRunner::setToggleCollectFunction(const QString &toggleCollectF m_argumentForToggleCollect = "--toggle-collect=" + toggleCollectFunction; } -Callgrind::ParseDataPtr CallgrindToolRunner::parserData() const -{ - return m_parser.parserData(); -} - void CallgrindToolRunner::showStatusMessage(const QString &message) { Debugger::showPermanentStatusMessage(message); @@ -253,7 +242,7 @@ void CallgrindToolRunner::triggerParse() if (!res) // failed to run callgrind return; showStatusMessage(Tr::tr("Parsing Profile Data...")); - m_parser.parse(m_hostOutputFile); + emit parserDataReady(parseDataFile(m_hostOutputFile)); }; // TODO: Store the handle and cancel on CallgrindToolRunner destructor? // TODO: Should d'tor of context object cancel the running task? diff --git a/src/plugins/valgrind/callgrindengine.h b/src/plugins/valgrind/callgrindengine.h index f5720d2ba0c..82db9bac378 100644 --- a/src/plugins/valgrind/callgrindengine.h +++ b/src/plugins/valgrind/callgrindengine.h @@ -23,8 +23,6 @@ public: void start() override; - Callgrind::ParseDataPtr parserData() const; - /// controller actions void dump() { run(Dump); } void reset() { run(ResetEventCounters); } @@ -52,7 +50,7 @@ protected: QString progressTitle() const override; signals: - void parserDataReady(CallgrindToolRunner *engine); + void parserDataReady(const Callgrind::ParseDataPtr &data); private: void showStatusMessage(const QString &message); @@ -83,7 +81,6 @@ private: Utils::FilePath m_valgrindOutputFile; // On the device that runs valgrind Utils::FilePath m_hostOutputFile; // On the device that runs creator - Callgrind::Parser m_parser; bool m_paused = false; QString m_argumentForToggleCollect; diff --git a/src/plugins/valgrind/callgrindtool.cpp b/src/plugins/valgrind/callgrindtool.cpp index adbc052e070..3d3c42832c9 100644 --- a/src/plugins/valgrind/callgrindtool.cpp +++ b/src/plugins/valgrind/callgrindtool.cpp @@ -137,7 +137,6 @@ public: void visualisationFunctionSelected(const Function *function); void showParserResults(const ParseDataPtr &data); - void takeParserDataFromRunControl(CallgrindToolRunner *rc); void setParserData(const ParseDataPtr &data); void doSetParseData(const ParseDataPtr &data); void engineFinished(); @@ -710,7 +709,7 @@ void CallgrindTool::setupRunner(CallgrindToolRunner *toolRunner) { RunControl *runControl = toolRunner->runControl(); - connect(toolRunner, &CallgrindToolRunner::parserDataReady, this, &CallgrindTool::takeParserDataFromRunControl); + connect(toolRunner, &CallgrindToolRunner::parserDataReady, this, &CallgrindTool::setParserData); connect(runControl, &RunControl::stopped, this, &CallgrindTool::engineFinished); connect(this, &CallgrindTool::dumpRequested, toolRunner, &CallgrindToolRunner::dump); @@ -870,14 +869,7 @@ void CallgrindTool::loadExternalLogFile() Debugger::showPermanentStatusMessage(Tr::tr("Parsing Profile Data...")); QCoreApplication::processEvents(); - Parser parser; - parser.parse(filePath); - setParserData(parser.parserData()); -} - -void CallgrindTool::takeParserDataFromRunControl(CallgrindToolRunner *rc) -{ - setParserData(rc->parserData()); + setParserData(parseDataFile(filePath)); } void CallgrindTool::setParserData(const ParseDataPtr &data) diff --git a/tests/auto/valgrind/callgrind/callgrindparsertests.cpp b/tests/auto/valgrind/callgrind/callgrindparsertests.cpp index 76d5282fb66..cc90f0b20ca 100644 --- a/tests/auto/valgrind/callgrind/callgrindparsertests.cpp +++ b/tests/auto/valgrind/callgrind/callgrindparsertests.cpp @@ -21,9 +21,10 @@ using namespace Valgrind::Callgrind; namespace { -static QString dataFile(const char *file) +static Utils::FilePath dataFile(const char *file) { - return QLatin1String(PARSERTESTS_DATA_DIR) + QLatin1String("/") + QLatin1String(file); + return Utils::FilePath::fromString( + QLatin1String(PARSERTESTS_DATA_DIR) + QLatin1String("/") + QLatin1String(file)); } void testCostItem(const CostItem *item, quint64 expectedPosition, quint64 expectedCost) @@ -80,13 +81,6 @@ void CallgrindParserTests::cleanup() { } -ParseDataPtr parseDataFile(const QString &dataFile) -{ - Parser p; - p.parse(Utils::FilePath::fromString(dataFile)); - return p.parserData(); -} - void CallgrindParserTests::testHeaderData() { const ParseDataPtr data(parseDataFile(dataFile("simpleFunction.out")));