From c7d8b9b01c140a44abf2c93a4354ac0f66a68590 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Fri, 31 Jan 2020 15:52:20 +0100 Subject: [PATCH] QMakeProjectManager: Move some parsing code out of the UI thread When parsing larger qmake project, the callbacks from the parser threads are currently overloading the UI thread, often rendering the application non-responsive until the project is completely loaded. This patch moves some expensive operations from the UI thread into the parser threads, at the cost of a somewhat ugly two-stage setup for some types of objects. On my Linux machine, I measured that the time spent in parser callback code went down by almost 50% when loading the Qt Creator super project. Task-number: QTCREATORBUG-18533 Change-Id: If9624da5b07e81a50c180693580b20a70e1aaea7 Reviewed-by: hjk --- .../qmakeprojectmanager/qmakeparsernodes.cpp | 150 +++++++++++------- .../qmakeprojectmanager/qmakeparsernodes.h | 8 +- .../qmakeprojectmanager/qmakeproject.cpp | 24 +-- 3 files changed, 113 insertions(+), 69 deletions(-) diff --git a/src/plugins/qmakeprojectmanager/qmakeparsernodes.cpp b/src/plugins/qmakeprojectmanager/qmakeparsernodes.cpp index b8d2ab3afc7..a226ca6400f 100644 --- a/src/plugins/qmakeprojectmanager/qmakeparsernodes.cpp +++ b/src/plugins/qmakeprojectmanager/qmakeparsernodes.cpp @@ -118,6 +118,8 @@ public: QtSupport::ProFileReader *readerCumulative; QMakeGlobals *qmakeGlobals; QMakeVfs *qmakeVfs; + QSet parentFilePaths; + bool includedInExcactParse; }; class QmakePriFileEvalResult @@ -146,6 +148,8 @@ public: class QmakeEvalResult { public: + ~QmakeEvalResult() { qDeleteAll(directChildren); } + enum EvalResultState { EvalAbort, EvalFail, EvalPartial, EvalOk }; EvalResultState state; ProjectType projectType; @@ -158,23 +162,33 @@ public: QHash newVarValues; QStringList errors; QSet directoriesWithWildcards; + QList directChildren; + QList> priFiles; + QList proFiles; }; } // namespace Internal QmakePriFile::QmakePriFile(QmakeBuildSystem *buildSystem, QmakeProFile *qmakeProFile, - const FilePath &filePath) : - m_buildSystem(buildSystem), - m_qmakeProFile(qmakeProFile) + const FilePath &filePath) : m_filePath(filePath) { - Q_ASSERT(buildSystem); - m_priFileDocument = std::make_unique(this, filePath); + finishInitialization(buildSystem, qmakeProFile); +} + +QmakePriFile::QmakePriFile(const FilePath &filePath) : m_filePath(filePath) { } + +void QmakePriFile::finishInitialization(QmakeBuildSystem *buildSystem, QmakeProFile *qmakeProFile) +{ + QTC_ASSERT(buildSystem, return); + m_buildSystem = buildSystem; + m_qmakeProFile = qmakeProFile; + m_priFileDocument = std::make_unique(this, filePath()); Core::DocumentManager::addDocument(m_priFileDocument.get()); } FilePath QmakePriFile::filePath() const { - return m_priFileDocument->filePath(); + return m_filePath; } FilePath QmakePriFile::directoryPath() const @@ -1180,23 +1194,30 @@ QByteArray QmakeProFile::cxxDefines() const QmakeProFile::QmakeProFile(QmakeBuildSystem *buildSystem, const FilePath &filePath) : QmakePriFile(buildSystem, this, filePath) { - // The lifetime of the m_parserFutureWatcher is shorter - // than of this, so this is all safe - QObject::connect(&m_parseFutureWatcher, &QFutureWatcherBase::finished, - [this](){ applyAsyncEvaluate(); }); + setupFutureWatcher(); } +QmakeProFile::QmakeProFile(const FilePath &filePath) : QmakePriFile(filePath) { } + QmakeProFile::~QmakeProFile() { qDeleteAll(m_extraCompilers); - m_parseFutureWatcher.cancel(); - m_parseFutureWatcher.waitForFinished(); + m_parseFutureWatcher->cancel(); + m_parseFutureWatcher->waitForFinished(); + delete m_parseFutureWatcher; if (m_readerExact) applyAsyncEvaluate(); cleanupProFileReaders(); } +void QmakeProFile::setupFutureWatcher() +{ + m_parseFutureWatcher = new QFutureWatcher; + QObject::connect(m_parseFutureWatcher, &QFutureWatcherBase::finished, + [this](){ applyAsyncEvaluate(); }); +} + bool QmakeProFile::isParent(QmakeProFile *node) { while ((node = dynamic_cast(node->parent()))) { @@ -1287,13 +1308,13 @@ void QmakeProFile::asyncUpdate() setupReader(); if (!includedInExactParse()) m_readerExact->setExact(false); - m_parseFutureWatcher.waitForFinished(); + m_parseFutureWatcher->waitForFinished(); QmakeEvalInput input = evalInput(); QFuture future = Utils::runAsync(ProjectExplorerPlugin::sharedThreadPool(), QThread::LowestPriority, &QmakeProFile::asyncEvaluate, this, input); - m_parseFutureWatcher.setFuture(future); + m_parseFutureWatcher->setFuture(future); } bool QmakeProFile::isFileFromWildcard(const QString &filePath) const @@ -1315,6 +1336,9 @@ QmakeEvalInput QmakeProFile::evalInput() const input.readerCumulative = m_readerCumulative; input.qmakeGlobals = m_buildSystem->qmakeGlobals(); input.qmakeVfs = m_buildSystem->qmakeVfs(); + input.includedInExcactParse = includedInExactParse(); + for (const QmakePriFile *pri = this; pri; pri = pri->parent()) + input.parentFilePaths.insert(pri->filePath()); return input; } @@ -1580,6 +1604,47 @@ QmakeEvalResult *QmakeProFile::evaluate(const QmakeEvalInput &input) if (cumulativeBuildPassReader && cumulativeBuildPassReader != input.readerCumulative) delete cumulativeBuildPassReader; + QList> toCompare; + toCompare.append(qMakePair(nullptr, &result->includedFiles)); + while (!toCompare.isEmpty()) { + QmakePriFile *pn = toCompare.first().first; + QmakeIncludedPriFile *tree = toCompare.first().second; + toCompare.pop_front(); + + // Loop prevention: Make sure that exact same node is not in our parent chain + for (QmakeIncludedPriFile *priFile : tree->children) { + bool loop = input.parentFilePaths.contains(priFile->name); + for (const QmakePriFile *n = pn; n && !loop; n = n->parent()) { + if (n->filePath() == priFile->name) + loop = true; + } + if (loop) + continue; // Do nothing + + if (priFile->proFile) { + auto *qmakePriFileNode = new QmakePriFile(priFile->name); + if (pn) + pn->addChild(qmakePriFileNode); + else + result->directChildren << qmakePriFileNode; + qmakePriFileNode->setIncludedInExactParse(input.includedInExcactParse + && result->state == QmakeEvalResult::EvalOk); + result->priFiles.append(qMakePair(qmakePriFileNode, priFile->result)); + toCompare.append(qMakePair(qmakePriFileNode, priFile)); + } else { + auto *qmakeProFileNode = new QmakeProFile(priFile->name); + if (pn) + pn->addChild(qmakeProFileNode); + else + result->directChildren << qmakeProFileNode; + qmakeProFileNode->setIncludedInExactParse(input.includedInExcactParse + && result->exactSubdirs.contains(qmakeProFileNode->filePath())); + qmakeProFileNode->setParseInProgress(true); + result->proFiles << qmakeProFileNode; + } + } + } + return result; } @@ -1591,8 +1656,8 @@ void QmakeProFile::asyncEvaluate(QFutureInterface &fi, QmakeE void QmakeProFile::applyAsyncEvaluate() { - if (m_parseFutureWatcher.isFinished()) - applyEvaluate(m_parseFutureWatcher.result()); + if (m_parseFutureWatcher->isFinished()) + applyEvaluate(m_parseFutureWatcher->result()); m_buildSystem->decrementPendingEvaluateFutures(); } @@ -1655,53 +1720,22 @@ void QmakeProFile::applyEvaluate(QmakeEvalResult *evalResult) // // Add/Remove pri files, sub projects // - FilePath buildDirectory = buildDir(); - - QList> toCompare; - - toCompare.append(qMakePair(this, &result->includedFiles)); - makeEmpty(); + for (QmakePriFile * const toAdd : qAsConst(result->directChildren)) + addChild(toAdd); + result->directChildren.clear(); - while (!toCompare.isEmpty()) { - QmakePriFile *pn = toCompare.first().first; - QmakeIncludedPriFile *tree = toCompare.first().second; - toCompare.pop_front(); - - for (QmakeIncludedPriFile *priFile : tree->children) { - // Loop preventation, make sure that exact same node is not in our parent chain - bool loop = false; - QmakePriFile *n = pn; - while ((n = n->parent())) { - if (n->filePath() == priFile->name) { - loop = true; - break; - } - } - - if (loop) - continue; // Do nothing - - if (priFile->proFile) { - auto *qmakePriFileNode = new QmakePriFile(m_buildSystem, this, priFile->name); - pn->addChild(qmakePriFileNode); - qmakePriFileNode->setIncludedInExactParse( - (result->state == QmakeEvalResult::EvalOk) && pn->includedInExactParse()); - qmakePriFileNode->update(priFile->result); - toCompare.append(qMakePair(qmakePriFileNode, priFile)); - } else { - auto *qmakeProFileNode = new QmakeProFile(m_buildSystem, priFile->name); - pn->addChild(qmakeProFileNode); - qmakeProFileNode->setIncludedInExactParse( - result->exactSubdirs.contains(qmakeProFileNode->filePath()) - && pn->includedInExactParse()); - qmakeProFileNode->setParseInProgress(true); - qmakeProFileNode->asyncUpdate(); - } - } + for (const auto priFiles : qAsConst(result->priFiles)) { + priFiles.first->finishInitialization(m_buildSystem, this); + priFiles.first->update(priFiles.second); } + for (QmakeProFile * const proFile : qAsConst(result->proFiles)) { + proFile->finishInitialization(m_buildSystem, proFile); + proFile->setupFutureWatcher(); + proFile->asyncUpdate(); + } QmakePriFile::update(result->includedFiles.result); m_validParse = (result->state == QmakeEvalResult::EvalOk); diff --git a/src/plugins/qmakeprojectmanager/qmakeparsernodes.h b/src/plugins/qmakeprojectmanager/qmakeparsernodes.h index 61620e59fcf..9a5977348af 100644 --- a/src/plugins/qmakeprojectmanager/qmakeparsernodes.h +++ b/src/plugins/qmakeprojectmanager/qmakeparsernodes.h @@ -125,8 +125,10 @@ class QMAKEPROJECTMANAGER_EXPORT QmakePriFile { public: QmakePriFile(QmakeBuildSystem *buildSystem, QmakeProFile *qmakeProFile, const Utils::FilePath &filePath); + explicit QmakePriFile(const Utils::FilePath &filePath); virtual ~QmakePriFile(); + void finishInitialization(QmakeBuildSystem *buildSystem, QmakeProFile *qmakeProFile); Utils::FilePath filePath() const; Utils::FilePath directoryPath() const; virtual QString displayName() const; @@ -241,6 +243,7 @@ private: QMap m_files; QSet m_recursiveEnumerateFiles; // FIXME: Remove this?! QSet m_watchedFolders; + const Utils::FilePath m_filePath; bool m_includedInExactParse = true; friend class QmakeProFile; @@ -294,8 +297,11 @@ class QMAKEPROJECTMANAGER_EXPORT QmakeProFile : public QmakePriFile { public: QmakeProFile(QmakeBuildSystem *buildSystem, const Utils::FilePath &filePath); + explicit QmakeProFile(const Utils::FilePath &filePath); ~QmakeProFile() override; + void setupFutureWatcher(); + bool isParent(QmakeProFile *node); QString displayName() const final; @@ -390,7 +396,7 @@ private: QMap m_wildcardDirectoryContents; // Async stuff - QFutureWatcher m_parseFutureWatcher; + QFutureWatcher *m_parseFutureWatcher = nullptr; QtSupport::ProFileReader *m_readerExact = nullptr; QtSupport::ProFileReader *m_readerCumulative = nullptr; }; diff --git a/src/plugins/qmakeprojectmanager/qmakeproject.cpp b/src/plugins/qmakeprojectmanager/qmakeproject.cpp index f780541f890..ca2a977258f 100644 --- a/src/plugins/qmakeprojectmanager/qmakeproject.cpp +++ b/src/plugins/qmakeprojectmanager/qmakeproject.cpp @@ -67,6 +67,7 @@ #include #include +#include #include #include @@ -710,16 +711,19 @@ QString QmakeBuildSystem::qmakeSysroot() void QmakeBuildSystem::destroyProFileReader(QtSupport::ProFileReader *reader) { - delete reader; - if (!--m_qmakeGlobalsRefCnt) { - QString dir = projectFilePath().toString(); - if (!dir.endsWith(QLatin1Char('/'))) - dir += QLatin1Char('/'); - QtSupport::ProFileCacheManager::instance()->discardFiles(dir, qmakeVfs()); - QtSupport::ProFileCacheManager::instance()->decRefCount(); - - m_qmakeGlobals.reset(); - } + // The ProFileReader destructor is super expensive (but thread-safe). + const auto deleteFuture = runAsync(ProjectExplorerPlugin::sharedThreadPool(), QThread::LowestPriority, + [reader] { delete reader; }); + onFinished(deleteFuture, this, [this](const QFuture &) { + if (!--m_qmakeGlobalsRefCnt) { + QString dir = projectFilePath().toString(); + if (!dir.endsWith(QLatin1Char('/'))) + dir += QLatin1Char('/'); + QtSupport::ProFileCacheManager::instance()->discardFiles(dir, qmakeVfs()); + QtSupport::ProFileCacheManager::instance()->decRefCount(); + m_qmakeGlobals.reset(); + } + }); } void QmakeBuildSystem::activeTargetWasChanged(Target *t)