llvm-profdata: Reduce memory usage by using Error callback rather than member

dwblaikie · dwblaikie · commit 98cce00371d2 · 2017-07-10T03:04:59.000Z
Reduces llvm-profdata memory usage on a large profile from 7.8GB to 5.1GB. The ProfData API now supports reporting all the errors/warnings rather than only the first, though llvm-profdata ignores everything after the first for now to preserve existing behavior. (if there's a desire for other behavior, happy to implement that - but might be as well left for a separate patch) Reviewers: davidxl Differential Revision: https://reviews.llvm.org/D35149 llvm-svn: 307516
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -581,16 +581,15 @@ struct InstrProfValueSiteRecord {
 
   /// Merge data from another InstrProfValueSiteRecord
   /// Optionally scale merged counts by \p Weight.
-  void merge(SoftInstrProfErrors &SIPE, InstrProfValueSiteRecord &Input,
-             uint64_t Weight = 1);
+  void merge(InstrProfValueSiteRecord &Input, uint64_t Weight,
+             function_ref<void(instrprof_error)> Warn);
   /// Scale up value profile data counts.
-  void scale(SoftInstrProfErrors &SIPE, uint64_t Weight);
+  void scale(uint64_t Weight, function_ref<void(instrprof_error)> Warn);
 };
 
 /// Profiling information for a single function.
 struct InstrProfRecord {
   std::vector<uint64_t> Counts;
-  SoftInstrProfErrors SIPE;
 
   InstrProfRecord() = default;
   InstrProfRecord(std::vector<uint64_t> Counts) : Counts(std::move(Counts)) {}
@@ -653,11 +652,12 @@ struct InstrProfRecord {
 
   /// Merge the counts in \p Other into this one.
   /// Optionally scale merged counts by \p Weight.
-  void merge(InstrProfRecord &Other, uint64_t Weight = 1);
+  void merge(InstrProfRecord &Other, uint64_t Weight,
+             function_ref<void(instrprof_error)> Warn);
 
   /// Scale up profile counts (including value profile data) by
   /// \p Weight.
-  void scale(uint64_t Weight);
+  void scale(uint64_t Weight, function_ref<void(instrprof_error)> Warn);
 
   /// Sort value profile data (per site) by count.
   void sortValueData() {
@@ -675,9 +675,6 @@ struct InstrProfRecord {
   /// Clear value data entries
   void clearValueData() { ValueData = nullptr; }
 
-  /// Get the error contained within the record's soft error counter.
-  Error takeError() { return SIPE.takeError(); }
-
 private:
   struct ValueProfData {
     std::vector<InstrProfValueSiteRecord> IndirectCallSites;
@@ -729,11 +726,13 @@ struct InstrProfRecord {
 
   // Merge Value Profile data from Src record to this record for ValueKind.
   // Scale merged value counts by \p Weight.
-  void mergeValueProfData(uint32_t ValueKind, InstrProfRecord &Src,
-                          uint64_t Weight);
+  void mergeValueProfData(uint32_t ValkeKind, InstrProfRecord &Src,
+                          uint64_t Weight,
+                          function_ref<void(instrprof_error)> Warn);
 
   // Scale up value profile data count.
-  void scaleValueProfData(uint32_t ValueKind, uint64_t Weight);
+  void scaleValueProfData(uint32_t ValueKind, uint64_t Weight,
+                          function_ref<void(instrprof_error)> Warn);
 };
 
 struct NamedInstrProfRecord : InstrProfRecord {
diff --git a/llvm/include/llvm/ProfileData/InstrProfWriter.h b/llvm/include/llvm/ProfileData/InstrProfWriter.h
@@ -51,10 +51,15 @@ class InstrProfWriter {
   /// Add function counts for the given function. If there are already counts
   /// for this function and the hash and number of counts match, each counter is
   /// summed. Optionally scale counts by \p Weight.
-  Error addRecord(NamedInstrProfRecord &&I, uint64_t Weight = 1);
+  void addRecord(NamedInstrProfRecord &&I, uint64_t Weight,
+                 function_ref<void(Error)> Warn);
+  void addRecord(NamedInstrProfRecord &&I, function_ref<void(Error)> Warn) {
+    addRecord(std::move(I), 1, Warn);
+  }
 
   /// Merge existing function counts from the given writer.
-  Error mergeRecordsFromWriter(InstrProfWriter &&IPW);
+  void mergeRecordsFromWriter(InstrProfWriter &&IPW,
+                              function_ref<void(Error)> Warn);
 
   /// Write the profile to \c OS
   void write(raw_fd_ostream &OS);
@@ -87,8 +92,8 @@ class InstrProfWriter {
   void setOutputSparse(bool Sparse);
 
 private:
-  Error addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I,
-                  uint64_t Weight = 1);
+  void addRecord(StringRef Name, uint64_t Hash, InstrProfRecord &&I,
+                 uint64_t Weight, function_ref<void(Error)> Warn);
   bool shouldEncodeData(const ProfilingData &PD);
   void writeImpl(ProfOStream &OS);
 };
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
@@ -460,9 +460,9 @@ Error readPGOFuncNameStrings(StringRef NameStrings, InstrProfSymtab &Symtab) {
   return Error::success();
 }
 
-void InstrProfValueSiteRecord::merge(SoftInstrProfErrors &SIPE,
-                                     InstrProfValueSiteRecord &Input,
-                                     uint64_t Weight) {
+void InstrProfValueSiteRecord::merge(InstrProfValueSiteRecord &Input,
+                                     uint64_t Weight,
+                                     function_ref<void(instrprof_error)> Warn) {
   this->sortByTargetValues();
   Input.sortByTargetValues();
   auto I = ValueData.begin();
@@ -475,33 +475,33 @@ void InstrProfValueSiteRecord::merge(SoftInstrProfErrors &SIPE,
       bool Overflowed;
       I->Count = SaturatingMultiplyAdd(J->Count, Weight, I->Count, &Overflowed);
       if (Overflowed)
-        SIPE.addError(instrprof_error::counter_overflow);
+        Warn(instrprof_error::counter_overflow);
       ++I;
       continue;
     }
     ValueData.insert(I, *J);
   }
 }
 
-void InstrProfValueSiteRecord::scale(SoftInstrProfErrors &SIPE,
-                                     uint64_t Weight) {
+void InstrProfValueSiteRecord::scale(uint64_t Weight,
+                                     function_ref<void(instrprof_error)> Warn) {
   for (auto I = ValueData.begin(), IE = ValueData.end(); I != IE; ++I) {
     bool Overflowed;
     I->Count = SaturatingMultiply(I->Count, Weight, &Overflowed);
     if (Overflowed)
-      SIPE.addError(instrprof_error::counter_overflow);
+      Warn(instrprof_error::counter_overflow);
   }
 }
 
 // Merge Value Profile data from Src record to this record for ValueKind.
 // Scale merged value counts by \p Weight.
-void InstrProfRecord::mergeValueProfData(uint32_t ValueKind,
-                                         InstrProfRecord &Src,
-                                         uint64_t Weight) {
+void InstrProfRecord::mergeValueProfData(
+    uint32_t ValueKind, InstrProfRecord &Src, uint64_t Weight,
+    function_ref<void(instrprof_error)> Warn) {
   uint32_t ThisNumValueSites = getNumValueSites(ValueKind);
   uint32_t OtherNumValueSites = Src.getNumValueSites(ValueKind);
   if (ThisNumValueSites != OtherNumValueSites) {
-    SIPE.addError(instrprof_error::value_site_count_mismatch);
+    Warn(instrprof_error::value_site_count_mismatch);
     return;
   }
   if (!ThisNumValueSites)
@@ -511,14 +511,15 @@ void InstrProfRecord::mergeValueProfData(uint32_t ValueKind,
   MutableArrayRef<InstrProfValueSiteRecord> OtherSiteRecords =
       Src.getValueSitesForKind(ValueKind);
   for (uint32_t I = 0; I < ThisNumValueSites; I++)
-    ThisSiteRecords[I].merge(SIPE, OtherSiteRecords[I], Weight);
+    ThisSiteRecords[I].merge(OtherSiteRecords[I], Weight, Warn);
 }
 
-void InstrProfRecord::merge(InstrProfRecord &Other, uint64_t Weight) {
+void InstrProfRecord::merge(InstrProfRecord &Other, uint64_t Weight,
+                            function_ref<void(instrprof_error)> Warn) {
   // If the number of counters doesn't match we either have bad data
   // or a hash collision.
   if (Counts.size() != Other.Counts.size()) {
-    SIPE.addError(instrprof_error::count_mismatch);
+    Warn(instrprof_error::count_mismatch);
     return;
   }
 
@@ -527,27 +528,30 @@ void InstrProfRecord::merge(InstrProfRecord &Other, uint64_t Weight) {
     Counts[I] =
         SaturatingMultiplyAdd(Other.Counts[I], Weight, Counts[I], &Overflowed);
     if (Overflowed)
-      SIPE.addError(instrprof_error::counter_overflow);
+      Warn(instrprof_error::counter_overflow);
   }
 
   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
-    mergeValueProfData(Kind, Other, Weight);
+    mergeValueProfData(Kind, Other, Weight, Warn);
 }
 
-void InstrProfRecord::scaleValueProfData(uint32_t ValueKind, uint64_t Weight) {
+void InstrProfRecord::scaleValueProfData(
+    uint32_t ValueKind, uint64_t Weight,
+    function_ref<void(instrprof_error)> Warn) {
   for (auto &R : getValueSitesForKind(ValueKind))
-    R.scale(SIPE, Weight);
+    R.scale(Weight, Warn);
 }
 
-void InstrProfRecord::scale(uint64_t Weight) {
+void InstrProfRecord::scale(uint64_t Weight,
+                            function_ref<void(instrprof_error)> Warn) {
   for (auto &Count : this->Counts) {
     bool Overflowed;
     Count = SaturatingMultiply(Count, Weight, &Overflowed);
     if (Overflowed)
-      SIPE.addError(instrprof_error::counter_overflow);
+      Warn(instrprof_error::counter_overflow);
   }
   for (uint32_t Kind = IPVK_First; Kind <= IPVK_Last; ++Kind)
-    scaleValueProfData(Kind, Weight);
+    scaleValueProfData(Kind, Weight, Warn);
 }
 
 // Map indirect call target name hash to name string.
diff --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -176,14 +176,16 @@ void InstrProfWriter::setOutputSparse(bool Sparse) {
   this->Sparse = Sparse;
 }
 
-Error InstrProfWriter::addRecord(NamedInstrProfRecord &&I, uint64_t Weight) {
+void InstrProfWriter::addRecord(NamedInstrProfRecord &&I, uint64_t Weight,
+                                function_ref<void(Error)> Warn) {
   auto Name = I.Name;
   auto Hash = I.Hash;
-  return addRecord(Name, Hash, std::move(I), Weight);
+  addRecord(Name, Hash, std::move(I), Weight, Warn);
 }
 
-Error InstrProfWriter::addRecord(StringRef Name, uint64_t Hash,
-                                 InstrProfRecord &&I, uint64_t Weight) {
+void InstrProfWriter::addRecord(StringRef Name, uint64_t Hash,
+                                InstrProfRecord &&I, uint64_t Weight,
+                                function_ref<void(Error)> Warn) {
   auto &ProfileDataMap = FunctionData[Name];
 
   bool NewFunc;
@@ -192,27 +194,28 @@ Error InstrProfWriter::addRecord(StringRef Name, uint64_t Hash,
       ProfileDataMap.insert(std::make_pair(Hash, InstrProfRecord()));
   InstrProfRecord &Dest = Where->second;
 
+  auto MapWarn = [&](instrprof_error E) {
+    Warn(make_error<InstrProfError>(E));
+  };
+
   if (NewFunc) {
     // We've never seen a function with this name and hash, add it.
     Dest = std::move(I);
     if (Weight > 1)
-      Dest.scale(Weight);
+      Dest.scale(Weight, MapWarn);
   } else {
     // We're updating a function we've seen before.
-    Dest.merge(I, Weight);
+    Dest.merge(I, Weight, MapWarn);
   }
 
   Dest.sortValueData();
-
-  return Dest.takeError();
 }
 
-Error InstrProfWriter::mergeRecordsFromWriter(InstrProfWriter &&IPW) {
+void InstrProfWriter::mergeRecordsFromWriter(InstrProfWriter &&IPW,
+                                             function_ref<void(Error)> Warn) {
   for (auto &I : IPW.FunctionData)
     for (auto &Func : I.getValue())
-      if (Error E = addRecord(I.getKey(), Func.first, std::move(Func.second)))
-        return E;
-  return Error::success();
+      addRecord(I.getKey(), Func.first, std::move(Func.second), 1, Warn);
 }
 
 bool InstrProfWriter::shouldEncodeData(const ProfilingData &PD) {
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -159,23 +159,36 @@ static void loadInput(const WeightedFile &Input, WriterContext *WC) {
 
   for (auto &I : *Reader) {
     const StringRef FuncName = I.Name;
-    if (Error E = WC->Writer.addRecord(std::move(I), Input.Weight)) {
+    bool Reported = false;
+    WC->Writer.addRecord(std::move(I), Input.Weight, [&](Error E) {
+      if (Reported) {
+        consumeError(std::move(E));
+        return;
+      }
+      Reported = true;
       // Only show hint the first time an error occurs.
       instrprof_error IPE = InstrProfError::take(std::move(E));
       std::unique_lock<std::mutex> ErrGuard{WC->ErrLock};
       bool firstTime = WC->WriterErrorCodes.insert(IPE).second;
       handleMergeWriterError(make_error<InstrProfError>(IPE), Input.Filename,
                              FuncName, firstTime);
-    }
+    });
   }
   if (Reader->hasError())
     WC->Err = Reader->getError();
 }
 
 /// Merge the \p Src writer context into \p Dst.
 static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
-  if (Error E = Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer)))
+  bool Reported = false;
+  Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) {
+    if (Reported) {
+      consumeError(std::move(E));
+      return;
+    }
+    Reported = true;
     Dst->Err = std::move(E);
+  });
 }
 
 static void mergeInstrProfile(const WeightedFileVector &Inputs,
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
diff --git a/llvm/unittests/ProfileData/InstrProfTest.cpp b/llvm/unittests/ProfileData/InstrProfTest.cpp
