Bug 1580091 - Use BaseProfilerMaybeMutex in BlocksRingBuffer - r=gregtatum
☠☠ backed out by 9c10aac03568 ☠ ☠
authorGerald Squelart <gsquelart@mozilla.com>
Tue, 17 Sep 2019 01:49:24 +0000
changeset 493500 79b29286f90612c83a0417748252c723d9e7c575
parent 493499 6f34c2e57ccffa57742f27aa9ac93016683e2174
child 493501 ec72dfc7301e34aded8b4e2e7a66b5baf679a1e3
push id95524
push usergsquelart@mozilla.com
push dateTue, 17 Sep 2019 05:47:40 +0000
treeherderautoland@27afea20c396 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgregtatum
bugs1580091
milestone71.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 1580091 - Use BaseProfilerMaybeMutex in BlocksRingBuffer - r=gregtatum In some situations we will *need* to use a `BlocksRingBuffer` that absolutely does not use a mutex -- In particular in the critical section of `SamplerThread::Run()`, when a thread is suspended, because any action on any mutex (even a private one that no-one else interacts with) can result in a hang. As a bonus, `BlocksRingBuffer`s that are known not to be used in multi-thread situations (e.g., backtraces, extracted stacks for duplication, etc.) will waste less resources trying to lock/unlock their mutex. Differential Revision: https://phabricator.services.mozilla.com/D45305
mozglue/baseprofiler/public/BlocksRingBuffer.h
mozglue/tests/TestBaseProfiler.cpp
tools/profiler/tests/gtest/GeckoProfiler.cpp
--- a/mozglue/baseprofiler/public/BlocksRingBuffer.h
+++ b/mozglue/baseprofiler/public/BlocksRingBuffer.h
@@ -164,149 +164,166 @@ class BlocksRingBuffer {
     // between `BlockIndex` and `Index`.
     friend class BlocksRingBuffer;
     explicit BlockIndex(Index aBlockIndex) : mBlockIndex(aBlockIndex) {}
     explicit operator Index() const { return mBlockIndex; }
 
     Index mBlockIndex;
   };
 
+  enum class ThreadSafety { WithoutMutex, WithMutex };
+
   // Default constructor starts out-of-session (nothing to read or write).
-  BlocksRingBuffer() = default;
+  explicit BlocksRingBuffer(ThreadSafety aThreadSafety)
+      : mMutex(aThreadSafety != ThreadSafety::WithoutMutex) {}
 
   // Constructors with no entry destructor, the oldest entries will be silently
   // overwritten/destroyed.
 
   // Create a buffer of the given length.
-  explicit BlocksRingBuffer(PowerOfTwo<Length> aLength)
-      : mMaybeUnderlyingBuffer(Some(UnderlyingBuffer(aLength))) {}
+  explicit BlocksRingBuffer(ThreadSafety aThreadSafety,
+                            PowerOfTwo<Length> aLength)
+      : mMutex(aThreadSafety != ThreadSafety::WithoutMutex),
+        mMaybeUnderlyingBuffer(Some(UnderlyingBuffer(aLength))) {}
 
   // Take ownership of an existing buffer.
-  BlocksRingBuffer(UniquePtr<Buffer::Byte[]> aExistingBuffer,
+  BlocksRingBuffer(ThreadSafety aThreadSafety,
+                   UniquePtr<Buffer::Byte[]> aExistingBuffer,
                    PowerOfTwo<Length> aLength)
-      : mMaybeUnderlyingBuffer(
+      : mMutex(aThreadSafety != ThreadSafety::WithoutMutex),
+        mMaybeUnderlyingBuffer(
             Some(UnderlyingBuffer(std::move(aExistingBuffer), aLength))) {}
 
   // Use an externally-owned buffer.
-  BlocksRingBuffer(Buffer::Byte* aExternalBuffer, PowerOfTwo<Length> aLength)
-      : mMaybeUnderlyingBuffer(
+  BlocksRingBuffer(ThreadSafety aThreadSafety, Buffer::Byte* aExternalBuffer,
+                   PowerOfTwo<Length> aLength)
+      : mMutex(aThreadSafety != ThreadSafety::WithoutMutex),
+        mMaybeUnderlyingBuffer(
             Some(UnderlyingBuffer(aExternalBuffer, aLength))) {}
 
   // Constructors with an entry destructor, which will be called with an
   // `EntryReader` before the oldest entries get overwritten/destroyed.
   // Note that this entry destructor may be invoked from another caller's
   // function that writes/clears data, be aware of this re-entrancy! (Details
   // above class.)
 
   // Create a buffer of the given length.
   template <typename EntryDestructor>
-  explicit BlocksRingBuffer(PowerOfTwo<Length> aLength,
+  explicit BlocksRingBuffer(ThreadSafety aThreadSafety,
+                            PowerOfTwo<Length> aLength,
                             EntryDestructor&& aEntryDestructor)
-      : mMaybeUnderlyingBuffer(Some(UnderlyingBuffer(
+      : mMutex(aThreadSafety != ThreadSafety::WithoutMutex),
+        mMaybeUnderlyingBuffer(Some(UnderlyingBuffer(
             aLength, std::forward<EntryDestructor>(aEntryDestructor)))) {}
 
   // Take ownership of an existing buffer.
   template <typename EntryDestructor>
-  explicit BlocksRingBuffer(UniquePtr<Buffer::Byte[]> aExistingBuffer,
+  explicit BlocksRingBuffer(ThreadSafety aThreadSafety,
+                            UniquePtr<Buffer::Byte[]> aExistingBuffer,
                             PowerOfTwo<Length> aLength,
                             EntryDestructor&& aEntryDestructor)
-      : mMaybeUnderlyingBuffer(Some(UnderlyingBuffer(
+      : mMutex(aThreadSafety != ThreadSafety::WithoutMutex),
+        mMaybeUnderlyingBuffer(Some(UnderlyingBuffer(
             std::move(aExistingBuffer), aLength,
             std::forward<EntryDestructor>(aEntryDestructor)))) {}
 
   // Use an externally-owned buffer.
   template <typename EntryDestructor>
-  explicit BlocksRingBuffer(Buffer::Byte* aExternalBuffer,
+  explicit BlocksRingBuffer(ThreadSafety aThreadSafety,
+                            Buffer::Byte* aExternalBuffer,
                             PowerOfTwo<Length> aLength,
                             EntryDestructor&& aEntryDestructor)
-      : mMaybeUnderlyingBuffer(Some(UnderlyingBuffer(
+      : mMutex(aThreadSafety != ThreadSafety::WithoutMutex),
+        mMaybeUnderlyingBuffer(Some(UnderlyingBuffer(
             aExternalBuffer, aLength,
             std::forward<EntryDestructor>(aEntryDestructor)))) {}
 
   // Destructor explictly destroys all remaining entries, this may invoke the
   // caller-provided entry destructor.
   ~BlocksRingBuffer() {
 #ifdef DEBUG
     // Needed because of lock DEBUG-check in `DestroyAllEntries()`.
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
 #endif  // DEBUG
     DestroyAllEntries();
   }
 
   // Remove underlying buffer, if any.
   void Reset() {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     ResetUnderlyingBuffer();
   }
 
   // Create a buffer of the given length.
   void Set(PowerOfTwo<Length> aLength) {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     ResetUnderlyingBuffer();
     mMaybeUnderlyingBuffer.emplace(aLength);
   }
 
   // Take ownership of an existing buffer.
   void Set(UniquePtr<Buffer::Byte[]> aExistingBuffer,
            PowerOfTwo<Length> aLength) {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     ResetUnderlyingBuffer();
     mMaybeUnderlyingBuffer.emplace(std::move(aExistingBuffer), aLength);
   }
 
   // Use an externally-owned buffer.
   void Set(Buffer::Byte* aExternalBuffer, PowerOfTwo<Length> aLength) {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     ResetUnderlyingBuffer();
     mMaybeUnderlyingBuffer.emplace(aExternalBuffer, aLength);
   }
 
   // Create a buffer of the given length, with entry destructor.
   template <typename EntryDestructor>
   void Set(PowerOfTwo<Length> aLength, EntryDestructor&& aEntryDestructor) {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     ResetUnderlyingBuffer();
     mMaybeUnderlyingBuffer.emplace(
         aLength, std::forward<EntryDestructor>(aEntryDestructor));
   }
 
   // Take ownership of an existing buffer, with entry destructor.
   template <typename EntryDestructor>
   void Set(UniquePtr<Buffer::Byte[]> aExistingBuffer,
            PowerOfTwo<Length> aLength, EntryDestructor&& aEntryDestructor) {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     ResetUnderlyingBuffer();
     mMaybeUnderlyingBuffer.emplace(
         std::move(aExistingBuffer), aLength,
         std::forward<EntryDestructor>(aEntryDestructor));
   }
 
   // Use an externally-owned buffer, with entry destructor.
   template <typename EntryDestructor>
   void Set(Buffer::Byte* aExternalBuffer, PowerOfTwo<Length> aLength,
            EntryDestructor&& aEntryDestructor) {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     ResetUnderlyingBuffer();
     mMaybeUnderlyingBuffer.emplace(
         aExternalBuffer, aLength,
         std::forward<EntryDestructor>(aEntryDestructor));
   }
 
+  bool IsThreadSafe() const { return mMutex.IsActivated(); }
+
   // Lock the buffer mutex and run the provided callback.
   // This can be useful when the caller needs to explicitly lock down this
   // buffer, but not do anything else with it.
   template <typename Callback>
   auto LockAndRun(Callback&& aCallback) const {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     return std::forward<Callback>(aCallback)();
   }
 
   // Buffer length in bytes.
   Maybe<PowerOfTwo<Length>> BufferLength() const {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     return mMaybeUnderlyingBuffer.map([](const UnderlyingBuffer& aBuffer) {
       return aBuffer.mBuffer.BufferLength();
     });
     ;
   }
 
   // Size of external resources.
   // Note: `mEntryDestructor`'s potential external data (for its captures) is
@@ -339,17 +356,17 @@ class BlocksRingBuffer {
   };
 
   // Get a snapshot of the current state.
   // When out-of-session, mFirstReadIndex==mNextWriteIndex, and
   // mPushedBlockCount==mClearedBlockCount==0.
   // Note that these may change right after this thread-safe call, so they
   // should only be used for statistical purposes.
   State GetState() const {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     return {
         mFirstReadIndex, mNextWriteIndex,
         mMaybeUnderlyingBuffer ? mMaybeUnderlyingBuffer->mPushedBlockCount : 0,
         mMaybeUnderlyingBuffer ? mMaybeUnderlyingBuffer->mClearedBlockCount
                                : 0};
   }
 
   // No objects, no bytes! (May be useful for generic programming.)
@@ -637,17 +654,17 @@ class BlocksRingBuffer {
   };
 
   // Call `aCallback(BlocksRingBuffer::Reader*)` (nullptr when out-of-session),
   // and return whatever `aCallback` returns. Callback should not store
   // `Reader`, because it may become invalid after this call.
   template <typename Callback>
   auto Read(Callback&& aCallback) const {
     {
-      baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+      baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
       if (MOZ_LIKELY(mMaybeUnderlyingBuffer)) {
         Reader reader(*this);
         return std::forward<Callback>(aCallback)(&reader);
       }
     }
     return std::forward<Callback>(aCallback)(nullptr);
   }
 
@@ -665,17 +682,17 @@ class BlocksRingBuffer {
 
   // Call `aCallback(Maybe<BlocksRingBuffer::EntryReader>&&)` on the entry at
   // the given BlockIndex; The `Maybe` will be `Nothing` if out-of-session, or
   // if that entry doesn't exist anymore, or if we've reached just past the
   // last entry. Return whatever `aCallback` returns. Callback should not
   // store `EntryReader`, because it may become invalid after this call.
   template <typename Callback>
   auto ReadAt(BlockIndex aBlockIndex, Callback&& aCallback) const {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     MOZ_ASSERT(aBlockIndex <= mNextWriteIndex);
     Maybe<EntryReader> maybeEntryReader;
     if (MOZ_LIKELY(mMaybeUnderlyingBuffer) && aBlockIndex >= mFirstReadIndex &&
         aBlockIndex < mNextWriteIndex) {
       AssertBlockIndexIsValid(aBlockIndex);
       maybeEntryReader.emplace(ReaderInBlockAt(aBlockIndex));
     }
     return std::forward<Callback>(aCallback)(std::move(maybeEntryReader));
@@ -809,17 +826,17 @@ class BlocksRingBuffer {
   // whatever `aCallback` returns. Callback should not store `EntryWriter`,
   // because it may become invalid after this thread-safe call.
   // Note: `aCallbackBytes` is a callback instead of a simple value, to delay
   // this potentially-expensive computation until after we're checked that we're
   // in-session; use `Put(Length, Callback)` below if you know the size already.
   template <typename CallbackBytes, typename Callback>
   auto ReserveAndPut(CallbackBytes aCallbackBytes, Callback&& aCallback) {
     {  // Locked block.
-      baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+      baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
       if (MOZ_LIKELY(mMaybeUnderlyingBuffer)) {
         Length bytes = std::forward<CallbackBytes>(aCallbackBytes)();
         // Don't allow even half of the buffer length. More than that would
         // probably be unreasonable, and much more would risk having an entry
         // wrapping around and overwriting itself!
         MOZ_RELEASE_ASSERT(
             bytes < mMaybeUnderlyingBuffer->mBuffer.BufferLength().Value() / 2);
         // COmpute block size from the requested entry size.
@@ -899,25 +916,25 @@ class BlocksRingBuffer {
   template <typename T>
   BlockIndex PutObject(const T& aOb) {
     return PutObjects(aOb);
   }
 
   // Clear all entries, calling entry destructor (if any), and move read index
   // to the end so that these entries cannot be read anymore.
   void Clear() {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     ClearAllEntries();
   }
 
   // Clear all entries strictly before aBlockIndex, calling calling entry
   // destructor (if any), and move read index to the end so that these entries
   // cannot be read anymore.
   void ClearBefore(BlockIndex aBlockIndex) {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     if (!mMaybeUnderlyingBuffer) {
       return;
     }
     // Don't accept a not-yet-written index. One-past-the-end is ok.
     MOZ_ASSERT(aBlockIndex <= mNextWriteIndex);
     if (aBlockIndex <= mFirstReadIndex) {
       // Already cleared.
       return;
@@ -952,17 +969,17 @@ class BlocksRingBuffer {
     }
     // Move read index to given index, so there's effectively no more entries
     // before.
     mFirstReadIndex = aBlockIndex;
   }
 
 #ifdef DEBUG
   void Dump() const {
-    baseprofiler::detail::BaseProfilerAutoLock lock(mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(mMutex);
     if (!mMaybeUnderlyingBuffer) {
       printf("empty BlocksRingBuffer\n");
       return;
     }
     using ULL = unsigned long long;
     printf("start=%llu (%llu) end=%llu (%llu) - ", ULL(Index(mFirstReadIndex)),
            ULL(Index(mFirstReadIndex) &
                (mMaybeUnderlyingBuffer->mBuffer.BufferLength().Value() - 1)),
@@ -1082,17 +1099,17 @@ class BlocksRingBuffer {
 
   // Used to de/serialize a BlocksRingBuffer (e.g., containing a backtrace).
   friend struct Serializer<BlocksRingBuffer>;
   friend struct Deserializer<BlocksRingBuffer>;
   friend struct Serializer<UniquePtr<BlocksRingBuffer>>;
   friend struct Deserializer<UniquePtr<BlocksRingBuffer>>;
 
   // Mutex guarding the following members.
-  mutable baseprofiler::detail::BaseProfilerMutex mMutex;
+  mutable baseprofiler::detail::BaseProfilerMaybeMutex mMutex;
 
   struct UnderlyingBuffer {
     // Create a buffer of the given length.
     explicit UnderlyingBuffer(PowerOfTwo<Length> aLength) : mBuffer(aLength) {}
 
     // Take ownership of an existing buffer.
     UnderlyingBuffer(UniquePtr<Buffer::Byte[]> aExistingBuffer,
                      PowerOfTwo<Length> aLength)
@@ -1864,17 +1881,17 @@ struct BlocksRingBuffer::Deserializer<Va
 // A BlocksRingBuffer can hide another one!
 // This will be used to store marker backtraces; They can be read back into a
 // UniquePtr<BlocksRingBuffer>.
 // Format: len (ULEB128) | start | end | buffer (len bytes) | pushed | cleared
 // len==0 marks an out-of-session buffer, or empty buffer.
 template <>
 struct BlocksRingBuffer::Serializer<BlocksRingBuffer> {
   static Length Bytes(const BlocksRingBuffer& aBuffer) {
-    baseprofiler::detail::BaseProfilerAutoLock lock(aBuffer.mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(aBuffer.mMutex);
     if (aBuffer.mMaybeUnderlyingBuffer.isNothing()) {
       // Out-of-session, we only need 1 byte to store a length of 0.
       return ULEB128Size<Length>(0);
     }
     const auto start = Index(aBuffer.mFirstReadIndex);
     const auto end = Index(aBuffer.mNextWriteIndex);
     const auto len = end - start;
     if (len == 0) {
@@ -1882,17 +1899,17 @@ struct BlocksRingBuffer::Serializer<Bloc
       return ULEB128Size<Length>(0);
     }
     return ULEB128Size(len) + sizeof(start) + sizeof(end) + len +
            sizeof(aBuffer.mMaybeUnderlyingBuffer->mPushedBlockCount) +
            sizeof(aBuffer.mMaybeUnderlyingBuffer->mClearedBlockCount);
   }
 
   static void Write(EntryWriter& aEW, const BlocksRingBuffer& aBuffer) {
-    baseprofiler::detail::BaseProfilerAutoLock lock(aBuffer.mMutex);
+    baseprofiler::detail::BaseProfilerMaybeAutoLock lock(aBuffer.mMutex);
     if (aBuffer.mMaybeUnderlyingBuffer.isNothing()) {
       // Out-of-session, only store a length of 0.
       aEW.WriteULEB128<Length>(0);
       return;
     }
     const auto start = Index(aBuffer.mFirstReadIndex);
     const auto end = Index(aBuffer.mNextWriteIndex);
     MOZ_ASSERT(end - start <= std::numeric_limits<Length>::max());
@@ -2009,18 +2026,19 @@ struct BlocksRingBuffer::Deserializer<Un
     UniquePtr<BlocksRingBuffer> bufferUPtr;
     // Read the stored buffer length.
     const auto len = aER.ReadULEB128<BlocksRingBuffer::Length>();
     if (len == 0) {
       // 0-length means an "uninteresting" buffer, just return nullptr.
       return bufferUPtr;
     }
     // We have a non-empty buffer.
-    // allocate an empty BlocksRingBuffer.
-    bufferUPtr = MakeUnique<BlocksRingBuffer>();
+    // allocate an empty BlocksRingBuffer without mutex.
+    bufferUPtr = MakeUnique<BlocksRingBuffer>(
+        BlocksRingBuffer::ThreadSafety::WithoutMutex);
     // Rewind the reader before the length and deserialize the contents, using
     // the non-UniquePtr Deserializer.
     aER -= ULEB128Size(len);
     aER.ReadIntoObject(*bufferUPtr);
     return bufferUPtr;
   }
 };
 
--- a/mozglue/tests/TestBaseProfiler.cpp
+++ b/mozglue/tests/TestBaseProfiler.cpp
@@ -507,17 +507,18 @@ void TestBlocksRingBufferAPI() {
   constexpr uint32_t MBSize = 16;
   uint8_t buffer[MBSize * 3];
   for (size_t i = 0; i < MBSize * 3; ++i) {
     buffer[i] = uint8_t('A' + i);
   }
 
   // Start a temporary block to constrain buffer lifetime.
   {
-    BlocksRingBuffer rb(&buffer[MBSize], MakePowerOfTwo32<MBSize>(),
+    BlocksRingBuffer rb(BlocksRingBuffer::ThreadSafety::WithMutex,
+                        &buffer[MBSize], MakePowerOfTwo32<MBSize>(),
                         [&](BlocksRingBuffer::EntryReader& aReader) {
                           lastDestroyed = aReader.ReadObject<uint32_t>();
                         });
 
 #  define VERIFY_START_END_DESTROYED(aStart, aEnd, aLastDestroyed)          \
     {                                                                       \
       BlocksRingBuffer::State state = rb.GetState();                        \
       MOZ_RELEASE_ASSERT(ExtractBlockIndex(state.mRangeStart) == (aStart)); \
@@ -849,17 +850,17 @@ void TestBlocksRingBufferAPI() {
 
   printf("TestBlocksRingBufferAPI done\n");
 }
 
 void TestBlocksRingBufferUnderlyingBufferChanges() {
   printf("TestBlocksRingBufferUnderlyingBufferChanges...\n");
 
   // Out-of-session BlocksRingBuffer to start with.
-  BlocksRingBuffer rb;
+  BlocksRingBuffer rb(BlocksRingBuffer::ThreadSafety::WithMutex);
 
   // Block index to read at. Initially "null", but may be changed below.
   BlocksRingBuffer::BlockIndex bi;
 
   // Test all rb APIs when rb is out-of-session and therefore doesn't have an
   // underlying buffer.
   auto testOutOfSession = [&]() {
     MOZ_RELEASE_ASSERT(rb.BufferLength().isNothing());
@@ -1055,17 +1056,18 @@ void TestBlocksRingBufferThreading() {
   // Entry destructor will store about-to-be-cleared value in `lastDestroyed`.
   std::atomic<int> lastDestroyed{0};
 
   constexpr uint32_t MBSize = 8192;
   uint8_t buffer[MBSize * 3];
   for (size_t i = 0; i < MBSize * 3; ++i) {
     buffer[i] = uint8_t('A' + i);
   }
-  BlocksRingBuffer rb(&buffer[MBSize], MakePowerOfTwo32<MBSize>(),
+  BlocksRingBuffer rb(BlocksRingBuffer::ThreadSafety::WithMutex,
+                      &buffer[MBSize], MakePowerOfTwo32<MBSize>(),
                       [&](BlocksRingBuffer::EntryReader& aReader) {
                         lastDestroyed = aReader.ReadObject<int>();
                       });
 
   // Start reader thread.
   std::atomic<bool> stopReader{false};
   std::thread reader([&]() {
     for (;;) {
@@ -1143,17 +1145,18 @@ void TestBlocksRingBufferThreading() {
 void TestBlocksRingBufferSerialization() {
   printf("TestBlocksRingBufferSerialization...\n");
 
   constexpr uint32_t MBSize = 64;
   uint8_t buffer[MBSize * 3];
   for (size_t i = 0; i < MBSize * 3; ++i) {
     buffer[i] = uint8_t('A' + i);
   }
-  BlocksRingBuffer rb(&buffer[MBSize], MakePowerOfTwo32<MBSize>());
+  BlocksRingBuffer rb(BlocksRingBuffer::ThreadSafety::WithMutex,
+                      &buffer[MBSize], MakePowerOfTwo32<MBSize>());
 
   // Will expect literal string to always have the same address.
 #  define THE_ANSWER "The answer is "
   const char* theAnswer = THE_ANSWER;
 
   rb.PutObjects('0', WrapBlocksRingBufferLiteralCStringPointer(THE_ANSWER), 42,
                 std::string(" but pi="), 3.14);
   rb.ReadEach([&](BlocksRingBuffer::EntryReader& aER) {
@@ -1273,25 +1276,27 @@ void TestBlocksRingBufferSerialization()
 
   // 2nd BlocksRingBuffer to contain the 1st one. It has be be more than twice
   // the size.
   constexpr uint32_t MBSize2 = MBSize * 4;
   uint8_t buffer2[MBSize2 * 3];
   for (size_t i = 0; i < MBSize2 * 3; ++i) {
     buffer2[i] = uint8_t('B' + i);
   }
-  BlocksRingBuffer rb2(&buffer2[MBSize2], MakePowerOfTwo32<MBSize2>());
+  BlocksRingBuffer rb2(BlocksRingBuffer::ThreadSafety::WithoutMutex,
+                       &buffer2[MBSize2], MakePowerOfTwo32<MBSize2>());
   rb2.PutObject(rb);
 
   // 3rd BlocksRingBuffer deserialized from the 2nd one.
   uint8_t buffer3[MBSize * 3];
   for (size_t i = 0; i < MBSize * 3; ++i) {
     buffer3[i] = uint8_t('C' + i);
   }
-  BlocksRingBuffer rb3(&buffer3[MBSize], MakePowerOfTwo32<MBSize>());
+  BlocksRingBuffer rb3(BlocksRingBuffer::ThreadSafety::WithoutMutex,
+                       &buffer3[MBSize], MakePowerOfTwo32<MBSize>());
   rb2.ReadEach(
       [&](BlocksRingBuffer::EntryReader& aER) { aER.ReadIntoObject(rb3); });
 
   // And a 4th heap-allocated one.
   UniquePtr<BlocksRingBuffer> rb4up;
   rb2.ReadEach([&](BlocksRingBuffer::EntryReader& aER) {
     rb4up = aER.ReadObject<UniquePtr<BlocksRingBuffer>>();
   });
--- a/tools/profiler/tests/gtest/GeckoProfiler.cpp
+++ b/tools/profiler/tests/gtest/GeckoProfiler.cpp
@@ -35,17 +35,18 @@ using namespace mozilla;
 
 TEST(BaseProfiler, BlocksRingBuffer)
 {
   constexpr uint32_t MBSize = 256;
   uint8_t buffer[MBSize * 3];
   for (size_t i = 0; i < MBSize * 3; ++i) {
     buffer[i] = uint8_t('A' + i);
   }
-  BlocksRingBuffer rb(&buffer[MBSize], MakePowerOfTwo32<MBSize>());
+  BlocksRingBuffer rb(BlocksRingBuffer::ThreadSafety::WithMutex,
+                      &buffer[MBSize], MakePowerOfTwo32<MBSize>());
 
   {
     nsCString cs(NS_LITERAL_CSTRING("nsCString"));
     nsString s(NS_LITERAL_STRING("nsString"));
     nsAutoCString acs(NS_LITERAL_CSTRING("nsAutoCString"));
     nsAutoString as(NS_LITERAL_STRING("nsAutoString"));
     nsAutoCStringN<8> acs8(NS_LITERAL_CSTRING("nsAutoCStringN"));
     nsAutoStringN<8> as8(NS_LITERAL_STRING("nsAutoStringN"));