From c99ce1f455189864de9a2043730f704d7b024abf Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Wed, 4 Jan 2023 11:25:23 +0100 Subject: [PATCH] ExtraCompiler: Expose TaskItem for compile task This is an intermediate state before employing one common TaskTree inside CppProjectUpdater. Use multiple one-task TaskTrees for now. Refactor ExtraCompiler so that there is only one pure virtual method to be implemented (taskItem()) instead of two (2 run() overloads). Use TaskTree inside ExtraCompiler for running the compilation process. Change-Id: I6884934508e043594589d117f6d3f0aed94b84c2 Reviewed-by: Christian Kandeler --- src/plugins/cppeditor/cppprojectupdater.cpp | 51 +++--- src/plugins/cppeditor/cppprojectupdater.h | 4 +- src/plugins/projectexplorer/extracompiler.cpp | 145 +++++++++--------- src/plugins/projectexplorer/extracompiler.h | 45 +++--- src/plugins/python/pythonlanguageclient.cpp | 2 +- src/plugins/qbsprojectmanager/qbsproject.cpp | 2 +- 6 files changed, 121 insertions(+), 128 deletions(-) diff --git a/src/plugins/cppeditor/cppprojectupdater.cpp b/src/plugins/cppeditor/cppprojectupdater.cpp index 46b4ca1f141..1b19567cc28 100644 --- a/src/plugins/cppeditor/cppprojectupdater.cpp +++ b/src/plugins/cppeditor/cppprojectupdater.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include @@ -68,33 +69,29 @@ void CppProjectUpdater::update(const ProjectUpdateInfo &projectUpdateInfo, // extra compilers for (QPointer compiler : std::as_const(m_extraCompilers)) { - if (compiler->isDirty()) { - QPointer> watcher = new QFutureWatcher; - // queued connection to delay after the extra compiler updated its result contents, - // which is also done in the main thread when compiler->run() finished - connect(watcher, &QFutureWatcherBase::finished, - this, [this, watcher] { - // In very unlikely case the CppProjectUpdater::cancel() could have been - // invoked after posting the finished() signal and before this handler - // gets called. In this case the watcher is already deleted. - if (!watcher) - return; - m_projectUpdateFutureInterface->setProgressValue( - m_projectUpdateFutureInterface->progressValue() + 1); - m_extraCompilersFutureWatchers.remove(watcher); - watcher->deleteLater(); - if (!watcher->isCanceled()) - checkForExtraCompilersFinished(); - }, - Qt::QueuedConnection); - m_extraCompilersFutureWatchers += watcher; - watcher->setFuture(QFuture(compiler->run())); - m_futureSynchronizer.addFuture(watcher->future()); - } + if (!compiler->isDirty()) + continue; + + const auto destroyTaskTree = [this](TaskTree *taskTree) { + m_extraCompilerTasks.remove(taskTree); + taskTree->deleteLater(); + }; + TaskTree *taskTree = new TaskTree({compiler->compileFileItem()}); + connect(taskTree, &TaskTree::done, this, [this, taskTree, destroyTaskTree] { + destroyTaskTree(taskTree); + m_projectUpdateFutureInterface->setProgressValue( + m_projectUpdateFutureInterface->progressValue() + 1); + checkForExtraCompilersFinished(); + }); + connect(taskTree, &TaskTree::errorOccurred, this, [taskTree, destroyTaskTree] { + destroyTaskTree(taskTree); + }); + m_extraCompilerTasks.insert(taskTree); + taskTree->start(); } m_projectUpdateFutureInterface.reset(new QFutureInterface); - m_projectUpdateFutureInterface->setProgressRange(0, m_extraCompilersFutureWatchers.size() + m_projectUpdateFutureInterface->setProgressRange(0, m_extraCompilerTasks.size() + 1 /*generateFuture*/); m_projectUpdateFutureInterface->setProgressValue(0); m_projectUpdateFutureInterface->reportStarted(); @@ -109,8 +106,8 @@ void CppProjectUpdater::cancel() m_projectUpdateFutureInterface->reportFinished(); m_generateFutureWatcher.setFuture({}); m_isProjectInfoGenerated = false; - qDeleteAll(m_extraCompilersFutureWatchers); - m_extraCompilersFutureWatchers.clear(); + qDeleteAll(m_extraCompilerTasks); + m_extraCompilerTasks.clear(); m_extraCompilers.clear(); m_futureSynchronizer.cancelAllFutures(); } @@ -128,7 +125,7 @@ void CppProjectUpdater::onProjectInfoGenerated() void CppProjectUpdater::checkForExtraCompilersFinished() { - if (!m_extraCompilersFutureWatchers.isEmpty() || !m_isProjectInfoGenerated) + if (!m_extraCompilerTasks.isEmpty() || !m_isProjectInfoGenerated) return; // still need to wait m_projectUpdateFutureInterface->reportFinished(); diff --git a/src/plugins/cppeditor/cppprojectupdater.h b/src/plugins/cppeditor/cppprojectupdater.h index 2bcdc0a2cfb..9760c25da55 100644 --- a/src/plugins/cppeditor/cppprojectupdater.h +++ b/src/plugins/cppeditor/cppprojectupdater.h @@ -12,6 +12,8 @@ #include +namespace Utils { class TaskTree; } + namespace CppEditor { class ProjectInfo; @@ -53,7 +55,7 @@ private: QFutureWatcher m_generateFutureWatcher; bool m_isProjectInfoGenerated = false; - QSet *> m_extraCompilersFutureWatchers; + QSet m_extraCompilerTasks; std::unique_ptr> m_projectUpdateFutureInterface; Utils::FutureSynchronizer m_futureSynchronizer; }; diff --git a/src/plugins/projectexplorer/extracompiler.cpp b/src/plugins/projectexplorer/extracompiler.cpp index cc3f5ea8ca7..10672fdfe65 100644 --- a/src/plugins/projectexplorer/extracompiler.cpp +++ b/src/plugins/projectexplorer/extracompiler.cpp @@ -3,7 +3,6 @@ #include "extracompiler.h" -#include "buildconfiguration.h" #include "buildmanager.h" #include "kitinformation.h" #include "session.h" @@ -13,18 +12,14 @@ #include #include #include -#include #include +#include #include -#include #include -#include #include #include -#include -#include #include #include @@ -50,6 +45,9 @@ public: QTimer timer; void updateIssues(); + + FutureSynchronizer m_futureSynchronizer; + std::unique_ptr m_taskTree; }; ExtraCompiler::ExtraCompiler(const Project *project, const FilePath &source, @@ -65,7 +63,7 @@ ExtraCompiler::ExtraCompiler(const Project *project, const FilePath &source, connect(&d->timer, &QTimer::timeout, this, [this] { if (d->dirty && d->lastEditor) { d->dirty = false; - run(d->lastEditor->document()->contents()); + compileContent(d->lastEditor->document()->contents()); } }); @@ -128,7 +126,7 @@ FilePaths ExtraCompiler::targets() const return d->contents.keys(); } -void ExtraCompiler::forEachTarget(std::function func) +void ExtraCompiler::forEachTarget(std::function func) const { for (auto it = d->contents.constBegin(), end = d->contents.constEnd(); it != end; ++it) func(it.key()); @@ -144,6 +142,42 @@ QThreadPool *ExtraCompiler::extraCompilerThreadPool() return s_extraCompilerThreadPool(); } +Tasking::TaskItem ExtraCompiler::compileFileItem() +{ + return taskItemImpl(fromFileProvider()); +} + +void ExtraCompiler::compileFile() +{ + compileImpl(fromFileProvider()); +} + +void ExtraCompiler::compileContent(const QByteArray &content) +{ + compileImpl([content] { return content; }); +} + +void ExtraCompiler::compileImpl(const ContentProvider &provider) +{ + const auto finalize = [=] { + d->m_taskTree.release()->deleteLater(); + }; + d->m_taskTree.reset(new TaskTree({taskItemImpl(provider)})); + connect(d->m_taskTree.get(), &TaskTree::done, this, finalize); + connect(d->m_taskTree.get(), &TaskTree::errorOccurred, this, finalize); +} + +ExtraCompiler::ContentProvider ExtraCompiler::fromFileProvider() const +{ + const auto provider = [fileName = source()] { + QFile file(fileName.toString()); + if (!file.open(QFile::ReadOnly | QFile::Text)) + return QByteArray(); + return file.readAll(); + }; + return provider; +} + bool ExtraCompiler::isDirty() const { return d->dirty; @@ -186,7 +220,7 @@ void ExtraCompiler::onEditorChanged(Core::IEditor *editor) if (d->dirty) { d->dirty = false; - run(doc->contents()); + compileContent(doc->contents()); } } @@ -220,7 +254,7 @@ void ExtraCompiler::onEditorAboutToClose(Core::IEditor *editor) this, &ExtraCompiler::setDirty); if (d->dirty) { d->dirty = false; - run(doc->contents()); + compileContent(doc->contents()); } d->lastEditor = nullptr; } @@ -276,6 +310,11 @@ void ExtraCompilerPrivate::updateIssues() widget->setExtraSelections(TextEditor::TextEditorWidget::CodeWarningsSelection, selections); } +Utils::FutureSynchronizer *ExtraCompiler::futureSynchronizer() const +{ + return &d->m_futureSynchronizer; +} + void ExtraCompiler::setContent(const FilePath &file, const QByteArray &contents) { auto it = d->contents.find(file); @@ -308,30 +347,25 @@ ProcessExtraCompiler::ProcessExtraCompiler(const Project *project, const FilePat ExtraCompiler(project, source, targets, parent) { } -ProcessExtraCompiler::~ProcessExtraCompiler() +Tasking::TaskItem ProcessExtraCompiler::taskItemImpl(const ContentProvider &provider) { - if (!m_watcher) - return; - m_watcher->cancel(); - m_watcher->waitForFinished(); -} - -void ProcessExtraCompiler::run(const QByteArray &sourceContents) -{ - ContentProvider contents = [sourceContents]() { return sourceContents; }; - runImpl(contents); -} - -QFuture ProcessExtraCompiler::run() -{ - const FilePath fileName = source(); - ContentProvider contents = [fileName]() { - QFile file(fileName.toString()); - if (!file.open(QFile::ReadOnly | QFile::Text)) - return QByteArray(); - return file.readAll(); + const auto setupTask = [=](AsyncTask &async) { + async.setThreadPool(extraCompilerThreadPool()); + async.setAsyncCallData(&ProcessExtraCompiler::runInThread, this, command(), + workingDirectory(), arguments(), provider, buildEnvironment()); + async.setFutureSynchronizer(futureSynchronizer()); }; - return runImpl(contents); + const auto taskDone = [=](const AsyncTask &async) { + if (async.results().size() == 0) + return; + const FileNameToContentsHash data = async.result(); + if (data.isEmpty()) + return; // There was some kind of error... + for (auto it = data.constBegin(), end = data.constEnd(); it != end; ++it) + setContent(it.key(), it.value()); + updateCompileTime(); + }; + return Tasking::Async(setupTask, taskDone); } FilePath ProcessExtraCompiler::workingDirectory() const @@ -356,27 +390,10 @@ Tasks ProcessExtraCompiler::parseIssues(const QByteArray &stdErr) return {}; } -QFuture ProcessExtraCompiler::runImpl(const ContentProvider &provider) -{ - if (m_watcher) - delete m_watcher; - - m_watcher = new QFutureWatcher(); - connect(m_watcher, &QFutureWatcher::finished, - this, &ProcessExtraCompiler::cleanUp); - - m_watcher->setFuture(runAsync(extraCompilerThreadPool(), - &ProcessExtraCompiler::runInThread, this, - command(), workingDirectory(), arguments(), provider, - buildEnvironment())); - return m_watcher->future(); -} - -void ProcessExtraCompiler::runInThread( - QFutureInterface &futureInterface, - const FilePath &cmd, const FilePath &workDir, - const QStringList &args, const ContentProvider &provider, - const Environment &env) +void ProcessExtraCompiler::runInThread(QFutureInterface &futureInterface, + const FilePath &cmd, const FilePath &workDir, + const QStringList &args, const ContentProvider &provider, + const Environment &env) { if (cmd.isEmpty() || !cmd.toFileInfo().isExecutable()) return; @@ -396,9 +413,10 @@ void ProcessExtraCompiler::runInThread( if (!process.waitForStarted()) return; - while (!futureInterface.isCanceled()) + while (!futureInterface.isCanceled()) { if (process.waitForFinished(200)) break; + } if (futureInterface.isCanceled()) return; @@ -406,23 +424,4 @@ void ProcessExtraCompiler::runInThread( futureInterface.reportResult(handleProcessFinished(&process)); } -void ProcessExtraCompiler::cleanUp() -{ - QTC_ASSERT(m_watcher, return); - auto future = m_watcher->future(); - delete m_watcher; - m_watcher = nullptr; - if (!future.resultCount()) - return; - const FileNameToContentsHash data = future.result(); - - if (data.isEmpty()) - return; // There was some kind of error... - - for (auto it = data.constBegin(), end = data.constEnd(); it != end; ++it) - setContent(it.key(), it.value()); - - updateCompileTime(); -} - } // namespace ProjectExplorer diff --git a/src/plugins/projectexplorer/extracompiler.h b/src/plugins/projectexplorer/extracompiler.h index 85daad2ef46..eb3704113b8 100644 --- a/src/plugins/projectexplorer/extracompiler.h +++ b/src/plugins/projectexplorer/extracompiler.h @@ -8,8 +8,10 @@ #include "task.h" #include -#include + #include +#include +#include #include #include @@ -18,15 +20,16 @@ #include #include -QT_FORWARD_DECLARE_CLASS(QThreadPool); QT_BEGIN_NAMESPACE template class QFutureInterface; -template -class QFutureWatcher; +class QThreadPool; QT_END_NAMESPACE -namespace Utils { class QtcProcess; } +namespace Utils { +class FutureSynchronizer; +class QtcProcess; +} namespace ProjectExplorer { @@ -48,9 +51,10 @@ public: QByteArray content(const Utils::FilePath &file) const; Utils::FilePaths targets() const; - void forEachTarget(std::function func); + void forEachTarget(std::function func) const; - virtual QFuture run() = 0; + Utils::Tasking::TaskItem compileFileItem(); + void compileFile(); bool isDirty() const; signals: @@ -58,18 +62,23 @@ signals: protected: static QThreadPool *extraCompilerThreadPool(); + + Utils::FutureSynchronizer *futureSynchronizer() const; void setContent(const Utils::FilePath &file, const QByteArray &content); void updateCompileTime(); Utils::Environment buildEnvironment() const; void setCompileIssues(const Tasks &issues); + using ContentProvider = std::function; private: void onTargetsBuilt(Project *project); void onEditorChanged(Core::IEditor *editor); void onEditorAboutToClose(Core::IEditor *editor); void setDirty(); - // This method may not block! - virtual void run(const QByteArray &sourceContent) = 0; + ContentProvider fromFileProvider() const; + void compileContent(const QByteArray &content); + void compileImpl(const ContentProvider &provider); + virtual Utils::Tasking::TaskItem taskItemImpl(const ContentProvider &provider) = 0; const std::unique_ptr d; }; @@ -81,17 +90,8 @@ public: ProcessExtraCompiler(const Project *project, const Utils::FilePath &source, const Utils::FilePaths &targets, QObject *parent = nullptr); - ~ProcessExtraCompiler() override; protected: - // This will run a process in a thread, if - // * command() does not return an empty file name - // * command() is exectuable - // * prepareToRun returns true - // * The process is not yet running - void run(const QByteArray &sourceContents) override; - QFuture run() override; - // Information about the process to run: virtual Utils::FilePath workingDirectory() const; virtual Utils::FilePath command() const = 0; @@ -104,15 +104,11 @@ protected: virtual Tasks parseIssues(const QByteArray &stdErr); private: - using ContentProvider = std::function; - QFuture runImpl(const ContentProvider &sourceContents); + Utils::Tasking::TaskItem taskItemImpl(const ContentProvider &provider) final; void runInThread(QFutureInterface &futureInterface, const Utils::FilePath &cmd, const Utils::FilePath &workDir, const QStringList &args, const ContentProvider &provider, const Utils::Environment &env); - void cleanUp(); - - QFutureWatcher *m_watcher = nullptr; }; class PROJECTEXPLORER_EXPORT ExtraCompilerFactory : public QObject @@ -127,8 +123,7 @@ public: virtual ExtraCompiler *create(const Project *project, const Utils::FilePath &source, - const Utils::FilePaths &targets) - = 0; + const Utils::FilePaths &targets) = 0; static QList extraCompilerFactories(); }; diff --git a/src/plugins/python/pythonlanguageclient.cpp b/src/plugins/python/pythonlanguageclient.cpp index 33181d45166..3e36c27e78b 100644 --- a/src/plugins/python/pythonlanguageclient.cpp +++ b/src/plugins/python/pythonlanguageclient.cpp @@ -249,7 +249,7 @@ void PyLSClient::updateExtraCompilers(ProjectExplorer::Project *project, updateExtraCompilerContents(extraCompiler, file); }); if (extraCompiler->isDirty()) - static_cast(extraCompiler)->run(); + extraCompiler->compileFile(); } else { m_extraCompilers[project] << oldCompilers.takeAt(index); } diff --git a/src/plugins/qbsprojectmanager/qbsproject.cpp b/src/plugins/qbsprojectmanager/qbsproject.cpp index 7c2f611c4a3..6c6a7febf5f 100644 --- a/src/plugins/qbsprojectmanager/qbsproject.cpp +++ b/src/plugins/qbsprojectmanager/qbsproject.cpp @@ -175,7 +175,7 @@ QbsBuildSystem::QbsBuildSystem(QbsBuildConfiguration *bc) CppEditor::GeneratedCodeModelSupport::update(m_extraCompilers); for (ExtraCompiler *compiler : m_extraCompilers) { if (compiler->isDirty()) - compiler->run(); + compiler->compileFile(); } m_sourcesForGeneratedFiles.clear(); });