Bug 1083101 - Make SyncObject's waiting jobs list lock-free. r=jrmuizel

Author: nical

gfx/2d/JobScheduler.cpp

@@ -84,7 +84,8 @@ JobScheduler::GetQueueForJob(Job* aJob)
 }
 
 Job::Job(SyncObject* aStart, SyncObject* aCompletion, WorkerThread* aThread)
-: mStartSync(aStart)
+: mNextWaitingJob(nullptr)
+, mStartSync(aStart)
 , mCompletionSync(aCompletion)
 , mPinToThread(aThread)
 {
@@ -124,6 +125,7 @@ SetEventJob::~SetEventJob()
 
 SyncObject::SyncObject(uint32_t aNumPrerequisites)
 : mSignals(aNumPrerequisites)
+, mFirstWaitingJob(nullptr)
 #ifdef DEBUG
 , mNumPrerequisites(aNumPrerequisites)
 , mAddedPrerequisites(0)
@@ -132,7 +134,7 @@ SyncObject::SyncObject(uint32_t aNumPrerequisites)
 
 SyncObject::~SyncObject()
 {
-  MOZ_ASSERT(mWaitingJobs.size() == 0);
+  MOZ_ASSERT(mFirstWaitingJob == nullptr);
 }
 
 bool
@@ -184,28 +186,41 @@ SyncObject::Signal()
 void
 SyncObject::AddWaitingJob(Job* aJob)
 {
-  CriticalSectionAutoEnter lock(&mWaitingJobsSection);
-  mWaitingJobs.push_back(aJob);
+  // Push (using atomics) the task into the list of waiting tasks.
+  for (;;) {
+    Job* first = mFirstWaitingJob;
+    aJob->mNextWaitingJob = first;
+    if (mFirstWaitingJob.compareExchange(first, aJob)) {
+      break;
+    }
+  }
 }
 
 void SyncObject::SubmitWaitingJobs()
 {
-  std::vector<Job*> tasksToSubmit;
-  {
-    // Scheduling the tasks can cause code that modifies <this>'s reference
-    // count to run concurrently, and cause the caller of this function to
-    // be owned by another thread. We need to make sure the reference count
-    // does not reach 0 on another thread before mWaitingJobs.clear(), so
-    // hold a strong ref to prevent that!
-    RefPtr<SyncObject> kungFuDeathGrip(this);
-
-    CriticalSectionAutoEnter lock(&mWaitingJobsSection);
-    tasksToSubmit = Move(mWaitingJobs);
-    mWaitingJobs.clear();
+  // Scheduling the tasks can cause code that modifies <this>'s reference
+  // count to run concurrently, and cause the caller of this function to
+  // be owned by another thread. We need to make sure the reference count
+  // does not reach 0 on another thread before the end of this method, so
+  // hold a strong ref to prevent that!
+  RefPtr<SyncObject> kungFuDeathGrip(this);
+
+  // First atomically swap mFirstWaitingJob and waitingJobs...
+  Job* waitingJobs = nullptr;
+  for (;;) {
+    waitingJobs = mFirstWaitingJob;
+    if (mFirstWaitingJob.compareExchange(waitingJobs, nullptr)) {
+      break;
+    }
   }
 
-  for (Job* task : tasksToSubmit) {
-    JobScheduler::GetQueueForJob(task)->SubmitJob(task);
+  // ... and submit all of the waiting tasks in waitingJob now that they belong
+  // to this thread.
+  while (waitingJobs) {
+    Job* next = waitingJobs->mNextWaitingJob;
+    waitingJobs->mNextWaitingJob = nullptr;
+    JobScheduler::GetQueueForJob(waitingJobs)->SubmitJob(waitingJobs);
+    waitingJobs = next;
   }
 }
 

gfx/2d/JobScheduler.h

@@ -109,9 +109,16 @@ class Job {
   WorkerThread* GetWorkerThread() { return mPinToThread; }
 
 protected:
+  // An intrusive linked list of tasks waiting for a sync object to enter the
+  // signaled state. When the task is not waiting for a sync object, mNextWaitingJob
+  // should be null. This is only accessed from the thread that owns the task.
+  Job* mNextWaitingJob;
+
   RefPtr<SyncObject> mStartSync;
   RefPtr<SyncObject> mCompletionSync;
   WorkerThread* mPinToThread;
+
+  friend class SyncObject;
 };
 
 class EventObject;
@@ -205,9 +212,8 @@ class SyncObject final : public external::AtomicRefCounted<SyncObject> {
 
   void SubmitWaitingJobs();
 
-  std::vector<Job*> mWaitingJobs;
-  CriticalSection mWaitingJobsSection; // for concurrent access to mWaintingJobs
   Atomic<int32_t> mSignals;
+  Atomic<Job*> mFirstWaitingJob;
 
 #ifdef DEBUG
   uint32_t mNumPrerequisites;