From e18e0d36a54738743947cb84fd93e7eaecf052df Mon Sep 17 00:00:00 2001
From: Gerald Squelart <gsquelart@mozilla.com>
Date: Wed, 11 Oct 2017 14:25:21 +1100
Subject: [PATCH] Bug 1407810 - Tweak atomics in MultiWriterQueue - r=jwwang

This queue may be used heavily when multiple threads are running media code
that logs thousands of messages per second, so using less strict memory
ordering can help with speed.
Benchmarks show an improvement of up to twice the queueing speed in some
situations.

MozReview-Commit-ID: 70UOL8XAZGp

--HG--
extra : rebase_source : 38de1f6ce8a404e2ccc1591392176151edc8d078
---
 dom/media/doctor/MultiWriterQueue.h | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/dom/media/doctor/MultiWriterQueue.h b/dom/media/doctor/MultiWriterQueue.h
index 87b9cbdc5951..d154498a3714 100644
--- a/dom/media/doctor/MultiWriterQueue.h
+++ b/dom/media/doctor/MultiWriterQueue.h
@@ -300,7 +300,10 @@ private:
     T mT;
     // mValid should be atomically changed to true *after* mT has been written,
     // so that the reader can only see valid data.
-    Atomic<bool> mValid{ false };
+    // ReleaseAcquire, because when set to `true`, we want the just-written mT
+    // to be visible to the thread reading this `true`; and when set to `false`,
+    // we want the previous reads to have completed.
+    Atomic<bool, ReleaseAcquire> mValid{ false };
   };
 
   // Buffer contains a sequence of BufferedElements starting at a specific
@@ -447,7 +450,11 @@ private:
   // Index of the next element to write. Modified when an element index is
   // claimed for a push. If the last element of a buffer is claimed, that push
   // will be responsible for adding a new head buffer.
-  Atomic<Index::ValueType> mNextElementToWrite{ 0 };
+  // Relaxed, because there is no synchronization based on this variable, each
+  // thread just needs to get a different value, and will then write different
+  // things (which themselves have some atomic validation before they may be
+  // read elsewhere, independent of this `mNextElementToWrite`.)
+  Atomic<Index::ValueType, Relaxed> mNextElementToWrite{ 0 };
 
   // Index that a live recent buffer reaches. If a push claims a lesser-or-
   // equal number, the corresponding buffer is guaranteed to still be alive:
@@ -456,15 +463,21 @@ private:
   //   including the one that just claimed a position within it.
   // Also, the push that claims this exact number is responsible for adding the
   // next buffer and updating this value accordingly.
-  Atomic<Index::ValueType> mBuffersCoverAtLeastUpTo;
+  // ReleaseAcquire, because when set to a certain value, the just-created
+  // buffer covering the new range must be visible to readers.
+  Atomic<Index::ValueType, ReleaseAcquire> mBuffersCoverAtLeastUpTo;
 
   // Pointer to the most recent buffer. Never null.
   // This is the most recent of a deque of yet-unread buffers.
   // Only modified when adding a new head buffer.
-  Atomic<Buffer*> mMostRecentBuffer;
+  // ReleaseAcquire, because when modified, the just-created new buffer must be
+  // visible to readers.
+  Atomic<Buffer*, ReleaseAcquire> mMostRecentBuffer;
 
   // Stack of reusable buffers.
-  Atomic<Buffer*> mReusableBuffers;
+  // ReleaseAcquire, because when modified, the just-added buffer must be
+  // visible to readers.
+  Atomic<Buffer*, ReleaseAcquire> mReusableBuffers;
 
   // Template-provided locking mechanism to protect PopAll()-only member
   // variables below.
@@ -518,8 +531,10 @@ private:
     }
 
   private:
-    Atomic<int> mCount;
-    Atomic<int> mWatermark;
+    // Relaxed, as these are just gathering stats, so consistency is not
+    // critical.
+    Atomic<int, Relaxed> mCount;
+    Atomic<int, Relaxed> mWatermark;
   };
   // All buffers in the mMostRecentBuffer deque.
   AtomicCountAndWatermark mLiveBuffersStats;