Commit bf5c1cc4 authored by Nikolai Kosjar's avatar Nikolai Kosjar
Browse files

Clang: Avoid duplicate jobs without changes in-between



This could happen, e.g. with this message order:

  >>> updateTranslationUnitsForEditor()
  add job<1>
  run job<1>
  >>> updateVisibleTranslationUnits(Utf8String(), {})
  >>> updateVisibleTranslationUnits(path, {path})
  add job<2>
  finish job<1>
  run job<2> -- Ops, nothing is changed but job<2> is started

This led to an outdated translation unit (e.g. wrong highlighting).

Now JobQueue checks for duplicates in the queue and checks all the
currently running jobs.

Change-Id: I05843fddcbd21ce0489681c283227c0027ded428
Reviewed-by: David Schulz's avatarDavid Schulz <david.schulz@qt.io>
parent 380d756a
......@@ -41,11 +41,22 @@ JobQueue::JobQueue(Documents &documents, ProjectParts &projectParts)
{
}
void JobQueue::add(const JobRequest &job)
bool JobQueue::add(const JobRequest &job)
{
qCDebug(jobsLog) << "Adding" << job;
if (m_queue.contains(job)) {
qCDebug(jobsLog) << "Not adding duplicate request" << job;
return false;
}
if (isJobRunningForJobRequest(job)) {
qCDebug(jobsLog) << "Not adding duplicate request for already running job" << job;
return false;
}
qCDebug(jobsLog) << "Adding" << job;
m_queue.append(job);
return true;
}
int JobQueue::size() const
......@@ -200,15 +211,30 @@ JobRequests JobQueue::takeJobRequestsToRunNow()
bool JobQueue::isJobRunningForTranslationUnit(const Utf8String &translationUnitId)
{
if (m_isJobRunningHandler)
return m_isJobRunningHandler(translationUnitId);
if (m_isJobRunningForTranslationUnitHandler)
return m_isJobRunningForTranslationUnitHandler(translationUnitId);
return false;
}
bool JobQueue::isJobRunningForJobRequest(const JobRequest &jobRequest)
{
if (m_isJobRunningForJobRequestHandler)
return m_isJobRunningForJobRequestHandler(jobRequest);
return false;
}
void JobQueue::setIsJobRunningHandler(const IsJobRunningHandler &isJobRunningHandler)
void JobQueue::setIsJobRunningForTranslationUnitHandler(
const IsJobRunningForTranslationUnitHandler &isJobRunningHandler)
{
m_isJobRunningForTranslationUnitHandler = isJobRunningHandler;
}
void JobQueue::setIsJobRunningForJobRequestHandler(
const JobQueue::IsJobRunningForJobRequestHandler &isJobRunningHandler)
{
m_isJobRunningHandler = isJobRunningHandler;
m_isJobRunningForJobRequestHandler = isJobRunningHandler;
}
JobRequests JobQueue::queue() const
......
......@@ -39,12 +39,17 @@ class JobQueue
public:
JobQueue(Documents &documents, ProjectParts &projects);
void add(const JobRequest &job);
bool add(const JobRequest &job);
JobRequests processQueue();
using IsJobRunningHandler = std::function<bool(const Utf8String &)>;
void setIsJobRunningHandler(const IsJobRunningHandler &isJobRunningHandler);
using IsJobRunningForTranslationUnitHandler = std::function<bool(const Utf8String &)>;
void setIsJobRunningForTranslationUnitHandler(
const IsJobRunningForTranslationUnitHandler &isJobRunningHandler);
using IsJobRunningForJobRequestHandler = std::function<bool(const JobRequest &)>;
void setIsJobRunningForJobRequestHandler(
const IsJobRunningForJobRequestHandler &isJobRunningHandler);
public: // for tests
JobRequests queue() const;
......@@ -53,6 +58,7 @@ public: // for tests
private:
bool isJobRunningForTranslationUnit(const Utf8String &translationUnitId);
bool isJobRunningForJobRequest(const JobRequest &jobRequest);
JobRequests takeJobRequestsToRunNow();
void removeOutDatedRequests();
bool isJobRequestOutDated(const JobRequest &jobRequest) const;
......@@ -61,7 +67,8 @@ private:
Documents &m_documents;
ProjectParts &m_projectParts;
IsJobRunningHandler m_isJobRunningHandler;
IsJobRunningForTranslationUnitHandler m_isJobRunningForTranslationUnitHandler;
IsJobRunningForJobRequestHandler m_isJobRunningForJobRequestHandler;
JobRequests m_queue;
};
......
......@@ -86,6 +86,23 @@ JobRequest::JobRequest()
id = ++idCounter;
}
bool JobRequest::operator==(const JobRequest &other) const
{
return type == other.type
&& requirements == other.requirements
&& filePath == other.filePath
&& projectPartId == other.projectPartId
&& unsavedFilesChangeTimePoint == other.unsavedFilesChangeTimePoint
&& projectChangeTimePoint == other.projectChangeTimePoint
&& documentRevision == other.documentRevision
&& preferredTranslationUnit == other.preferredTranslationUnit
&& line == other.line
&& column == other.column
&& ticketNumber == other.ticketNumber;
}
JobRequest::Requirements JobRequest::requirementsForType(Type type)
{
switch (type) {
......
......@@ -71,6 +71,8 @@ public:
JobRequest();
bool operator==(const JobRequest &other) const;
public:
quint64 id = 0;
Type type;
......
......@@ -45,8 +45,11 @@ Jobs::Jobs(Documents &documents,
, m_client(client)
, m_queue(documents, projectParts)
{
m_queue.setIsJobRunningHandler([this](const Utf8String &translationUnitId) {
return isJobRunning(translationUnitId);
m_queue.setIsJobRunningForTranslationUnitHandler([this](const Utf8String &translationUnitId) {
return isJobRunningForTranslationUnit(translationUnitId);
});
m_queue.setIsJobRunningForJobRequestHandler([this](const JobRequest &jobRequest) {
return isJobRunningForJobRequest(jobRequest);
});
}
......@@ -146,12 +149,21 @@ JobRequests Jobs::queue() const
return m_queue.queue();
}
bool Jobs::isJobRunning(const Utf8String &translationUnitId) const
bool Jobs::isJobRunningForTranslationUnit(const Utf8String &translationUnitId) const
{
const auto hasJobRequest = [translationUnitId](const RunningJob &runningJob) {
const auto hasTranslationUnitId = [translationUnitId](const RunningJob &runningJob) {
return runningJob.translationUnitId == translationUnitId;
};
return Utils::anyOf(m_running.values(), hasTranslationUnitId);
}
bool Jobs::isJobRunningForJobRequest(const JobRequest &jobRequest) const
{
const auto hasJobRequest = [jobRequest](const RunningJob &runningJob) {
return runningJob.jobRequest == jobRequest;
};
return Utils::anyOf(m_running.values(), hasJobRequest);
}
......
......@@ -68,7 +68,8 @@ public:
public /*for tests*/:
QList<RunningJob> runningJobs() const;
JobRequests queue() const;
bool isJobRunning(const Utf8String &translationUnitId) const;
bool isJobRunningForTranslationUnit(const Utf8String &translationUnitId) const;
bool isJobRunningForJobRequest(const JobRequest &jobRequest) const;
private:
JobRequests runJobs(const JobRequests &jobRequest);
......
......@@ -42,6 +42,7 @@
#include <cmbunregisterprojectsforeditormessage.h>
#include <cmbunregistertranslationunitsforeditormessage.h>
#include <QCoreApplication>
#include <QFile>
using testing::Property;
......@@ -322,6 +323,20 @@ TEST_F(ClangClangCodeModelServer, SupportiveTranslationUnitIsSetupAfterFirstEdit
ASSERT_TRUE(isSupportiveTranslationUnitInitialized(filePathA));
}
TEST_F(ClangClangCodeModelServer, DoNotRunDuplicateJobs)
{
registerProjectAndFile(filePathA, 3);
ASSERT_TRUE(waitUntilAllJobsFinished());
updateUnsavedContent(filePathA, unsavedContent(filePathAUnsavedVersion2), 1);
ASSERT_TRUE(waitUntilAllJobsFinished());
ASSERT_TRUE(isSupportiveTranslationUnitInitialized(filePathA));
updateUnsavedContent(filePathA, unsavedContent(filePathAUnsavedVersion2), 2);
QCoreApplication::processEvents(); // adds + runs a job
updateVisibilty(Utf8String(), Utf8String());
updateVisibilty(filePathA, filePathA); // triggers adding + runnings job on next processevents()
}
TEST_F(ClangClangCodeModelServer, OpenDocumentAndEdit)
{
registerProjectAndFile(filePathA, 4);
......
......@@ -95,6 +95,29 @@ TEST_F(JobQueue, AddJob)
ASSERT_THAT(jobQueue.queue().size(), Eq(1));
}
TEST_F(JobQueue, DoNotAddDuplicate)
{
const JobRequest request = createJobRequest(filePath1,
JobRequest::Type::UpdateDocumentAnnotations);
jobQueue.add(request);
const bool added = jobQueue.add(request);
ASSERT_FALSE(added);
}
TEST_F(JobQueue, DoNotAddDuplicateForWhichAJobIsAlreadyRunning)
{
jobQueue.setIsJobRunningForJobRequestHandler([](const JobRequest &) {
return true;
});
const bool added = jobQueue.add(createJobRequest(filePath1,
JobRequest::Type::UpdateDocumentAnnotations));
ASSERT_FALSE(added);
}
TEST_F(JobQueue, ProcessEmpty)
{
jobQueue.processQueue();
......@@ -115,7 +138,7 @@ TEST_F(JobQueue, ProcessSingleJob)
TEST_F(JobQueue, ProcessUntilEmpty)
{
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations));
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations));
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::CreateInitialDocumentPreamble));
JobRequests jobsToRun;
ASSERT_THAT(jobQueue.size(), Eq(2));
......@@ -235,7 +258,7 @@ TEST_F(JobQueue, PrioritizeCurrentDocumentOverVisible)
TEST_F(JobQueue, RunNothingForNotCurrentOrVisibleDocument)
{
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations));
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations));
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::CreateInitialDocumentPreamble));
documents.setVisibleInEditors({});
documents.setUsedByCurrentEditor(Utf8StringLiteral("aNonExistingFilePath"));
......@@ -247,7 +270,7 @@ TEST_F(JobQueue, RunNothingForNotCurrentOrVisibleDocument)
TEST_F(JobQueue, RunOnlyOneJobPerTranslationUnitIfMultipleAreInQueue)
{
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations));
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations));
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::CreateInitialDocumentPreamble));
const JobRequests jobsToRun = jobQueue.processQueue();
......@@ -276,9 +299,9 @@ TEST_F(JobQueue, RunJobsForDistinctTranslationUnits)
TEST_F(JobQueue, DoNotRunJobForTranslationUnittThatIsBeingProcessed)
{
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations));
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations));
jobQueue.add(createJobRequest(filePath1, JobRequest::Type::CreateInitialDocumentPreamble));
JobRequests jobsToRun = jobQueue.processQueue();
jobQueue.setIsJobRunningHandler([](const Utf8String &) {
jobQueue.setIsJobRunningForTranslationUnitHandler([](const Utf8String &) {
return true;
});
......
......@@ -108,7 +108,7 @@ TEST_F(Jobs, IsJobRunning)
jobs.add(createJobRequest(filePath1, JobRequest::Type::UpdateDocumentAnnotations));
jobs.process();
const bool isJobRunning = jobs.isJobRunning(document.translationUnit().id());
const bool isJobRunning = jobs.isJobRunningForTranslationUnit(document.translationUnit().id());
ASSERT_TRUE(isJobRunning);
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment