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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Does this class have unit test coverage?



================
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) {
----------------
Remove the Mutex() here, since that's the default (& probably () for the Finalizing initialization for consistency)?


================
Comment at: lib/xray/xray_buffer_queue.cc:48
     return ErrorCode::QueueFinalizing;
-  __sanitizer::BlockingMutexLock Guard(&Mutex);
-  if (Buffers.empty())
+  __sanitizer::SpinMutexLock Guard(&Mutex);
+
----------------
Why the change in locks/locking scheme?


================
Comment at: lib/xray/xray_buffer_queue.cc:66
+  // Blitz through the buffers array to find the buffer.
+  if (!std::any_of(OwnedBuffers.get(), OwnedBuffers.get() + BufferCount,
+                   [&Buf](void *P) { return P == Buf.Buffer; }))
----------------
std::none_of?


================
Comment at: lib/xray/xray_buffer_queue.cc:94
 BufferQueue::~BufferQueue() {
-  for (auto &T : Buffers) {
+  for (auto I = Buffers.get(); I != Buffers.get() + BufferCount; ++I) {
+    auto &T = *I;
----------------
Maybe here and other similar loops:

  for (auto I = Buffers.get(), E = I + BufferCount; I != E; ++I)

(there's an LLVM style guide rule/suggestion to cache the end)


https://reviews.llvm.org/D38119