[InstrProf][Temporal] Add weight field to traces
authorEllis Hoag <ellis.sparky.hoag@gmail.com>
Wed, 12 Apr 2023 17:21:38 +0000 (10:21 -0700)
committerEllis Hoag <ellis.sparky.hoag@gmail.com>
Thu, 13 Apr 2023 17:37:05 +0000 (10:37 -0700)
As discussed in [0], add a `weight` field to temporal profiling traces found in profiles. This allows users to use the `--weighted-input=` flag in the `llvm-profdata merge` command to weight traces from different scenarios differently.

Note that this is a breaking change, but since [1] landed very recently and there is no way to "use" this trace data, there should be no users of this feature. We believe it is acceptable to land this change without bumping the profile format version.

[0] https://reviews.llvm.org/D147812#4259507
[1] https://reviews.llvm.org/D147287

Reviewed By: snehasish

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

12 files changed:
compiler-rt/test/profile/instrprof-timestamp.c
llvm/docs/CommandGuide/llvm-profdata.rst
llvm/include/llvm/ProfileData/InstrProf.h
llvm/include/llvm/ProfileData/InstrProfReader.h
llvm/include/llvm/ProfileData/InstrProfWriter.h
llvm/lib/ProfileData/InstrProfReader.cpp
llvm/lib/ProfileData/InstrProfWriter.cpp
llvm/test/tools/llvm-profdata/merge-traces.proftext
llvm/test/tools/llvm-profdata/read-traces.proftext
llvm/test/tools/llvm-profdata/trace-limit.proftext
llvm/tools/llvm-profdata/llvm-profdata.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp

index 50838cc..ad14e66 100644 (file)
@@ -2,14 +2,14 @@
 // RUN: %clang_pgogen -o %t -mllvm -pgo-temporal-instrumentation %s
 // RUN: env LLVM_PROFILE_FILE=%t.0.profraw %run %t n
 // RUN: env LLVM_PROFILE_FILE=%t.1.profraw %run %t y
-// RUN: llvm-profdata merge -o %t.profdata %t.0.profraw %t.1.profraw
+// RUN: llvm-profdata merge -o %t.profdata %t.0.profraw --weighted-input=5,%t.1.profraw
 // RUN: llvm-profdata show --temporal-profile-traces %t.profdata | FileCheck %s --implicit-check-not=unused
 
 // RUN: rm -f %t.profdata
 // RUN: %clang_pgogen -o %t -mllvm -pgo-temporal-instrumentation -mllvm -pgo-block-coverage %s
 // RUN: env LLVM_PROFILE_FILE=%t.0.profraw %run %t n
 // RUN: env LLVM_PROFILE_FILE=%t.1.profraw %run %t y
-// RUN: llvm-profdata merge -o %t.profdata %t.0.profraw %t.1.profraw
+// RUN: llvm-profdata merge -o %t.profdata %t.0.profraw --weighted-input=5,%t.1.profraw
 // RUN: llvm-profdata show --temporal-profile-traces %t.profdata | FileCheck %s --implicit-check-not=unused
 
 extern void exit(int);
@@ -36,12 +36,12 @@ int main(int argc, const char *argv[]) {
 }
 
 // CHECK: Temporal Profile Traces (samples=2 seen=2):
-// CHECK:   Temporal Profile Trace 0 (count=4):
+// CHECK:   Temporal Profile Trace 0 (weight=1 count=4):
 // CHECK:     main
 // CHECK:     a
 // CHECK:     b
 // CHECK:     c
-// CHECK:   Temporal Profile Trace 1 (count=3):
+// CHECK:   Temporal Profile Trace 1 (weight=5 count=3):
 // CHECK:     a
 // CHECK:     c
 // CHECK:     b
index 515c18b..c0a551f 100644 (file)
@@ -197,6 +197,18 @@ OPTIONS
  When ``-debug-info-correlate`` was used for instrumentation, use this option
  to correlate the raw profile.
 
+.. option:: --temporal-profile-trace-reservoir-size
+
+ The maximum number of temporal profile traces to be stored in the output
+ profile. If more traces are added, we will use reservoir sampling to select
+ which traces to keep. Note that changing this value between different merge
+ invocations on the same indexed profile could result in sample bias. The
+ default value is 100.
+
+.. option:: --temporal-profile-max-trace-length
+
+ The maximum number of functions in a single temporal profile trace. Longer
+ traces will be truncated. The default value is 1000.
 
 EXAMPLES
 ^^^^^^^^
index 3a8d254..2a9da23 100644 (file)
@@ -335,7 +335,10 @@ enum class instrprof_error {
 
 /// An ordered list of functions identified by their NameRef found in
 /// INSTR_PROF_DATA
-using TemporalProfTraceTy = std::vector<uint64_t>;
+struct TemporalProfTraceTy {
+  uint64_t Weight = 1;
+  std::vector<uint64_t> FunctionNameRefs;
+};
 
 inline std::error_code make_error_code(instrprof_error E) {
   return std::error_code(static_cast<int>(E), instrprof_category());
index 483f1d3..80c5284 100644 (file)
@@ -208,8 +208,13 @@ public:
   create(std::unique_ptr<MemoryBuffer> Buffer,
          const InstrProfCorrelator *Correlator = nullptr);
 
+  /// \param Weight for raw profiles use this as the temporal profile trace
+  ///               weight
   /// \returns a list of temporal profile traces.
-  virtual const SmallVector<TemporalProfTraceTy> &getTemporalProfTraces() {
+  virtual SmallVector<TemporalProfTraceTy> &
+  getTemporalProfTraces(std::optional<uint64_t> Weight = {}) {
+    // For non-raw profiles we ignore the input weight and instead use the
+    // weights already in the traces.
     return TemporalProfTraces;
   }
   /// \returns the total number of temporal profile traces seen.
@@ -395,7 +400,8 @@ public:
     return *Symtab.get();
   }
 
-  const SmallVector<TemporalProfTraceTy> &getTemporalProfTraces() override;
+  SmallVector<TemporalProfTraceTy> &
+  getTemporalProfTraces(std::optional<uint64_t> Weight = {}) override;
 
 private:
   Error createSymtab(InstrProfSymtab &Symtab);
index 254e66a..2be8c78 100644 (file)
@@ -88,7 +88,7 @@ public:
 
   /// Add \p SrcTraces using reservoir sampling where \p SrcStreamSize is the
   /// total number of temporal profiling traces the source has seen.
-  void addTemporalProfileTraces(SmallVector<TemporalProfTraceTy> SrcTraces,
+  void addTemporalProfileTraces(SmallVectorImpl<TemporalProfTraceTy> &SrcTraces,
                                 uint64_t SrcStreamSize);
 
   /// Add a memprof record for a function identified by its \p Id.
index 18f70a3..47c0a57 100644 (file)
@@ -281,7 +281,8 @@ Error TextInstrProfReader::readHeader() {
 /// Temporal profile trace data is stored in the header immediately after
 /// ":temporal_prof_traces". The first integer is the number of traces, the
 /// second integer is the stream size, then the following lines are the actual
-/// traces which consist of a comma separated list of function names.
+/// traces which consist of a weight and a comma separated list of function
+/// names.
 Error TextInstrProfReader::readTemporalProfTraceData() {
   if ((++Line).is_at_end())
     return error(instrprof_error::eof);
@@ -301,10 +302,17 @@ Error TextInstrProfReader::readTemporalProfTraceData() {
       return error(instrprof_error::eof);
 
     TemporalProfTraceTy Trace;
+    if (Line->getAsInteger(0, Trace.Weight))
+      return error(instrprof_error::malformed);
+
+    if ((++Line).is_at_end())
+      return error(instrprof_error::eof);
+
     SmallVector<StringRef> FuncNames;
     Line->split(FuncNames, ",", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
     for (auto &FuncName : FuncNames)
-      Trace.push_back(IndexedInstrProf::ComputeHash(FuncName.trim()));
+      Trace.FunctionNameRefs.push_back(
+          IndexedInstrProf::ComputeHash(FuncName.trim()));
     TemporalProfTraces.push_back(std::move(Trace));
   }
   return success();
@@ -438,8 +446,9 @@ InstrProfKind RawInstrProfReader<IntPtrT>::getProfileKind() const {
 }
 
 template <class IntPtrT>
-const SmallVector<TemporalProfTraceTy> &
-RawInstrProfReader<IntPtrT>::getTemporalProfTraces() {
+SmallVector<TemporalProfTraceTy> &
+RawInstrProfReader<IntPtrT>::getTemporalProfTraces(
+    std::optional<uint64_t> Weight) {
   if (TemporalProfTimestamps.empty()) {
     assert(TemporalProfTraces.empty());
     return TemporalProfTraces;
@@ -447,8 +456,10 @@ RawInstrProfReader<IntPtrT>::getTemporalProfTraces() {
   // Sort functions by their timestamps to build the trace.
   std::sort(TemporalProfTimestamps.begin(), TemporalProfTimestamps.end());
   TemporalProfTraceTy Trace;
+  if (Weight)
+    Trace.Weight = *Weight;
   for (auto &[TimestampValue, NameRef] : TemporalProfTimestamps)
-    Trace.push_back(NameRef);
+    Trace.FunctionNameRefs.push_back(NameRef);
   TemporalProfTraces = {std::move(Trace)};
   return TemporalProfTraces;
 }
@@ -1147,19 +1158,21 @@ Error IndexedInstrProfReader::readHeader() {
     TemporalProfTraceStreamSize =
         support::endian::readNext<uint64_t, little, unaligned>(Ptr);
     for (unsigned i = 0; i < NumTraces; i++) {
-      // Expect at least one 64 bit field: NumFunctions
-      if (Ptr + sizeof(uint64_t) > PtrEnd)
+      // Expect at least two 64 bit fields: Weight and NumFunctions
+      if (Ptr + 2 * sizeof(uint64_t) > PtrEnd)
         return error(instrprof_error::truncated);
+      TemporalProfTraceTy Trace;
+      Trace.Weight =
+          support::endian::readNext<uint64_t, little, unaligned>(Ptr);
       const uint64_t NumFunctions =
           support::endian::readNext<uint64_t, little, unaligned>(Ptr);
       // Expect at least NumFunctions 64 bit fields
       if (Ptr + NumFunctions * sizeof(uint64_t) > PtrEnd)
         return error(instrprof_error::truncated);
-      TemporalProfTraceTy Trace;
       for (unsigned j = 0; j < NumFunctions; j++) {
         const uint64_t NameRef =
             support::endian::readNext<uint64_t, little, unaligned>(Ptr);
-        Trace.push_back(NameRef);
+        Trace.FunctionNameRefs.push_back(NameRef);
       }
       TemporalProfTraces.push_back(std::move(Trace));
     }
index e181228..473fa35 100644 (file)
@@ -291,9 +291,9 @@ void InstrProfWriter::addBinaryIds(ArrayRef<llvm::object::BuildID> BIs) {
 }
 
 void InstrProfWriter::addTemporalProfileTrace(TemporalProfTraceTy Trace) {
-  if (Trace.size() > MaxTemporalProfTraceLength)
-    Trace.resize(MaxTemporalProfTraceLength);
-  if (Trace.empty())
+  if (Trace.FunctionNameRefs.size() > MaxTemporalProfTraceLength)
+    Trace.FunctionNameRefs.resize(MaxTemporalProfTraceLength);
+  if (Trace.FunctionNameRefs.empty())
     return;
 
   if (TemporalProfTraceStreamSize < TemporalProfTraceReservoirSize) {
@@ -311,7 +311,7 @@ void InstrProfWriter::addTemporalProfileTrace(TemporalProfTraceTy Trace) {
 }
 
 void InstrProfWriter::addTemporalProfileTraces(
-    SmallVector<TemporalProfTraceTy> SrcTraces, uint64_t SrcStreamSize) {
+    SmallVectorImpl<TemporalProfTraceTy> &SrcTraces, uint64_t SrcStreamSize) {
   // Assume that the source has the same reservoir size as the destination to
   // avoid needing to record it in the indexed profile format.
   bool IsDestSampled =
@@ -356,7 +356,7 @@ void InstrProfWriter::mergeRecordsFromWriter(InstrProfWriter &&IPW,
   for (auto &I : IPW.BinaryIds)
     addBinaryIds(I);
 
-  addTemporalProfileTraces(std::move(IPW.TemporalProfTraces),
+  addTemporalProfileTraces(IPW.TemporalProfTraces,
                            IPW.TemporalProfTraceStreamSize);
 
   MemProfFrameData.reserve(IPW.MemProfFrameData.size());
@@ -591,8 +591,9 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
     OS.write(TemporalProfTraces.size());
     OS.write(TemporalProfTraceStreamSize);
     for (auto &Trace : TemporalProfTraces) {
-      OS.write(Trace.size());
-      for (auto &NameRef : Trace)
+      OS.write(Trace.Weight);
+      OS.write(Trace.FunctionNameRefs.size());
+      for (auto &NameRef : Trace.FunctionNameRefs)
         OS.write(NameRef);
     }
   }
@@ -779,7 +780,8 @@ void InstrProfWriter::writeTextTemporalProfTraceData(raw_fd_ostream &OS,
   OS << "# Temporal Profile Trace Stream Size:\n"
      << TemporalProfTraceStreamSize << "\n";
   for (auto &Trace : TemporalProfTraces) {
-    for (auto &NameRef : Trace)
+    OS << "# Weight:\n" << Trace.Weight << "\n";
+    for (auto &NameRef : Trace.FunctionNameRefs)
       OS << Symtab.getFuncName(NameRef) << ",";
     OS << "\n";
   }
index c244a1f..bcf29ba 100644 (file)
 # SEEN2: Temporal Profile Traces (samples=2 seen=2):
 # SEEN3: Temporal Profile Traces (samples=2 seen=3):
 # SEEN4: Temporal Profile Traces (samples=2 seen=4):
-# SAMPLE1: Temporal Profile Trace 0 (count=3):
+# SAMPLE1: Temporal Profile Trace 0 (weight=1 count=3):
 # SAMPLE1:   a
 # SAMPLE1:   b
 # SAMPLE1:   c
-# SAMPLE2: Temporal Profile Trace 1 (count=3):
+# SAMPLE2: Temporal Profile Trace 1 (weight=1 count=3):
 # SAMPLE2:   a
 # SAMPLE2:   b
 # SAMPLE2:   c
@@ -27,6 +27,8 @@
 1
 # Trace Stream Size:
 1
+# Weight
+1
 a, b, c
 
 
index c811ef2..87f69fe 100644 (file)
@@ -6,15 +6,15 @@
 # RUN: llvm-profdata show --temporal-profile-traces %t.1.proftext | FileCheck %s
 
 # CHECK: Temporal Profile Traces (samples=3 seen=3):
-# CHECK: Temporal Profile Trace 0 (count=3):
+# CHECK: Temporal Profile Trace 0 (weight=1 count=3):
 # CHECK:   foo
 # CHECK:   bar
 # CHECK:   goo
-# CHECK: Temporal Profile Trace 1 (count=3):
+# CHECK: Temporal Profile Trace 1 (weight=3 count=3):
 # CHECK:   foo
 # CHECK:   goo
 # CHECK:   bar
-# CHECK: Temporal Profile Trace 2 (count=1):
+# CHECK: Temporal Profile Trace 2 (weight=1 count=1):
 # CHECK:   goo
 
 # Header
 3
 # Trace Stream Size:
 3
+# Weight
+1
 foo, bar, goo
+# Weight
+3
 foo,goo,bar,
+# Weight
+1
 goo
 
 foo
index f92e015..cf6edd6 100644 (file)
@@ -1,16 +1,16 @@
-# RUN: llvm-profdata merge --max-temporal-profile-trace-length=0 %s -o %t.profdata
+# RUN: llvm-profdata merge --temporal-profile-max-trace-length=0 %s -o %t.profdata
 # RUN: llvm-profdata show --temporal-profile-traces %t.profdata | FileCheck %s --check-prefix=NONE
 
-# RUN: llvm-profdata merge --max-temporal-profile-trace-length=2 %s -o %t.profdata
+# RUN: llvm-profdata merge --temporal-profile-max-trace-length=2 %s -o %t.profdata
 # RUN: llvm-profdata show --temporal-profile-traces %t.profdata | FileCheck %s --check-prefixes=CHECK,SOME
 
-# RUN: llvm-profdata merge --max-temporal-profile-trace-length=1000 %s -o %t.profdata
+# RUN: llvm-profdata merge --temporal-profile-max-trace-length=1000 %s -o %t.profdata
 # RUN: llvm-profdata show --temporal-profile-traces %t.profdata | FileCheck %s --check-prefixes=CHECK,ALL
 
 # NONE: Temporal Profile Traces (samples=0 seen=0):
 # CHECK: Temporal Profile Traces (samples=1 seen=1):
-# SOME:   Trace 0 (count=2):
-# ALL:    Trace 0 (count=3):
+# SOME:   Trace 0 (weight=1 count=2):
+# ALL:    Trace 0 (weight=1 count=3):
 
 # Header
 :ir
@@ -19,6 +19,8 @@
 1
 # Trace Stream Size:
 1
+# Weight
+1
 a, b, c
 
 
index 44219e4..3db5479 100644 (file)
@@ -344,7 +344,7 @@ static void loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
   }
 
   if (Reader->hasTemporalProfile()) {
-    auto &Traces = Reader->getTemporalProfTraces();
+    auto &Traces = Reader->getTemporalProfTraces(Input.Weight);
     if (!Traces.empty())
       WC->Writer.addTemporalProfileTraces(
           Traces, Reader->getTemporalProfTraceStreamSize());
@@ -1279,8 +1279,8 @@ static int merge_main(int argc, const char *argv[]) {
       "temporal-profile-trace-reservoir-size", cl::init(100),
       cl::desc("The maximum number of stored temporal profile traces (default: "
                "100)"));
-  cl::opt<uint64_t> MaxTemporalProfTraceLength(
-      "max-temporal-profile-trace-length", cl::init(10000),
+  cl::opt<uint64_t> TemporalProfMaxTraceLength(
+      "temporal-profile-max-trace-length", cl::init(10000),
       cl::desc("The maximum length of a single temporal profile trace "
                "(default: 10000)"));
 
@@ -1326,7 +1326,7 @@ static int merge_main(int argc, const char *argv[]) {
     mergeInstrProfile(WeightedInputs, DebugInfoFilename, Remapper.get(),
                       OutputFilename, OutputFormat,
                       TemporalProfTraceReservoirSize,
-                      MaxTemporalProfTraceLength, OutputSparse, NumThreads,
+                      TemporalProfMaxTraceLength, OutputSparse, NumThreads,
                       FailureMode, ProfiledBinary);
   else
     mergeSampleProfile(WeightedInputs, Remapper.get(), OutputFilename,
@@ -2635,9 +2635,9 @@ static int showInstrProfile(
     OS << "Temporal Profile Traces (samples=" << Traces.size()
        << " seen=" << Reader->getTemporalProfTraceStreamSize() << "):\n";
     for (unsigned i = 0; i < Traces.size(); i++) {
-      OS << "  Temporal Profile Trace " << i << " (count=" << Traces[i].size()
-         << "):\n";
-      for (auto &NameRef : Traces[i])
+      OS << "  Temporal Profile Trace " << i << " (weight=" << Traces[i].Weight
+         << " count=" << Traces[i].FunctionNameRefs.size() << "):\n";
+      for (auto &NameRef : Traces[i].FunctionNameRefs)
         OS << "    " << Reader->getSymtab().getFuncName(NameRef) << "\n";
     }
   }
index dc58347..26211bd 100644 (file)
@@ -39,6 +39,14 @@ ErrorEquals(instrprof_error Expected, Error E) {
   return ::testing::AssertionFailure() << "error: " << FoundMsg << "\n";
 }
 
+namespace llvm {
+bool operator==(const TemporalProfTraceTy &lhs,
+                const TemporalProfTraceTy &rhs) {
+  return lhs.Weight == rhs.Weight &&
+         lhs.FunctionNameRefs == rhs.FunctionNameRefs;
+}
+} // end namespace llvm
+
 namespace {
 
 struct InstrProfTest : ::testing::Test {
@@ -234,13 +242,15 @@ TEST_F(InstrProfTest, test_merge_temporal_prof_traces_truncated) {
   ASSERT_THAT_ERROR(Writer.mergeProfileKind(InstrProfKind::TemporalProfile),
                     Succeeded());
 
-  auto LargeTrace = {IndexedInstrProf::ComputeHash("foo"),
-                     IndexedInstrProf::ComputeHash("bar"),
-                     IndexedInstrProf::ComputeHash("goo")};
-  auto SmallTrace = {IndexedInstrProf::ComputeHash("foo"),
-                     IndexedInstrProf::ComputeHash("bar")};
+  TemporalProfTraceTy LargeTrace, SmallTrace;
+  LargeTrace.FunctionNameRefs = {IndexedInstrProf::ComputeHash("foo"),
+                                 IndexedInstrProf::ComputeHash("bar"),
+                                 IndexedInstrProf::ComputeHash("goo")};
+  SmallTrace.FunctionNameRefs = {IndexedInstrProf::ComputeHash("foo"),
+                                 IndexedInstrProf::ComputeHash("bar")};
 
-  Writer.addTemporalProfileTraces({LargeTrace, SmallTrace}, 2);
+  SmallVector Traces = {LargeTrace, SmallTrace};
+  Writer.addTemporalProfileTraces(Traces, 2);
 
   auto Profile = Writer.writeBuffer();
   readProfile(std::move(Profile));
@@ -261,11 +271,13 @@ TEST_F(InstrProfTest, test_merge_traces_from_writer) {
   ASSERT_THAT_ERROR(Writer2.mergeProfileKind(InstrProfKind::TemporalProfile),
                     Succeeded());
 
-  auto FooTrace = {IndexedInstrProf::ComputeHash("foo")};
-  auto BarTrace = {IndexedInstrProf::ComputeHash("bar")};
+  TemporalProfTraceTy FooTrace, BarTrace;
+  FooTrace.FunctionNameRefs = {IndexedInstrProf::ComputeHash("foo")};
+  BarTrace.FunctionNameRefs = {IndexedInstrProf::ComputeHash("bar")};
 
-  Writer.addTemporalProfileTraces({FooTrace}, 1);
-  Writer2.addTemporalProfileTraces({BarTrace}, 1);
+  SmallVector Traces1({FooTrace}), Traces2({BarTrace});
+  Writer.addTemporalProfileTraces(Traces1, 1);
+  Writer2.addTemporalProfileTraces(Traces2, 1);
   Writer.mergeRecordsFromWriter(std::move(Writer2), Err);
 
   auto Profile = Writer.writeBuffer();
@@ -284,15 +296,19 @@ TEST_F(InstrProfTest, test_merge_traces_sampled) {
   ASSERT_THAT_ERROR(Writer.mergeProfileKind(InstrProfKind::TemporalProfile),
                     Succeeded());
 
-  auto FooTrace = {IndexedInstrProf::ComputeHash("foo")};
-  auto BarTrace = {IndexedInstrProf::ComputeHash("bar")};
-  auto GooTrace = {IndexedInstrProf::ComputeHash("Goo")};
+  TemporalProfTraceTy FooTrace, BarTrace, GooTrace;
+  FooTrace.FunctionNameRefs = {IndexedInstrProf::ComputeHash("foo")};
+  BarTrace.FunctionNameRefs = {IndexedInstrProf::ComputeHash("bar")};
+  GooTrace.FunctionNameRefs = {IndexedInstrProf::ComputeHash("Goo")};
 
   // Add some sampled traces
-  Writer.addTemporalProfileTraces({FooTrace, BarTrace, GooTrace}, 5);
+  SmallVector SampledTraces = {FooTrace, BarTrace, GooTrace};
+  Writer.addTemporalProfileTraces(SampledTraces, 5);
   // Add some unsampled traces
-  Writer.addTemporalProfileTraces({BarTrace, GooTrace}, 2);
-  Writer.addTemporalProfileTraces({FooTrace}, 1);
+  SmallVector UnsampledTraces = {BarTrace, GooTrace};
+  Writer.addTemporalProfileTraces(UnsampledTraces, 2);
+  UnsampledTraces = {FooTrace};
+  Writer.addTemporalProfileTraces(UnsampledTraces, 1);
 
   auto Profile = Writer.writeBuffer();
   readProfile(std::move(Profile));