[PATCH] D38119: [XRay][compiler-rt] Use a hand-written circular buffer in BufferQueue

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added a comment.

In https://reviews.llvm.org/D38119#882394, @dblaikie wrote:

> Does this class have unit test coverage?


Yes.



================
Comment at: lib/xray/xray_buffer_queue.cc:28
+    : BufferSize(B), Buffers(new std::tuple<Buffer, bool>[N]()), BufferCount(N),
+      Mutex(), Finalizing{0}, OwnedBuffers(new void *[N]()),
+      Next(Buffers.get()), First(nullptr) {
----------------
dblaikie wrote:
> Remove the Mutex() here, since that's the default (& probably () for the Finalizing initialization for consistency)?
We can't use `(...)` for Finalizing because it's an aggregate (no constructors).


================
Comment at: lib/xray/xray_buffer_queue.cc:48
     return ErrorCode::QueueFinalizing;
-  __sanitizer::BlockingMutexLock Guard(&Mutex);
-  if (Buffers.empty())
+  __sanitizer::SpinMutexLock Guard(&Mutex);
+
----------------
dblaikie wrote:
> Why the change in locks/locking scheme?
We explain it in the commit description -- since we no longer need to be allocating memory, and we posit that the bodies of these functions are now quicker, that we'd have a better chance to not causing too much latency due to contention for these functions.


https://reviews.llvm.org/D38119