[XRay] FDR Record Producer/Consumer Implementation

[dberris]

Summary

This patch defines two new base types called RecordProducer and
RecordConsumer which have default implementations for convenience
(particularly for testing).

A RecordProducer implementation has one member function called
produce() which serves as a factory constructor for Record
instances. This code exercises the RecordInitializer code path in the
implementation for FileBasedRecordProducer.

A RecordConsumer has a single member function called consume(...)
which, as the name implies, consumes instances of
std::unique_ptr<Record>. We have two implementations, one of which is
used in the test to generate a vector of std::unique_ptr<Record>
similar to how the LogBuilder implementation works.

We introduce a test in FDRProducerConsumerTest which ensures that
records we write through the FDRTraceWriter can be loaded by the
FileBasedRecordProducer. The record(s) loaded this way are written
again through the FDRTraceWriter into a separate string, which we then
compare. This ensures that the read-in bytes to create the Record
instances in memory can be replicated when written out through the
FDRTraceWriter.

This change depends on D51210 and is part of the refactoring of D50441
into smaller, more focused changes.

[dberris]

Rebase.

[eizan]

llvm/lib/XRay/FDRRecordProducer.cpp

31 ↗

(On Diff #163464)

nit: Instead of doing the bitwise ops directly here and in the switch statement argument + elsewhere, why not declare something like bool is_metadata(uint8_t b) and enum MetadataType metadata_type(uint8t_t d). That way you don't need somebody to refer to your comment at the top of this function, and if you use enum values you don't need to annotate each 'case N:' line.

[dberris]

Address comment by @eizan.

[dberris]

llvm/lib/XRay/FDRRecordProducer.cpp

31 ↗

(On Diff #163464)

Good point -- I've refactored this a bit to make it easier to see the mapping between the loaded type and the type of the record to create.

[RKSimon]

llvm/trunk/lib/XRay/FDRRecordProducer.cpp

50

@dberris Why are you mapping an integer to an enum with the same value?!? Replace the uint8_t T arg with an MetadataRecordKinds enum arg, or just get rid of the enum (AFAICT its only used in this function) and use the uint8_t directly.

[dberris]

llvm/trunk/lib/XRay/FDRRecordProducer.cpp

50

So, there are two reasons:

• The data we're getting is a byte, and we need to decide whether the byte maps to the number we're expecting. The caller of the function (below) only has access to the byte.

• I could just static cast the byte into a MetadataRecordKinds value, but that's undefined behaviour (not all the values of T will be valid values for the enum).

If I get rid of the enum, I have to keep the enum names and values as comments (previous version of this patch).

Ultimately the enum is a convenience. This mapping though is a little suspect.

This is where being able to just share the enum definition between compiler-rt and llvm would make things a lot easier. :(

[dberris]

llvm/trunk/lib/XRay/FDRRecordProducer.cpp

50

Actually, I think you're right -- there's a better way to do this. Fixed in r341205.

llvm/trunk/include/llvm/XRay/FDRRecordConsumer.h

+//===- FDRRecordConsumer.h - XRay Flight Data Recorder Mode Records -------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_INCLUDE_LLVM_XRAY_FDRRECORDCONSUMER_H_
+#define LLVM_INCLUDE_LLVM_XRAY_FDRRECORDCONSUMER_H_
+
+#include "llvm/Support/Error.h"
+#include "llvm/XRay/FDRRecords.h"
+#include <algorithm>
+#include <memory>
+#include <vector>
+
+namespace llvm {
+namespace xray {
+
+class RecordConsumer {
+public:
+  virtual Error consume(std::unique_ptr<Record> R) = 0;
+  virtual ~RecordConsumer() = default;
+};
+
+// This consumer will collect all the records into a vector of records, in
+// arrival order.
+class LogBuilderConsumer : public RecordConsumer {
+  std::vector<std::unique_ptr<Record>> &Records;
+
+public:
+  explicit LogBuilderConsumer(std::vector<std::unique_ptr<Record>> &R)
+      : RecordConsumer(), Records(R) {}
+
+  Error consume(std::unique_ptr<Record> R) override;
+};
+
+// A PipelineConsumer applies a set of visitors to every consumed Record, in the
+// order by which the visitors are added to the pipeline in the order of
+// appearance.
+class PipelineConsumer : public RecordConsumer {
+  std::vector<RecordVisitor *> Visitors;
+
+public:
+  PipelineConsumer(std::initializer_list<RecordVisitor *> V)
+      : RecordConsumer(), Visitors(V) {}
+
+  Error consume(std::unique_ptr<Record> R) override;
+};
+
+} // namespace xray
+} // namespace llvm
+
+#endif // LLVM_INCLUDE_LLVM_XRAY_FDRRECORDCONSUMER_H_

llvm/trunk/include/llvm/XRay/FDRRecordProducer.h

+//===- FDRRecordProducer.h - XRay FDR Mode Record Producer ----------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_INCLUDE_LLVM_XRAY_FDRRECORDPRODUCER_H_
+#define LLVM_INCLUDE_LLVM_XRAY_FDRRECORDPRODUCER_H_
+
+#include "llvm/Support/Error.h"
+#include "llvm/XRay/FDRRecords.h"
+#include "llvm/XRay/XRayRecord.h"
+#include <memory>
+
+namespace llvm {
+namespace xray {
+
+class RecordProducer {
+public:
+  /// All producer implementations must yield either an Error or a non-nullptr
+  /// unique_ptr<Record>.
+  virtual Expected<std::unique_ptr<Record>> produce() = 0;
+  virtual ~RecordProducer() = default;
+};
+
+class FileBasedRecordProducer : public RecordProducer {
+  const XRayFileHeader &Header;
+  DataExtractor &E;
+  uint32_t &OffsetPtr;
+
+public:
+  FileBasedRecordProducer(const XRayFileHeader &FH, DataExtractor &DE,
+                          uint32_t &OP)
+      : Header(FH), E(DE), OffsetPtr(OP) {}
+
+  /// This producer encapsulates the logic for loading a File-backed
+  /// RecordProducer hidden behind a DataExtractor.
+  Expected<std::unique_ptr<Record>> produce() override;
+};
+
+} // namespace xray
+} // namespace llvm
+
+#endif // LLVM_INCLUDE_LLVM_XRAY_FDRRECORDPRODUCER_H_

llvm/trunk/lib/XRay/CMakeLists.txt

 add_llvm_library(LLVMXRay
+  FDRRecordProducer.cpp
   FDRRecords.cpp
   FDRTraceWriter.cpp
   FileHeaderReader.cpp
   InstrumentationMap.cpp
+  LogBuilderConsumer.cpp
   Profile.cpp
   RecordInitializer.cpp
   Trace.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/XRay
 
   DEPENDS
   LLVMSupport
   LLVMObject
 
   LINK_LIBS
   LLVMSupport
   LLVMObject
   )

llvm/trunk/lib/XRay/FDRRecordProducer.cpp

+//===- FDRRecordProducer.cpp - XRay FDR Mode Record Producer --------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include "llvm/XRay/FDRRecordProducer.h"
+#include "llvm/Support/DataExtractor.h"
+
+namespace llvm {
+namespace xray {
+
+namespace {
+
+// Keep this in sync with the values written in the XRay FDR mode runtime in
+// compiler-rt.
+enum class MetadataRecordKinds : uint8_t {
+  NewBuffer,
+  EndOfBuffer,
+  NewCPUId,
+  TSCWrap,
+  WalltimeMarker,
+  CustomEventMarker,
+  CallArgument,
+  BufferExtents,
+  TypedEventMarker,
+  Pid,
+  // This is an end marker, used to identify the upper bound for this enum.
+  EnumEndMarker,
+};
+
+Expected<std::unique_ptr<Record>>
+metadataRecordType(const XRayFileHeader &Header, uint8_t T) {
+  if (T >= static_cast<uint8_t>(MetadataRecordKinds::EnumEndMarker))
+    return createStringError(std::make_error_code(std::errc::invalid_argument),
+                             "Invalid metadata record type: %d", T);
+  static constexpr MetadataRecordKinds Mapping[] = {
+      MetadataRecordKinds::NewBuffer,
+      MetadataRecordKinds::EndOfBuffer,
+      MetadataRecordKinds::NewCPUId,
+      MetadataRecordKinds::TSCWrap,
+      MetadataRecordKinds::WalltimeMarker,
+      MetadataRecordKinds::CustomEventMarker,
+      MetadataRecordKinds::CallArgument,
+      MetadataRecordKinds::BufferExtents,
+      MetadataRecordKinds::TypedEventMarker,
+      MetadataRecordKinds::Pid,
+  };
+  switch (Mapping[T]) {
+  case MetadataRecordKinds::NewBuffer:
+    return make_unique<NewBufferRecord>();
+  case MetadataRecordKinds::EndOfBuffer:
+    if (Header.Version >= 2)
+      return createStringError(
+          std::make_error_code(std::errc::executable_format_error),
+          "End of buffer records are no longer supported starting version "
+          "2 of the log.");
+    return make_unique<EndBufferRecord>();
+  case MetadataRecordKinds::NewCPUId:
+    return make_unique<NewCPUIDRecord>();
+  case MetadataRecordKinds::TSCWrap:
+    return make_unique<TSCWrapRecord>();
+  case MetadataRecordKinds::WalltimeMarker:
+    return make_unique<WallclockRecord>();
+  case MetadataRecordKinds::CustomEventMarker:
+    return make_unique<CustomEventRecord>();
+  case MetadataRecordKinds::CallArgument:
+    return make_unique<CallArgRecord>();
+  case MetadataRecordKinds::BufferExtents:
+    return make_unique<BufferExtents>();
+  case MetadataRecordKinds::TypedEventMarker:
+    return createStringError(std::make_error_code(std::errc::invalid_argument),
+                             "Encountered an unsupported TypedEventMarker.");
+  case MetadataRecordKinds::Pid:
+    return make_unique<PIDRecord>();
+  case MetadataRecordKinds::EnumEndMarker:
+    llvm_unreachable("Invalid MetadataRecordKind");
+  }
+}
+
+} // namespace
+
+Expected<std::unique_ptr<Record>> FileBasedRecordProducer::produce() {
+  // At the top level, we read one byte to determine the type of the record to
+  // create. This byte will comprise of the following bits:
+  //
+  //   - offset 0: A '1' indicates a metadata record, a '0' indicates a function
+  //     record.
+  //   - offsets 1-7: For metadata records, this will indicate the kind of
+  //     metadata record should be loaded.
+  //
+  // We read first byte, then create the appropriate type of record to consume
+  // the rest of the bytes.
+  auto PreReadOffset = OffsetPtr;
+  uint8_t FirstByte = E.getU8(&OffsetPtr);
+  std::unique_ptr<Record> R;
+
+  // For metadata records, handle especially here.
+  if (FirstByte & 0x01) {
+    auto LoadedType = FirstByte >> 1;
+    auto MetadataRecordOrErr = metadataRecordType(Header, LoadedType);
+    if (!MetadataRecordOrErr)
+      return joinErrors(
+          MetadataRecordOrErr.takeError(),
+          createStringError(
+              std::make_error_code(std::errc::executable_format_error),
+              "Encountered an unsupported metadata record (%d) at offset %d.",
+              LoadedType, PreReadOffset));
+    R = std::move(MetadataRecordOrErr.get());
+  } else {
+    R = llvm::make_unique<FunctionRecord>();
+  }
+  RecordInitializer RI(E, OffsetPtr);
+
+  if (auto Err = R->apply(RI))
+    return std::move(Err);
+
+  assert(R != nullptr);
+  return std::move(R);
+}
+
+} // namespace xray
+} // namespace llvm

llvm/trunk/lib/XRay/LogBuilderConsumer.cpp

+//===- FDRRecordConsumer.h - XRay Flight Data Recorder Mode Records -------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include "llvm/XRay/FDRRecordConsumer.h"
+
+namespace llvm {
+namespace xray {
+
+Error LogBuilderConsumer::consume(std::unique_ptr<Record> R) {
+  if (!R)
+    return createStringError(
+        std::make_error_code(std::errc::invalid_argument),
+        "Must not call RecordConsumer::consume() with a null pointer.");
+  Records.push_back(std::move(R));
+  return Error::success();
+}
+
+Error PipelineConsumer::consume(std::unique_ptr<Record> R) {
+  if (!R)
+    return createStringError(
+        std::make_error_code(std::errc::invalid_argument),
+        "Must not call RecordConsumer::consume() with a null pointer.");
+
+  // We apply all of the visitors in order, and concatenate errors
+  // appropriately.
+  Error Result = Error::success();
+  for (auto *V : Visitors)
+    Result = joinErrors(std::move(Result), R->apply(*V));
+  return Result;
+}
+
+} // namespace xray
+} // namespace llvm

llvm/trunk/unittests/XRay/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   XRay
   )
 
 add_llvm_unittest(XRayTests
+  FDRProducerConsumerTest.cpp
   FDRTraceWriterTest.cpp
   GraphTest.cpp
   ProfileTest.cpp
   )
 
 add_dependencies(XRayTests intrinsics_gen)

llvm/trunk/unittests/XRay/FDRProducerConsumerTest.cpp

+//===- llvm/unittest/XRay/FDRProducerConsumerTest.cpp -----------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Test for round-trip record writing and reading.
+//
+//===----------------------------------------------------------------------===//
+#include "llvm/Support/DataExtractor.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/XRay/FDRLogBuilder.h"
+#include "llvm/XRay/FDRRecordConsumer.h"
+#include "llvm/XRay/FDRRecordProducer.h"
+#include "llvm/XRay/FDRRecords.h"
+#include "llvm/XRay/FDRTraceWriter.h"
+#include "llvm/XRay/FileHeaderReader.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <string>
+#include <tuple>
+
+namespace llvm {
+namespace xray {
+namespace {
+
+using ::testing::Eq;
+using ::testing::IsEmpty;
+using ::testing::Not;
+
+template <class RecordType> std::unique_ptr<Record> MakeRecord();
+
+template <> std::unique_ptr<Record> MakeRecord<BufferExtents>() {
+  return make_unique<BufferExtents>(1);
+}
+
+template <> std::unique_ptr<Record> MakeRecord<NewBufferRecord>() {
+  return make_unique<NewBufferRecord>(1);
+}
+
+template <> std::unique_ptr<Record> MakeRecord<NewCPUIDRecord>() {
+  return make_unique<NewCPUIDRecord>(1);
+}
+
+template <> std::unique_ptr<Record> MakeRecord<TSCWrapRecord>() {
+  return make_unique<TSCWrapRecord>(1);
+}
+
+template <> std::unique_ptr<Record> MakeRecord<WallclockRecord>() {
+  return make_unique<WallclockRecord>(1, 2);
+}
+
+template <> std::unique_ptr<Record> MakeRecord<CustomEventRecord>() {
+  return make_unique<CustomEventRecord>(4, 1, "data");
+}
+
+template <> std::unique_ptr<Record> MakeRecord<CallArgRecord>() {
+  return make_unique<CallArgRecord>(1);
+}
+
+template <> std::unique_ptr<Record> MakeRecord<PIDRecord>() {
+  return make_unique<PIDRecord>(1);
+}
+
+template <> std::unique_ptr<Record> MakeRecord<FunctionRecord>() {
+  return make_unique<FunctionRecord>(RecordTypes::ENTER, 1, 2);
+}
+
+template <class T> class RoundTripTest : public ::testing::Test {
+public:
+  RoundTripTest() : Data(), OS(Data) {
+    H.Version = 3;
+    H.Type = 1;
+    H.ConstantTSC = true;
+    H.NonstopTSC = true;
+    H.CycleFrequency = 3e9;
+
+    Writer = make_unique<FDRTraceWriter>(OS, H);
+    Record = MakeRecord<T>();
+  }
+
+protected:
+  std::string Data;
+  raw_string_ostream OS;
+  XRayFileHeader H;
+  std::unique_ptr<FDRTraceWriter> Writer;
+  std::unique_ptr<Record> Record;
+};
+
+TYPED_TEST_CASE_P(RoundTripTest);
+
+// This test ensures that the writing and reading implementations are in sync --
+// that given write(read(write(R))) == R.
+TYPED_TEST_P(RoundTripTest, RoundTripsSingleValue) {
+  auto &R = this->Record;
+  ASSERT_FALSE(errorToBool(R->apply(*this->Writer)));
+  this->OS.flush();
+
+  DataExtractor DE(this->Data, true, 8);
+  uint32_t OffsetPtr = 0;
+  auto HeaderOrErr = readBinaryFormatHeader(DE, OffsetPtr);
+  if (!HeaderOrErr)
+    FAIL() << HeaderOrErr.takeError();
+
+  FileBasedRecordProducer P(HeaderOrErr.get(), DE, OffsetPtr);
+  std::vector<std::unique_ptr<Record>> Records;
+  LogBuilderConsumer C(Records);
+  while (DE.isValidOffsetForDataOfSize(OffsetPtr, 1)) {
+    auto R = P.produce();
+    if (!R)
+      FAIL() << R.takeError();
+    if (auto E = C.consume(std::move(R.get())))
+      FAIL() << E;
+  }
+  ASSERT_THAT(Records, Not(IsEmpty()));
+  std::string Data2;
+  raw_string_ostream OS2(Data2);
+  FDRTraceWriter Writer2(OS2, this->H);
+  for (auto &P : Records)
+    ASSERT_FALSE(errorToBool(P->apply(Writer2)));
+  OS2.flush();
+
+  EXPECT_EQ(Data2.substr(sizeof(XRayFileHeader)),
+            this->Data.substr(sizeof(XRayFileHeader)));
+  EXPECT_THAT(Records[0]->type(), Eq(R->type()));
+}
+
+REGISTER_TYPED_TEST_CASE_P(RoundTripTest, RoundTripsSingleValue);
+
+using RecordTypes =
+    ::testing::Types<BufferExtents, NewBufferRecord, NewCPUIDRecord,
+                     TSCWrapRecord, WallclockRecord, CustomEventRecord,
+                     CallArgRecord, BufferExtents, PIDRecord, FunctionRecord>;
+INSTANTIATE_TYPED_TEST_CASE_P(Records, RoundTripTest, RecordTypes);
+
+} // namespace
+} // namespace xray
+} // namespace llvm