[llvm] [BOLT][DWARF][NFC] Change how we re-size CloneUnitCtxMap (PR #75876)

[Alexander Yermolovich via llvm-commits]

https://github.com/ayermolo updated https://github.com/llvm/llvm-project/pull/75876

>From 721d8a6d5113cf93b125789027b1fcbfe240a2ea Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Mon, 18 Dec 2023 14:11:10 -0800
Subject: [PATCH 1/2] [BOLT][DWARF][NFC] Change how we re-size CloneUnitCtxMap

We would always allocate maximum amount for vector containing DWARFUnitInfo. In
real usecases what ends up hapenning is we allocate a giant vector when
processing one CU, or for thin-lto case multiple CUs. This lead to a lot of
memory overhead, and 2x BOLT processing slowdown for at least one service built
with monolithic DWARF.

For binaries built with LTO with clang all of CUs that have cross references
will share an abbrev table and will be processed in one batch. Rest of CUs are
processesd in --cu-processing-batch-size size. Which defaults to 1.

For theoretical cases where cross-cu references are present, but they do not
share abbrev will increase the size of CloneUnitCtxMap as each CU is being
processsed.
---
 bolt/lib/Core/DIEBuilder.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp
index caa5ecbea521dc..4aa54521076e31 100644
--- a/bolt/lib/Core/DIEBuilder.cpp
+++ b/bolt/lib/Core/DIEBuilder.cpp
@@ -272,7 +272,7 @@ void DIEBuilder::buildCompileUnits(const std::vector<DWARFUnit *> &CUs) {
   // set, even if they themselves don't have src cross cu ref. We could have
   // cases where this is not the case. In which case this container needs to be
   // big enough for all.
-  getState().CloneUnitCtxMap.resize(DwarfContext->getNumCompileUnits());
+  getState().CloneUnitCtxMap.resize(CUs.size());
   getState().Type = ProcessingType::CUs;
   for (DWARFUnit *CU : CUs)
     registerUnit(*CU, false);
@@ -897,6 +897,8 @@ void DIEBuilder::registerUnit(DWARFUnit &DU, bool NeedSort) {
                 });
   }
   getState().UnitIDMap[getHash(DU)] = getState().DUList.size();
+  if (getState().DUList.size() == getState().CloneUnitCtxMap.size())
+    getState().CloneUnitCtxMap.emplace_back();
   getState().DUList.push_back(&DU);
 }
 

>From 5856dd1eea081a9df597bc780ac00fa6801715ab Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Wed, 20 Dec 2023 14:28:10 -0800
Subject: [PATCH 2/2] Updated comments

---
 bolt/lib/Core/DIEBuilder.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp
index 4aa54521076e31..8d16f499bf2fc3 100644
--- a/bolt/lib/Core/DIEBuilder.cpp
+++ b/bolt/lib/Core/DIEBuilder.cpp
@@ -266,12 +266,10 @@ void DIEBuilder::buildCompileUnits(const bool Init) {
 }
 void DIEBuilder::buildCompileUnits(const std::vector<DWARFUnit *> &CUs) {
   BuilderState.reset(new State());
-  // Initializing to full size because there could be cross CU references with
-  // different abbrev offsets. LLVM happens to output CUs that have cross CU
-  // references with the same abbrev table. So destinations end up in the first
-  // set, even if they themselves don't have src cross cu ref. We could have
-  // cases where this is not the case. In which case this container needs to be
-  // big enough for all.
+  // Allocating enough for current batch being processed.
+  // In real use cases we either processing a batch of CUs with no cross
+  // references, or if they do have them it is due to LTO. With clang they will share the
+  // same abbrev table. In either case this vector will not grow.
   getState().CloneUnitCtxMap.resize(CUs.size());
   getState().Type = ProcessingType::CUs;
   for (DWARFUnit *CU : CUs)
@@ -897,6 +895,8 @@ void DIEBuilder::registerUnit(DWARFUnit &DU, bool NeedSort) {
                 });
   }
   getState().UnitIDMap[getHash(DU)] = getState().DUList.size();
+  // This handles the case where we do have cross cu references, but CUs do not
+  // share the same abbrev table.
   if (getState().DUList.size() == getState().CloneUnitCtxMap.size())
     getState().CloneUnitCtxMap.emplace_back();
   getState().DUList.push_back(&DU);