Bug 1083101 - Make SyncObject's list of waiting jobs lock-free. r=jrmuizel
authorNicolas Silva <nsilva@mozilla.com>
Thu, 24 Sep 2015 17:35:47 +0200
changeset 297639 c20fd7506bb727853db6bf9d462b987dd8b9aa0e
parent 297638 b5e97b0facb7708f2b95265fb190686d954de78d
child 297640 61db1a51a7c452dfb6155ec88db0752364d7184b
push id5392
push userraliiev@mozilla.com
push dateMon, 14 Dec 2015 20:08:23 +0000
treeherdermozilla-beta@16ce8562a975 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1083101
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1083101 - Make SyncObject's list of waiting jobs lock-free. r=jrmuizel
gfx/2d/JobScheduler.cpp
gfx/2d/JobScheduler.h
--- a/gfx/2d/JobScheduler.cpp
+++ b/gfx/2d/JobScheduler.cpp
@@ -79,17 +79,18 @@ JobScheduler::SubmitJob(Job* aJob)
 MultiThreadedJobQueue*
 JobScheduler::GetQueueForJob(Job* aJob)
 {
   return aJob->IsPinnedToAThread() ? aJob->GetWorkerThread()->GetJobQueue()
                                     : GetDrawingQueue();
 }
 
 Job::Job(SyncObject* aStart, SyncObject* aCompletion, WorkerThread* aThread)
-: mStartSync(aStart)
+: mNextWaitingJob(nullptr)
+, mStartSync(aStart)
 , mCompletionSync(aCompletion)
 , mPinToThread(aThread)
 {
   if (mStartSync) {
     mStartSync->AddSubsequent(this);
   }
   if (mCompletionSync) {
     mCompletionSync->AddPrerequisite(this);
@@ -119,25 +120,26 @@ SetEventJob::SetEventJob(EventObject* aE
 , mEvent(aEvent)
 {}
 
 SetEventJob::~SetEventJob()
 {}
 
 SyncObject::SyncObject(uint32_t aNumPrerequisites)
 : mSignals(aNumPrerequisites)
+, mFirstWaitingJob(nullptr)
 #ifdef DEBUG
 , mNumPrerequisites(aNumPrerequisites)
 , mAddedPrerequisites(0)
 #endif
 {}
 
 SyncObject::~SyncObject()
 {
-  MOZ_ASSERT(mWaitingJobs.size() == 0);
+  MOZ_ASSERT(mFirstWaitingJob == nullptr);
 }
 
 bool
 SyncObject::Register(Job* aJob)
 {
   MOZ_ASSERT(aJob);
 
   // For now, ensure that when we schedule the first subsequent, we have already
@@ -179,38 +181,51 @@ SyncObject::Signal()
   if (signals == 0) {
     SubmitWaitingJobs();
   }
 }
 
 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);
+  // 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);
 
-    CriticalSectionAutoEnter lock(&mWaitingJobsSection);
-    tasksToSubmit = Move(mWaitingJobs);
-    mWaitingJobs.clear();
+  // 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;
   }
 }
 
 bool
 SyncObject::IsSignaled()
 {
   return mSignals == 0;
 }
--- a/gfx/2d/JobScheduler.h
+++ b/gfx/2d/JobScheduler.h
@@ -104,19 +104,26 @@ public:
   //already_AddRefed<SyncObject> GetAndResetStartSync();
   SyncObject* GetStartSync() { return mStartSync; }
 
   bool IsPinnedToAThread() const { return !!mPinToThread; }
 
   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;
 
 /// This task will set an EventObject.
 ///
 /// Typically used as the final task, so that the main thread can block on the
 /// corresponfing EventObject until all of the tasks are processed.
@@ -200,19 +207,18 @@ private:
   // Called by Job's constructor
   void AddSubsequent(Job* aJob);
   void AddPrerequisite(Job* aJob);
 
   void AddWaitingJob(Job* aJob);
 
   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;
   Atomic<uint32_t> mAddedPrerequisites;
 #endif
 
   friend class Job;
   friend class JobScheduler;