[PATCH] D31384: [XRay] [compiler-rt] Write buffer length to FDR log before writing buffer.

[Keith via Phabricator via llvm-commits]

kpw created this revision.

Currently the FDR log writer, upon flushing, dumps a sequence of buffers from
its freelist to disk. A reader can read the first buffer up to an EOB record,
but then it is unclear how far ahead to scan to find the next threads traces.

There are a few ways to handle this problem.

1. The reader has externalized knowledge of the buffer size.
2. The size of buffers is in the file header or otherwise encoded in the log.
3. Only write out the portion of the buffer with records. When released, the buffers are marked with a size.
4. The reader looks for memory that matches a pattern and synchronizes on it.

2 and 3 seem the most flexible and 2 does not rule 3 out.

This is an implementation of 2.

In addition, the function handler for fdr more aggressively checks for
finalization and makes an attempt to release its buffer.


https://reviews.llvm.org/D31384

Files:
  lib/xray/xray_fdr_logging.cc
  lib/xray/xray_fdr_logging_impl.h


Index: lib/xray/xray_fdr_logging_impl.h
===================================================================
--- lib/xray/xray_fdr_logging_impl.h
+++ lib/xray/xray_fdr_logging_impl.h
@@ -308,9 +308,23 @@
     const std::atomic<XRayLogInitStatus> &LoggingStatus,
     const std::shared_ptr<BufferQueue> &BQ) XRAY_NEVER_INSTRUMENT {
   // Bail out right away if logging is not initialized yet.
-  if (LoggingStatus.load(std::memory_order_acquire) !=
-      XRayLogInitStatus::XRAY_LOG_INITIALIZED)
+  // We should take the opportunity to release the buffer though.
+  auto Status = LoggingStatus.load(std::memory_order_acquire);
+  if (Status != XRayLogInitStatus::XRAY_LOG_INITIALIZED) {
+    if (RecordPtr != nullptr &&
+        (Status == XRayLogInitStatus::XRAY_LOG_FINALIZING ||
+         Status == XRayLogInitStatus::XRAY_LOG_FINALIZED)) {
+      writeEOBMetadata();
+      auto EC = BQ->releaseBuffer(Buffer);
+      if (EC != BufferQueue::ErrorCode::Ok) {
+        Report("Failed to release buffer at %p; error=%s\n", Buffer.Buffer,
+               BufferQueue::getErrorString(EC));
+        return;
+      }
+      RecordPtr = nullptr;
+    }
     return;
+  }
 
   // We use a thread_local variable to keep track of which CPUs we've already
   // run, and the TSC times for these CPUs. This allows us to stop repeating the
Index: lib/xray/xray_fdr_logging.cc
===================================================================
--- lib/xray/xray_fdr_logging.cc
+++ lib/xray/xray_fdr_logging.cc
@@ -125,9 +125,16 @@
   clock_gettime(CLOCK_REALTIME, &Header.TS);
   retryingWriteAll(Fd, reinterpret_cast<char *>(&Header),
                    reinterpret_cast<char *>(&Header) + sizeof(Header));
+
   LocalBQ->apply([&](const BufferQueue::Buffer &B) {
-    retryingWriteAll(Fd, reinterpret_cast<char *>(B.Buffer),
-                     reinterpret_cast<char *>(B.Buffer) + B.Size);
+    uint64_t BufferSize = B.Size;
+    if (BufferSize > 0) {
+      retryingWriteAll(Fd, reinterpret_cast<char *>(&BufferSize),
+                       reinterpret_cast<char *>(&BufferSize) +
+                           sizeof(BufferSize));
+      retryingWriteAll(Fd, reinterpret_cast<char *>(B.Buffer),
+                       reinterpret_cast<char *>(B.Buffer) + B.Size);
+    }
   });
   LogFlushStatus.store(XRayLogFlushStatus::XRAY_LOG_FLUSHED,
                        std::memory_order_release);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31384.93103.patch
Type: text/x-patch
Size: 2389 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170327/144f712e/attachment.bin>