[compiler-rt] [llvm] [clang-tools-extra] [clang] [Profile] Allow profile merging with multiple correlate files. (PR #75957)

Ellis Hoag via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 20 11:26:41 PST 2023


================
@@ -481,3 +509,49 @@ Error BinaryInstrProfCorrelator<IntPtrT>::correlateProfileNameImpl() {
   this->Names.append(this->Ctx->NameStart, this->Ctx->NameSize);
   return Error::success();
 }
+
+llvm::Expected<std::unique_ptr<InstrProfCorrelators>> InstrProfCorrelators::get(
+    ArrayRef<std::pair<StringRef, InstrProfCorrelator::ProfCorrelatorKind>>
+        CorrelateInputs,
+    uint32_t MaxWarnings) {
+  StringMap<std::unique_ptr<InstrProfCorrelator>> CorrelatorMap;
+  StringMap<StringRef> FileMap;
+  auto WarnCounter =
+      std::make_unique<InstrProfCorrelator::AtomicWarningCounter>(MaxWarnings);
+  std::unique_ptr<std::mutex> CorrelateLock = std::make_unique<std::mutex>();
+  std::unique_ptr<std::mutex> WarnLock = std::make_unique<std::mutex>();
+  for (const auto &Input : CorrelateInputs) {
+    std::unique_ptr<InstrProfCorrelator> Correlator;
+    if (auto Err = InstrProfCorrelator::get(Input.first, Input.second,
+                                            *CorrelateLock.get(),
+                                            *WarnLock.get(), WarnCounter.get())
+                       .moveInto(Correlator))
+      return Err;
+    std::string BuildID = toHex(Correlator->getBuildID());
+    FileMap.try_emplace(BuildID, Input.first);
+    bool Inserted =
+        CorrelatorMap.try_emplace(BuildID, std::move(Correlator)).second;
+    if (!Inserted && WarnCounter->shouldEmitWarning()) {
+      std::lock_guard<std::mutex> Guard(*WarnLock);
+      WithColor::warning() << format(
+          "Duplicate build id (%s) found for %s and %s\n", BuildID.c_str(),
+          FileMap[BuildID].str().c_str(), Input.first.str().c_str());
+    }
+  }
+  return std::make_unique<InstrProfCorrelators>(
+      std::move(CorrelatorMap), std::move(CorrelateLock), std::move(WarnLock),
+      std::move(WarnCounter));
+}
+
+llvm::Expected<const InstrProfCorrelator *>
+InstrProfCorrelators::getCorrelator(object::BuildIDRef BuildID) const {
+  std::string BuildIDStr = toHex(BuildID);
+  auto I = CorrelatorMap.find(BuildIDStr);
+  if (I == CorrelatorMap.end())
+    return make_error<InstrProfError>(
+        instrprof_error::unable_to_correlate_profile,
+        "missing correlator file with build id " + BuildIDStr + "\n");
+  if (auto Err = I->getValue()->correlateProfileData())
----------------
ellishg wrote:

It seems that we are doing a "lazy correlation" by only calling `correlateProfileData()` when needed by the `InstrProfReader`. As I understand, this is the reason that this patch adds atomics and mutexes, which makes this patch significantly more complicated. Could we avoid this by calling `correlateProfileData()` once for each debug info/binary file before reading any raw profiles? I guess this would be wasted work if it turns out we don't need some debug info/binary file, but the user could easily avoid that wasted work by not passing in the file in the first place.

https://github.com/llvm/llvm-project/pull/75957


More information about the llvm-commits mailing list