From c8cec2dd0d7fa30e88c4e0fc0a6083d4bf2d7f05 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Tue, 13 Sep 2016 13:57:08 +0200 Subject: [PATCH] Clang: Base JobQueue on translation unit This enables a job per translation unit instead of per document. This does not change any behavior yet. Change-Id: Iafb8dab5da32b53dbb3010c16241bf89cbb81b38 Reviewed-by: David Schulz --- .../ipcsource/clangcompletecodejob.cpp | 10 ++--- .../ipcsource/clangcompletecodejob.h | 2 +- .../clangcreateinitialdocumentpreamblejob.cpp | 9 ++--- .../clangcreateinitialdocumentpreamblejob.h | 2 +- .../clangbackend/ipcsource/clangiasyncjob.h | 7 +++- .../clangbackend/ipcsource/clangjobqueue.cpp | 27 +++++++------- .../clangbackend/ipcsource/clangjobqueue.h | 5 +-- .../ipcsource/clangjobrequest.cpp | 20 +++++++++- .../clangbackend/ipcsource/clangjobrequest.h | 2 + .../clangbackend/ipcsource/clangjobs.cpp | 21 +++++------ src/tools/clangbackend/ipcsource/clangjobs.h | 3 +- .../clangrequestdocumentannotationsjob.cpp | 10 ++--- .../clangrequestdocumentannotationsjob.h | 2 +- .../clangupdatedocumentannotationsjob.cpp | 10 ++--- .../clangupdatedocumentannotationsjob.h | 2 +- tests/unit/unittest/clangjobqueue-test.cpp | 37 ++++++++++++++++--- tests/unit/unittest/clangjobs-test.cpp | 2 +- 17 files changed, 108 insertions(+), 63 deletions(-) diff --git a/src/tools/clangbackend/ipcsource/clangcompletecodejob.cpp b/src/tools/clangbackend/ipcsource/clangcompletecodejob.cpp index ce9d190bec0..8d13869bb8a 100644 --- a/src/tools/clangbackend/ipcsource/clangcompletecodejob.cpp +++ b/src/tools/clangbackend/ipcsource/clangcompletecodejob.cpp @@ -50,10 +50,10 @@ static CompleteCodeJob::AsyncResult runAsyncHelper(const TranslationUnit &transl return asyncResult; } -bool CompleteCodeJob::prepareAsyncRun() +IAsyncJob::AsyncPrepareResult CompleteCodeJob::prepareAsyncRun() { const JobRequest jobRequest = context().jobRequest; - QTC_ASSERT(jobRequest.type == JobRequest::Type::CompleteCode, return false); + QTC_ASSERT(jobRequest.type == JobRequest::Type::CompleteCode, return AsyncPrepareResult()); try { m_pinnedDocument = context().documentForJobRequest(); @@ -65,14 +65,12 @@ bool CompleteCodeJob::prepareAsyncRun() setRunner([translationUnit, unsavedFiles, line, column]() { return runAsyncHelper(translationUnit, unsavedFiles, line, column); }); - + return AsyncPrepareResult{translationUnit.id()}; } catch (const std::exception &exception) { qWarning() << "Error in CompleteCodeJob::prepareAsyncRun:" << exception.what(); - return false; + return AsyncPrepareResult(); } - - return true; } void CompleteCodeJob::finalizeAsyncRun() diff --git a/src/tools/clangbackend/ipcsource/clangcompletecodejob.h b/src/tools/clangbackend/ipcsource/clangcompletecodejob.h index 30c0bb8e3f9..142988d7403 100644 --- a/src/tools/clangbackend/ipcsource/clangcompletecodejob.h +++ b/src/tools/clangbackend/ipcsource/clangcompletecodejob.h @@ -43,7 +43,7 @@ class CompleteCodeJob : public AsyncJob public: using AsyncResult = CompleteCodeJobResult; - bool prepareAsyncRun() override; + AsyncPrepareResult prepareAsyncRun() override; void finalizeAsyncRun() override; private: diff --git a/src/tools/clangbackend/ipcsource/clangcreateinitialdocumentpreamblejob.cpp b/src/tools/clangbackend/ipcsource/clangcreateinitialdocumentpreamblejob.cpp index dd5e1f267f8..f24a3978e9e 100644 --- a/src/tools/clangbackend/ipcsource/clangcreateinitialdocumentpreamblejob.cpp +++ b/src/tools/clangbackend/ipcsource/clangcreateinitialdocumentpreamblejob.cpp @@ -39,10 +39,10 @@ static void runAsyncHelper(const TranslationUnit &translationUnit, translationUnit.reparse(translationUnitUpdateInput); } -bool CreateInitialDocumentPreambleJob::prepareAsyncRun() +IAsyncJob::AsyncPrepareResult CreateInitialDocumentPreambleJob::prepareAsyncRun() { const JobRequest jobRequest = context().jobRequest; - QTC_ASSERT(jobRequest.type == JobRequest::Type::CreateInitialDocumentPreamble, return false); + QTC_ASSERT(jobRequest.type == JobRequest::Type::CreateInitialDocumentPreamble, return AsyncPrepareResult()); try { m_pinnedDocument = context().documentForJobRequest(); @@ -53,14 +53,13 @@ bool CreateInitialDocumentPreambleJob::prepareAsyncRun() setRunner([translationUnit, updateInput]() { return runAsyncHelper(translationUnit, updateInput); }); + return AsyncPrepareResult{translationUnit.id()}; } catch (const std::exception &exception) { qWarning() << "Error in CreateInitialDocumentPreambleJob::prepareAsyncRun:" << exception.what(); - return false; + return AsyncPrepareResult(); } - - return true; } void CreateInitialDocumentPreambleJob::finalizeAsyncRun() diff --git a/src/tools/clangbackend/ipcsource/clangcreateinitialdocumentpreamblejob.h b/src/tools/clangbackend/ipcsource/clangcreateinitialdocumentpreamblejob.h index 21fc875982b..570b746460e 100644 --- a/src/tools/clangbackend/ipcsource/clangcreateinitialdocumentpreamblejob.h +++ b/src/tools/clangbackend/ipcsource/clangcreateinitialdocumentpreamblejob.h @@ -33,7 +33,7 @@ namespace ClangBackEnd { class CreateInitialDocumentPreambleJob : public AsyncJob { public: - bool prepareAsyncRun() override; + AsyncPrepareResult prepareAsyncRun() override; void finalizeAsyncRun() override; private: diff --git a/src/tools/clangbackend/ipcsource/clangiasyncjob.h b/src/tools/clangbackend/ipcsource/clangiasyncjob.h index dcc7df1c16f..98f61e857ab 100644 --- a/src/tools/clangbackend/ipcsource/clangiasyncjob.h +++ b/src/tools/clangbackend/ipcsource/clangiasyncjob.h @@ -41,6 +41,11 @@ class IAsyncJob public: static IAsyncJob *create(JobRequest::Type type); + struct AsyncPrepareResult { + operator bool() const { return !translationUnitId.isEmpty(); } + Utf8String translationUnitId; + }; + public: IAsyncJob(); virtual ~IAsyncJob(); @@ -52,7 +57,7 @@ public: FinishedHandler finishedHandler() const; void setFinishedHandler(const FinishedHandler &finishedHandler); - virtual bool prepareAsyncRun() = 0; + virtual AsyncPrepareResult prepareAsyncRun() = 0; virtual QFuture runAsync() = 0; virtual void finalizeAsyncRun() = 0; diff --git a/src/tools/clangbackend/ipcsource/clangjobqueue.cpp b/src/tools/clangbackend/ipcsource/clangjobqueue.cpp index 1beb1cdace6..cff2bd3bc05 100644 --- a/src/tools/clangbackend/ipcsource/clangjobqueue.cpp +++ b/src/tools/clangbackend/ipcsource/clangjobqueue.cpp @@ -27,6 +27,7 @@ #include "clangjobqueue.h" #include "clangdocument.h" #include "clangdocuments.h" +#include "clangtranslationunits.h" #include "projects.h" #include "unsavedfiles.h" @@ -164,43 +165,43 @@ void JobQueue::prioritizeRequests() JobRequests JobQueue::takeJobRequestsToRunNow() { JobRequests jobsToRun; - QSet documentsScheduledForThisRun; + using TranslationUnitIds = QSet; + TranslationUnitIds translationUnitsScheduledForThisRun; QMutableVectorIterator i(m_queue); while (i.hasNext()) { - const JobRequest &jobRequest = i.next(); + const JobRequest &request = i.next(); try { - const Document &document - = m_documents.document(jobRequest.filePath, - jobRequest.projectPartId); - const DocumentId documentId = DocumentId(jobRequest.filePath, jobRequest.projectPartId); + const Document &document = m_documents.document(request.filePath, + request.projectPartId); if (!document.isUsedByCurrentEditor() && !document.isVisibleInEditor()) continue; - if (documentsScheduledForThisRun.contains(documentId)) + const Utf8String id = document.translationUnit(request.preferredTranslationUnit).id(); + if (translationUnitsScheduledForThisRun.contains(id)) continue; - if (isJobRunningForDocument(documentId)) + if (isJobRunningForTranslationUnit(id)) continue; - documentsScheduledForThisRun.insert(documentId); - jobsToRun += jobRequest; + translationUnitsScheduledForThisRun.insert(id); + jobsToRun += request; i.remove(); } catch (const std::exception &exception) { qWarning() << "Error in Jobs::takeJobRequestsToRunNow for" - << jobRequest << ":" << exception.what(); + << request << ":" << exception.what(); } } return jobsToRun; } -bool JobQueue::isJobRunningForDocument(const JobQueue::DocumentId &documentId) +bool JobQueue::isJobRunningForTranslationUnit(const Utf8String &translationUnitId) { if (m_isJobRunningHandler) - return m_isJobRunningHandler(documentId.first, documentId.second); + return m_isJobRunningHandler(translationUnitId); return false; } diff --git a/src/tools/clangbackend/ipcsource/clangjobqueue.h b/src/tools/clangbackend/ipcsource/clangjobqueue.h index 1be38ceaf07..3781dda3d36 100644 --- a/src/tools/clangbackend/ipcsource/clangjobqueue.h +++ b/src/tools/clangbackend/ipcsource/clangjobqueue.h @@ -43,7 +43,7 @@ public: JobRequests processQueue(); - using IsJobRunningHandler = std::function; + using IsJobRunningHandler = std::function; void setIsJobRunningHandler(const IsJobRunningHandler &isJobRunningHandler); public: // for tests @@ -52,8 +52,7 @@ public: // for tests void prioritizeRequests(); private: - using DocumentId = QPair; - bool isJobRunningForDocument(const DocumentId &documentId); + bool isJobRunningForTranslationUnit(const Utf8String &translationUnitId); JobRequests takeJobRequestsToRunNow(); void removeOutDatedRequests(); bool isJobRequestOutDated(const JobRequest &jobRequest) const; diff --git a/src/tools/clangbackend/ipcsource/clangjobrequest.cpp b/src/tools/clangbackend/ipcsource/clangjobrequest.cpp index 4c3d692d08c..13cf7da26dc 100644 --- a/src/tools/clangbackend/ipcsource/clangjobrequest.cpp +++ b/src/tools/clangbackend/ipcsource/clangjobrequest.cpp @@ -25,6 +25,8 @@ #include "clangjobrequest.h" +#include + namespace ClangBackEnd { #define RETURN_TEXT_FOR_CASE(enumValue) case JobRequest::Type::enumValue: return #enumValue @@ -41,6 +43,18 @@ static const char *JobRequestTypeToText(JobRequest::Type type) } #undef RETURN_TEXT_FOR_CASE +#define RETURN_TEXT_FOR_CASE(enumValue) case PreferredTranslationUnit::enumValue: return #enumValue +const char *preferredTranslationUnitToText(PreferredTranslationUnit type) +{ + switch (type) { + RETURN_TEXT_FOR_CASE(RecentlyParsed); + RETURN_TEXT_FOR_CASE(PreviouslyParsed); + } + + return "UnhandledPreferredTranslationUnitType"; +} +#undef RETURN_TEXT_FOR_CASE + QDebug operator<<(QDebug debug, JobRequest::Type type) { debug << JobRequestTypeToText(type); @@ -53,12 +67,14 @@ QDebug operator<<(QDebug debug, const JobRequest &jobRequest) debug.nospace() << "Job<" << jobRequest.id << "," + << QFileInfo(jobRequest.filePath).fileName() + << "," << JobRequestTypeToText(jobRequest.type) << "," - << jobRequest.filePath + << preferredTranslationUnitToText(jobRequest.preferredTranslationUnit) << ">"; - return debug; + return debug.space(); } JobRequest::JobRequest() diff --git a/src/tools/clangbackend/ipcsource/clangjobrequest.h b/src/tools/clangbackend/ipcsource/clangjobrequest.h index 1979c51874e..f9a5604979a 100644 --- a/src/tools/clangbackend/ipcsource/clangjobrequest.h +++ b/src/tools/clangbackend/ipcsource/clangjobrequest.h @@ -25,6 +25,7 @@ #pragma once +#include "clangbackend_global.h" #include "clangclock.h" #include @@ -73,6 +74,7 @@ public: TimePoint unsavedFilesChangeTimePoint; TimePoint projectChangeTimePoint; uint documentRevision = 0; + PreferredTranslationUnit preferredTranslationUnit = PreferredTranslationUnit::RecentlyParsed; // For code completion quint32 line = 0; diff --git a/src/tools/clangbackend/ipcsource/clangjobs.cpp b/src/tools/clangbackend/ipcsource/clangjobs.cpp index c5202c15858..7626055eea6 100644 --- a/src/tools/clangbackend/ipcsource/clangjobs.cpp +++ b/src/tools/clangbackend/ipcsource/clangjobs.cpp @@ -45,9 +45,8 @@ Jobs::Jobs(Documents &documents, , m_client(client) , m_queue(documents, projectParts) { - m_queue.setIsJobRunningHandler([this](const Utf8String &filePath, - const Utf8String &projectPartId) { - return isJobRunning(filePath, projectPartId); + m_queue.setIsJobRunningHandler([this](const Utf8String &translationUnitId) { + return isJobRunning(translationUnitId); }); } @@ -97,13 +96,15 @@ bool Jobs::runJob(const JobRequest &jobRequest) JobContext context(jobRequest, &m_documents, &m_unsavedFiles, &m_client); asyncJob->setContext(context); - if (asyncJob->prepareAsyncRun()) { - qCDebug(jobsLog) << "Running" << jobRequest; + if (const IAsyncJob::AsyncPrepareResult prepareResult = asyncJob->prepareAsyncRun()) { + qCDebug(jobsLog) << "Running" << jobRequest + << "with TranslationUnit" << prepareResult.translationUnitId; asyncJob->setFinishedHandler([this](IAsyncJob *asyncJob){ onJobFinished(asyncJob); }); const QFuture future = asyncJob->runAsync(); - m_running.insert(asyncJob, RunningJob{jobRequest, future}); + const RunningJob runningJob{jobRequest, prepareResult.translationUnitId, future}; + m_running.insert(asyncJob, runningJob); return true; } else { qCDebug(jobsLog) << "Preparation failed for " << jobRequest; @@ -134,12 +135,10 @@ JobRequests Jobs::queue() const return m_queue.queue(); } -bool Jobs::isJobRunning(const Utf8String &filePath, const Utf8String &projectPartId) const +bool Jobs::isJobRunning(const Utf8String &translationUnitId) const { - const auto hasJobRequest = [filePath, projectPartId](const RunningJob &runningJob) { - const JobRequest &jobRequest = runningJob.jobRequest; - return filePath == jobRequest.filePath - && projectPartId == jobRequest.projectPartId; + const auto hasJobRequest = [translationUnitId](const RunningJob &runningJob) { + return runningJob.translationUnitId == translationUnitId; }; return Utils::anyOf(m_running.values(), hasJobRequest); diff --git a/src/tools/clangbackend/ipcsource/clangjobs.h b/src/tools/clangbackend/ipcsource/clangjobs.h index 85d47488580..21c3576a48f 100644 --- a/src/tools/clangbackend/ipcsource/clangjobs.h +++ b/src/tools/clangbackend/ipcsource/clangjobs.h @@ -43,6 +43,7 @@ class Jobs public: struct RunningJob { JobRequest jobRequest; + Utf8String translationUnitId; QFuture future; }; using RunningJobs = QHash; @@ -61,7 +62,7 @@ public: public /*for tests*/: QList runningJobs() const; JobRequests queue() const; - bool isJobRunning(const Utf8String &filePath, const Utf8String &projectPartId) const; + bool isJobRunning(const Utf8String &translationUnitId) const; private: JobRequests runJobs(const JobRequests &jobRequest); diff --git a/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.cpp b/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.cpp index 689fe2b9f19..e53ff0b8007 100644 --- a/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.cpp +++ b/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.cpp @@ -47,10 +47,11 @@ static RequestDocumentAnnotationsJob::AsyncResult runAsyncHelper( return asyncResult; } -bool RequestDocumentAnnotationsJob::prepareAsyncRun() +IAsyncJob::AsyncPrepareResult RequestDocumentAnnotationsJob::prepareAsyncRun() { const JobRequest jobRequest = context().jobRequest; - QTC_ASSERT(jobRequest.type == JobRequest::Type::RequestDocumentAnnotations, return false); + QTC_ASSERT(jobRequest.type == JobRequest::Type::RequestDocumentAnnotations, + return AsyncPrepareResult()); try { m_pinnedDocument = context().documentForJobRequest(); @@ -60,13 +61,12 @@ bool RequestDocumentAnnotationsJob::prepareAsyncRun() setRunner([translationUnit]() { return runAsyncHelper(translationUnit); }); + return AsyncPrepareResult{translationUnit.id()}; } catch (const std::exception &exception) { qWarning() << "Error in RequestDocumentAnnotationsJob::prepareAsyncRun:" << exception.what(); - return false; + return AsyncPrepareResult(); } - - return true; } void RequestDocumentAnnotationsJob::finalizeAsyncRun() diff --git a/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.h b/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.h index 04bdd7cfc3a..b9ff1c822c7 100644 --- a/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.h +++ b/src/tools/clangbackend/ipcsource/clangrequestdocumentannotationsjob.h @@ -46,7 +46,7 @@ class RequestDocumentAnnotationsJob : public AsyncJob #include +#include +#include #include #include #include @@ -60,7 +62,9 @@ protected: Utf8String createTranslationUnitForDeletedFile(); JobRequest createJobRequest(const Utf8String &filePath, - JobRequest::Type type) const; + JobRequest::Type type, + PreferredTranslationUnit preferredTranslationUnit + = PreferredTranslationUnit::RecentlyParsed) const; void updateDocumentRevision(); void updateUnsavedFiles(); @@ -240,7 +244,7 @@ TEST_F(JobQueue, RunNothingForNotCurrentOrVisibleDocument) ASSERT_THAT(jobsToRun.size(), Eq(0)); } -TEST_F(JobQueue, RunOnlyOneJobPerDocumentIfMultipleAreInQueue) +TEST_F(JobQueue, RunOnlyOneJobPerTranslationUnitIfMultipleAreInQueue) { jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations)); jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations)); @@ -251,12 +255,30 @@ TEST_F(JobQueue, RunOnlyOneJobPerDocumentIfMultipleAreInQueue) ASSERT_THAT(jobQueue.size(), Eq(1)); } -TEST_F(JobQueue, DoNotRunJobForDocumentThatIsBeingProcessed) +TEST_F(JobQueue, RunJobsForDistinctTranslationUnits) +{ + const TranslationUnit initialTu = document.translationUnit(); + document.translationUnits().updateParseTimePoint(initialTu.id(), std::chrono::steady_clock::now()); + const TranslationUnit alternativeTu = document.translationUnits().createAndAppend(); + document.translationUnits().updateParseTimePoint(alternativeTu.id(), std::chrono::steady_clock::now()); + jobQueue.add(createJobRequest(filePath1, + JobRequest::Type::UpdateDocumentAnnotations, + PreferredTranslationUnit::RecentlyParsed)); + jobQueue.add(createJobRequest(filePath1, + JobRequest::Type::UpdateDocumentAnnotations, + PreferredTranslationUnit::PreviouslyParsed)); + + const JobRequests jobsToRun = jobQueue.processQueue(); + + ASSERT_THAT(jobsToRun.size(), Eq(2)); + ASSERT_THAT(jobQueue.size(), Eq(0)); +} +TEST_F(JobQueue, DoNotRunJobForTranslationUnittThatIsBeingProcessed) { jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations)); jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations)); JobRequests jobsToRun = jobQueue.processQueue(); - jobQueue.setIsJobRunningHandler([](const Utf8String &, const Utf8String &) { + jobQueue.setIsJobRunningHandler([](const Utf8String &) { return true; }); @@ -397,8 +419,10 @@ Utf8String JobQueue::createTranslationUnitForDeletedFile() return temporaryFilePath; } -JobRequest JobQueue::createJobRequest(const Utf8String &filePath, - JobRequest::Type type) const +JobRequest JobQueue::createJobRequest( + const Utf8String &filePath, + JobRequest::Type type, + PreferredTranslationUnit preferredTranslationUnit) const { JobRequest jobRequest; jobRequest.type = type; @@ -407,6 +431,7 @@ JobRequest JobQueue::createJobRequest(const Utf8String &filePath, jobRequest.projectPartId = projectPartId; jobRequest.unsavedFilesChangeTimePoint = unsavedFiles.lastChangeTimePoint(); jobRequest.documentRevision = document.documentRevision(); + jobRequest.preferredTranslationUnit = preferredTranslationUnit; jobRequest.projectChangeTimePoint = projects.project(projectPartId).lastChangeTimePoint(); return jobRequest; diff --git a/tests/unit/unittest/clangjobs-test.cpp b/tests/unit/unittest/clangjobs-test.cpp index d5ef57155b5..fffc6f82375 100644 --- a/tests/unit/unittest/clangjobs-test.cpp +++ b/tests/unit/unittest/clangjobs-test.cpp @@ -108,7 +108,7 @@ TEST_F(Jobs, IsJobRunning) jobs.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations)); jobs.process(); - const bool isJobRunning = jobs.isJobRunning(filePath1, projectPartId); + const bool isJobRunning = jobs.isJobRunning(document.translationUnit().id()); ASSERT_TRUE(isJobRunning); }