[compiler-rt] r345445 - [XRay] Support generational buffers in FDR controller

Dean Michael Berris via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 26 20:00:22 PDT 2018


Author: dberris
Date: Fri Oct 26 20:00:21 2018
New Revision: 345445

URL: http://llvm.org/viewvc/llvm-project?rev=345445&view=rev
Log:
[XRay] Support generational buffers in FDR controller

Summary:
This is an intermediary step in the full support for generational buffer
management in the FDR runtime. This change makes the FDR controller
aware of the new generation number in the buffers handed out by the
BufferQueue type.

In the process of making this change, we've realised that the cleanest
way of ensuring that the backing store per generation is live while all
the threads that need access to it will need reference counting to tie
the backing store to the lifetime of all threads that have a handle on
buffers associated with the memory.

We also learn that we're missing the edge-case in the function exit
handler's implementation where the first record being written into the
buffer is a function exit, which is caught/fixed by the test for
generational buffer management.

We still haven't wired the controller into the FDR mode runtime, which
will need the reference counting on the backing store implemented to
ensure that we're being conservatively thread-safe with this approach.

Depends on D52974.

Reviewers: mboerger, eizan

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D53551

Modified:
    compiler-rt/trunk/lib/xray/tests/unit/fdr_controller_test.cc
    compiler-rt/trunk/lib/xray/xray_fdr_controller.h

Modified: compiler-rt/trunk/lib/xray/tests/unit/fdr_controller_test.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/xray/tests/unit/fdr_controller_test.cc?rev=345445&r1=345444&r2=345445&view=diff
==============================================================================
--- compiler-rt/trunk/lib/xray/tests/unit/fdr_controller_test.cc (original)
+++ compiler-rt/trunk/lib/xray/tests/unit/fdr_controller_test.cc Fri Oct 26 20:00:21 2018
@@ -183,7 +183,7 @@ protected:
 public:
   void SetUp() override {
     bool Success;
-    BQ = llvm::make_unique<BufferQueue>(sizeof(MetadataRecord) * 4 +
+    BQ = llvm::make_unique<BufferQueue>(sizeof(MetadataRecord) * 5 +
                                             sizeof(FunctionRecord) * 2,
                                         kBuffers, Success);
     ASSERT_TRUE(Success);
@@ -198,12 +198,12 @@ constexpr size_t BufferManagementTest::k
 TEST_F(BufferManagementTest, HandlesOverflow) {
   uint64_t TSC = 1;
   uint16_t CPU = 1;
-  for (size_t I = 0; I < kBuffers; ++I) {
+  for (size_t I = 0; I < kBuffers + 1; ++I) {
     ASSERT_TRUE(C->functionEnter(1, TSC++, CPU));
     ASSERT_TRUE(C->functionExit(1, TSC++, CPU));
   }
-  C->flush();
-  ASSERT_EQ(BQ->finalize(), BufferQueue::ErrorCode::Ok);
+  ASSERT_TRUE(C->flush());
+  ASSERT_THAT(BQ->finalize(), Eq(BufferQueue::ErrorCode::Ok));
 
   std::string Serialized = serialize(*BQ, 3);
   llvm::DataExtractor DE(Serialized, true, 8);
@@ -238,5 +238,28 @@ TEST_F(BufferManagementTest, HandlesFina
                       FuncId(1), RecordType(llvm::xray::RecordTypes::ENTER)))));
 }
 
+TEST_F(BufferManagementTest, HandlesGenerationalBufferQueue) {
+  uint64_t TSC = 1;
+  uint16_t CPU = 1;
+
+  ASSERT_TRUE(C->functionEnter(1, TSC++, CPU));
+  ASSERT_THAT(BQ->finalize(), Eq(BufferQueue::ErrorCode::Ok));
+  ASSERT_THAT(BQ->init(sizeof(MetadataRecord) * 4 + sizeof(FunctionRecord) * 2,
+                       kBuffers),
+              Eq(BufferQueue::ErrorCode::Ok));
+  EXPECT_TRUE(C->functionExit(1, TSC++, CPU));
+  ASSERT_TRUE(C->flush());
+
+  // We expect that we will only be able to find the function exit event, but
+  // not the function enter event, since we only have information about the new
+  // generation of the buffers.
+  std::string Serialized = serialize(*BQ, 3);
+  llvm::DataExtractor DE(Serialized, true, 8);
+  auto TraceOrErr = llvm::xray::loadTrace(DE);
+  EXPECT_THAT_EXPECTED(
+      TraceOrErr, HasValue(ElementsAre(AllOf(
+                      FuncId(1), RecordType(llvm::xray::RecordTypes::EXIT)))));
+}
+
 } // namespace
 } // namespace __xray

Modified: compiler-rt/trunk/lib/xray/xray_fdr_controller.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/xray/xray_fdr_controller.h?rev=345445&r1=345444&r2=345445&view=diff
==============================================================================
--- compiler-rt/trunk/lib/xray/xray_fdr_controller.h (original)
+++ compiler-rt/trunk/lib/xray/xray_fdr_controller.h Fri Oct 26 20:00:21 2018
@@ -42,7 +42,7 @@ template <size_t Version = 3> class FDRC
   bool finalized() const { return BQ == nullptr || BQ->finalizing(); }
 
   bool hasSpace(size_t S) {
-    return B.Data != nullptr &&
+    return B.Data != nullptr && B.Generation == BQ->generation() &&
            W.getNextRecord() + S <= reinterpret_cast<char *>(B.Data) + B.Size;
   }
 
@@ -51,8 +51,6 @@ template <size_t Version = 3> class FDRC
   }
 
   bool getNewBuffer() {
-    if (!returnBuffer())
-      return false;
     if (BQ->getBuffer(B) != BufferQueue::ErrorCode::Ok)
       return false;
 
@@ -60,6 +58,9 @@ template <size_t Version = 3> class FDRC
     DCHECK_EQ(W.getNextRecord(), B.Data);
     LatestTSC = 0;
     LatestCPU = 0;
+    First = true;
+    UndoableFunctionEnters = 0;
+    UndoableTailExits = 0;
     atomic_store(&B.Extents, 0, memory_order_release);
     return true;
   }
@@ -108,6 +109,8 @@ template <size_t Version = 3> class FDRC
       return returnBuffer();
 
     if (UNLIKELY(!hasSpace(S))) {
+      if (!returnBuffer())
+        return false;
       if (!getNewBuffer())
         return false;
       if (!setupNewBuffer())
@@ -128,24 +131,26 @@ template <size_t Version = 3> class FDRC
     if (BQ == nullptr)
       return false;
 
+    First = true;
     if (finalized()) {
       BQ->releaseBuffer(B); // ignore result.
       return false;
     }
 
-    First = true;
-    if (BQ->releaseBuffer(B) != BufferQueue::ErrorCode::Ok)
-      return false;
-    return true;
+    return BQ->releaseBuffer(B) == BufferQueue::ErrorCode::Ok;
   }
 
-  enum class PreambleResult { NoChange, WroteMetadata };
+  enum class PreambleResult { NoChange, WroteMetadata, InvalidBuffer };
   PreambleResult functionPreamble(uint64_t TSC, uint16_t CPU) {
     if (UNLIKELY(LatestCPU != CPU || LatestTSC == 0)) {
       // We update our internal tracking state for the Latest TSC and CPU we've
       // seen, then write out the appropriate metadata and function records.
       LatestTSC = TSC;
       LatestCPU = CPU;
+
+      if (B.Generation != BQ->generation())
+        return PreambleResult::InvalidBuffer;
+
       W.writeMetadata<MetadataRecord::RecordKinds::NewCPUId>(CPU, TSC);
       return PreambleResult::WroteMetadata;
     }
@@ -153,6 +158,10 @@ template <size_t Version = 3> class FDRC
     if (UNLIKELY(LatestCPU == LatestCPU && LatestTSC > TSC)) {
       // The TSC has wrapped around, from the last TSC we've seen.
       LatestTSC = TSC;
+
+      if (B.Generation != BQ->generation())
+        return PreambleResult::InvalidBuffer;
+
       W.writeMetadata<MetadataRecord::RecordKinds::TSCWrap>(TSC);
       return PreambleResult::WroteMetadata;
     }
@@ -160,7 +169,7 @@ template <size_t Version = 3> class FDRC
     return PreambleResult::NoChange;
   }
 
-  void rewindRecords(int32_t FuncId, uint64_t TSC, uint16_t CPU) {
+  bool rewindRecords(int32_t FuncId, uint64_t TSC, uint16_t CPU) {
     // Undo one enter record, because at this point we are either at the state
     // of:
     // - We are exiting a function that we recently entered.
@@ -169,6 +178,8 @@ template <size_t Version = 3> class FDRC
     //
     FunctionRecord F;
     W.undoWrites(sizeof(FunctionRecord));
+    if (B.Generation != BQ->generation())
+      return false;
     internal_memcpy(&F, W.getNextRecord(), sizeof(FunctionRecord));
 
     DCHECK(F.RecordKind ==
@@ -179,30 +190,35 @@ template <size_t Version = 3> class FDRC
     LatestTSC -= F.TSCDelta;
     if (--UndoableFunctionEnters != 0) {
       LastFunctionEntryTSC -= F.TSCDelta;
-      return;
+      return true;
     }
 
     LastFunctionEntryTSC = 0;
     auto RewindingTSC = LatestTSC;
     auto RewindingRecordPtr = W.getNextRecord() - sizeof(FunctionRecord);
     while (UndoableTailExits) {
+      if (B.Generation != BQ->generation())
+        return false;
       internal_memcpy(&F, RewindingRecordPtr, sizeof(FunctionRecord));
       DCHECK_EQ(F.RecordKind,
                 uint8_t(FunctionRecord::RecordKinds::FunctionTailExit));
       RewindingTSC -= F.TSCDelta;
       RewindingRecordPtr -= sizeof(FunctionRecord);
+      if (B.Generation != BQ->generation())
+        return false;
       internal_memcpy(&F, RewindingRecordPtr, sizeof(FunctionRecord));
 
       // This tail call exceeded the threshold duration. It will not be erased.
       if ((TSC - RewindingTSC) >= CycleThreshold) {
         UndoableTailExits = 0;
-        return;
+        return true;
       }
 
       --UndoableTailExits;
       W.undoWrites(sizeof(FunctionRecord) * 2);
       LatestTSC = RewindingTSC;
     }
+    return true;
   }
 
 public:
@@ -212,18 +228,17 @@ public:
       : BQ(BQ), B(B), W(W), WallClockReader(R), CycleThreshold(C) {}
 
   bool functionEnter(int32_t FuncId, uint64_t TSC, uint16_t CPU) {
-    if (finalized())
+    if (finalized() ||
+        !prepareBuffer(sizeof(MetadataRecord) + sizeof(FunctionRecord)))
       return returnBuffer();
 
-    if (!prepareBuffer(sizeof(MetadataRecord) + sizeof(FunctionRecord)))
+    auto PreambleStatus = functionPreamble(TSC, CPU);
+    if (PreambleStatus == PreambleResult::InvalidBuffer)
       return returnBuffer();
 
-    if (functionPreamble(TSC, CPU) == PreambleResult::WroteMetadata) {
-      UndoableFunctionEnters = 1;
-    } else {
-      ++UndoableFunctionEnters;
-    }
-
+    UndoableFunctionEnters = (PreambleStatus == PreambleResult::WroteMetadata)
+                                 ? 1
+                                 : UndoableFunctionEnters + 1;
     LastFunctionEntryTSC = TSC;
     LatestTSC = TSC;
     return W.writeFunction(FDRLogWriter::FunctionRecordKind::Enter,
@@ -237,12 +252,14 @@ public:
     if (!prepareBuffer(sizeof(MetadataRecord) + sizeof(FunctionRecord)))
       return returnBuffer();
 
-    if (functionPreamble(TSC, CPU) == PreambleResult::NoChange &&
+    auto PreambleStatus = functionPreamble(TSC, CPU);
+    if (PreambleStatus == PreambleResult::InvalidBuffer)
+      return returnBuffer();
+
+    if (PreambleStatus == PreambleResult::NoChange &&
         UndoableFunctionEnters != 0 &&
-        TSC - LastFunctionEntryTSC < CycleThreshold) {
-      rewindRecords(FuncId, TSC, CPU);
-      return true;
-    }
+        TSC - LastFunctionEntryTSC < CycleThreshold)
+      return rewindRecords(FuncId, TSC, CPU);
 
     UndoableTailExits = UndoableFunctionEnters ? UndoableTailExits + 1 : 0;
     UndoableFunctionEnters = 0;
@@ -253,15 +270,11 @@ public:
 
   bool functionEnterArg(int32_t FuncId, uint64_t TSC, uint16_t CPU,
                         uint64_t Arg) {
-    if (finalized())
+    if (finalized() ||
+        !prepareBuffer((2 * sizeof(MetadataRecord)) + sizeof(FunctionRecord)) ||
+        functionPreamble(TSC, CPU) == PreambleResult::InvalidBuffer)
       return returnBuffer();
 
-    if (!prepareBuffer((2 * sizeof(MetadataRecord)) + sizeof(FunctionRecord)))
-      return returnBuffer();
-
-    // Ignore the result of writing out the preamble.
-    functionPreamble(TSC, CPU);
-
     LatestTSC = TSC;
     LastFunctionEntryTSC = 0;
     UndoableFunctionEnters = 0;
@@ -273,15 +286,18 @@ public:
   }
 
   bool functionExit(int32_t FuncId, uint64_t TSC, uint16_t CPU) {
-    if (finalized())
+    if (finalized() ||
+        !prepareBuffer(sizeof(MetadataRecord) + sizeof(FunctionRecord)))
       return returnBuffer();
 
-    if (functionPreamble(TSC, CPU) == PreambleResult::NoChange &&
+    auto PreambleStatus = functionPreamble(TSC, CPU);
+    if (PreambleStatus == PreambleResult::InvalidBuffer)
+      return returnBuffer();
+
+    if (PreambleStatus == PreambleResult::NoChange &&
         UndoableFunctionEnters != 0 &&
-        TSC - LastFunctionEntryTSC < CycleThreshold) {
-      rewindRecords(FuncId, TSC, CPU);
-      return true;
-    }
+        TSC - LastFunctionEntryTSC < CycleThreshold)
+      return rewindRecords(FuncId, TSC, CPU);
 
     LatestTSC = TSC;
     UndoableFunctionEnters = 0;




More information about the llvm-commits mailing list