Revert "[XRay][compiler-rt] Use a hand-written circular buffer in BufferQueue"

This reverts r314766 (rL314766). Unit tests fail in multiple bots.

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@314786 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 32 insertions, 88 deletions

diff --git a/lib/xray/xray_buffer_queue.cc b/lib/xray/xray_buffer_queue.cc
index 99175a025..7ba755ac3 100644
--- a/lib/xray/xray_buffer_queue.cc
+++ b/lib/xray/xray_buffer_queue.cc
@@ -16,7 +16,6 @@
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_libc.h"
 
-#include <algorithm>
 #include <cstdlib>
 #include <tuple>
 
@@ -24,21 +23,18 @@ using namespace __xray;
 using namespace __sanitizer;
 
 BufferQueue::BufferQueue(std::size_t B, std::size_t N, bool &Success)
-    : BufferSize(B), Buffers(new std::tuple<Buffer, bool>[N]()),
-      BufferCount(N), Finalizing{0}, OwnedBuffers(new void *[N]()),
-      Next(Buffers.get()), First(nullptr) {
-  for (size_t i = 0; i < N; ++i) {
-    auto &T = Buffers[i];
+    : BufferSize(B), Buffers(N), Mutex(), OwnedBuffers(), Finalizing{0} {
+  for (auto &T : Buffers) {
     void *Tmp = malloc(BufferSize);
     if (Tmp == nullptr) {
       Success = false;
       return;
     }
+
     auto &Buf = std::get<0>(T);
-    std::get<1>(T) = false;
     Buf.Buffer = Tmp;
     Buf.Size = B;
-    OwnedBuffers[i] = Tmp;
+    OwnedBuffers.emplace(Tmp);
   }
   Success = true;
 }
@@ -46,43 +42,27 @@ BufferQueue::BufferQueue(std::size_t B, std::size_t N, bool &Success)
 BufferQueue::ErrorCode BufferQueue::getBuffer(Buffer &Buf) {
   if (__sanitizer::atomic_load(&Finalizing, __sanitizer::memory_order_acquire))
     return ErrorCode::QueueFinalizing;
-  __sanitizer::SpinMutexLock Guard(&Mutex);
-
-  if (Next == First)
+  __sanitizer::BlockingMutexLock Guard(&Mutex);
+  if (Buffers.empty())
     return ErrorCode::NotEnoughMemory;
-
-  auto &T = *Next;
+  auto &T = Buffers.front();
   auto &B = std::get<0>(T);
   Buf = B;
-
-  if (First == nullptr)
-    First = Next;
-  ++Next;
-  if (Next == (Buffers.get() + BufferCount))
-    Next = Buffers.get();
-
+  B.Buffer = nullptr;
+  B.Size = 0;
+  Buffers.pop_front();
   return ErrorCode::Ok;
 }
 
 BufferQueue::ErrorCode BufferQueue::releaseBuffer(Buffer &Buf) {
-  // Blitz through the buffers array to find the buffer.
-  if (std::none_of(OwnedBuffers.get(), OwnedBuffers.get() + BufferCount,
-                   [&Buf](void *P) { return P == Buf.Buffer; }))
+  if (OwnedBuffers.count(Buf.Buffer) == 0)
     return ErrorCode::UnrecognizedBuffer;
-  __sanitizer::SpinMutexLock Guard(&Mutex);
-
-  // This points to a semantic bug, we really ought to not be releasing more
-  // buffers than we actually get.
-  if (First == nullptr || First == Next)
-    return ErrorCode::NotEnoughMemory;
+  __sanitizer::BlockingMutexLock Guard(&Mutex);
 
   // Now that the buffer has been released, we mark it as "used".
-  *First = std::make_tuple(Buf, true);
+  Buffers.emplace(Buffers.end(), Buf, true /* used */);
   Buf.Buffer = nullptr;
   Buf.Size = 0;
-  ++First;
-  if (First == (Buffers.get() + BufferCount))
-    First = Buffers.get();
   return ErrorCode::Ok;
 }
 
@@ -94,8 +74,7 @@ BufferQueue::ErrorCode BufferQueue::finalize() {
 }
 
 BufferQueue::~BufferQueue() {
-  for (auto I = Buffers.get(), E = Buffers.get() + BufferCount; I != E; ++I) {
-    auto &T = *I;
+  for (auto &T : Buffers) {
     auto &Buf = std::get<0>(T);
     free(Buf.Buffer);
   }
diff --git a/lib/xray/xray_buffer_queue.h b/lib/xray/xray_buffer_queue.h
index 1115b4722..bd382a26c 100644
--- a/lib/xray/xray_buffer_queue.h
+++ b/lib/xray/xray_buffer_queue.h
@@ -17,8 +17,8 @@
 
 #include "sanitizer_common/sanitizer_atomic.h"
 #include "sanitizer_common/sanitizer_mutex.h"
-#include <cstdint>
-#include <memory>
+#include <deque>
+#include <unordered_set>
 #include <utility>
 
 namespace __xray {
@@ -36,23 +36,15 @@ public:
   };
 
 private:
-  // Size of each individual Buffer.
   size_t BufferSize;
 
   // We use a bool to indicate whether the Buffer has been used in this
   // freelist implementation.
-  std::unique_ptr<std::tuple<Buffer, bool>[]> Buffers;
-  size_t BufferCount;
-
-  __sanitizer::SpinMutex Mutex;
+  std::deque<std::tuple<Buffer, bool>> Buffers;
+  __sanitizer::BlockingMutex Mutex;
+  std::unordered_set<void *> OwnedBuffers;
   __sanitizer::atomic_uint8_t Finalizing;
 
-  // Sorted buffer pointers, making it quick to find buffers that we own.
-  std::unique_ptr<void *[]> OwnedBuffers;
-
-  std::tuple<Buffer, bool> *Next;
-  std::tuple<Buffer, bool> *First;
-
 public:
   enum class ErrorCode : unsigned {
     Ok,
@@ -125,9 +117,8 @@ public:
   /// Buffer is marked 'used' (i.e. has been the result of getBuffer(...) and a
   /// releaseBuffer(...) operation).
   template <class F> void apply(F Fn) {
-    __sanitizer::SpinMutexLock G(&Mutex);
-    for (auto I = Buffers.get(), E = Buffers.get() + BufferCount; I != E; ++I) {
-      const auto &T = *I;
+    __sanitizer::BlockingMutexLock G(&Mutex);
+    for (const auto &T : Buffers) {
       if (std::get<1>(T))
         Fn(std::get<0>(T));
     }
diff --git a/test/xray/TestCases/Linux/fdr-thread-order.cc b/test/xray/TestCases/Linux/fdr-thread-order.cc
index 8e8c421dc..6ac2114a5 100644
--- a/test/xray/TestCases/Linux/fdr-thread-order.cc
+++ b/test/xray/TestCases/Linux/fdr-thread-order.cc
@@ -1,63 +1,37 @@
 // RUN: %clangxx_xray -g -std=c++11 %s -o %t
 // RUN: rm fdr-thread-order.* || true
-// RUN: XRAY_OPTIONS="patch_premain=false xray_naive_log=false \
-// RUN:    xray_logfile_base=fdr-thread-order. xray_fdr_log=true verbosity=1 \
-// RUN:    xray_fdr_log_func_duration_threshold_us=0" %run %t 2>&1 | \
-// RUN:    FileCheck %s
-// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t \
-// RUN:    "`ls fdr-thread-order.* | head -1`"
-// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t \
-// RUN:    "`ls fdr-thread-order.* | head -1`" | \
-// RUN:    FileCheck %s --check-prefix TRACE
+// RUN: XRAY_OPTIONS="patch_premain=false xray_naive_log=false xray_logfile_base=fdr-thread-order. xray_fdr_log=true verbosity=1 xray_fdr_log_func_duration_threshold_us=0" %run %t 2>&1 | FileCheck %s
+// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t "`ls fdr-thread-order.* | head -1`" | FileCheck %s --check-prefix TRACE
 // RUN: rm fdr-thread-order.*
 // FIXME: Make llvm-xray work on non-x86_64 as well.
 // REQUIRES: x86_64-linux
 // REQUIRES: built-in-llvm-tree
 
 #include "xray/xray_log_interface.h"
-#include <atomic>
-#include <cassert>
 #include <thread>
+#include <cassert>
 
 constexpr auto kBufferSize = 16384;
 constexpr auto kBufferMax = 10;
 
-std::atomic<uint64_t> var{0};
-
-[[clang::xray_always_instrument]] void __attribute__((noinline)) f1() {
-  for (auto i = 0; i < 1 << 20; ++i)
-    ++var;
-}
-
-[[clang::xray_always_instrument]] void __attribute__((noinline)) f2() {
-  for (auto i = 0; i < 1 << 20; ++i)
-    ++var;
-}
+thread_local uint64_t var = 0;
+[[clang::xray_always_instrument]] void __attribute__((noinline)) f1() { ++var; }
+[[clang::xray_always_instrument]] void __attribute__((noinline)) f2() { ++var; }
 
 int main(int argc, char *argv[]) {
   using namespace __xray;
   FDRLoggingOptions Options;
-  __xray_patch();
   assert(__xray_log_init(kBufferSize, kBufferMax, &Options,
                          sizeof(FDRLoggingOptions)) ==
          XRayLogInitStatus::XRAY_LOG_INITIALIZED);
-
-  std::atomic_thread_fence(std::memory_order_acq_rel);
-
-  {
-    std::thread t1([] { f1(); });
-    std::thread t2([] { f2(); });
-    t1.join();
-    t2.join();
-  }
-
-  std::atomic_thread_fence(std::memory_order_acq_rel);
+  __xray_patch();
+  std::thread t1([] { f1(); });
+  std::thread t2([] { f2(); });
+  t1.join();
+  t2.join();
   __xray_log_finalize();
   __xray_log_flushLog();
-  __xray_unpatch();
-  return var > 0 ? 0 : 1;
   // CHECK: {{.*}}XRay: Log file in '{{.*}}'
-  // CHECK-NOT: Failed
 }
 
 // We want to make sure that the order of the function log doesn't matter.