Revert "Reduce llvm-gsymutil memory usage"

[peremyach]

Reverts #139907 as per discussion in #140545 due to tests becoming flaky

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: None (peremyach)

Changes

Reverts llvm/llvm-project#139907 as per discussion in #140545 due to tests becoming flaky

---

Full diff: https://github.com/llvm/llvm-project/pull/140696.diff

5 Files Affected:

• (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h (-5)
• (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h (+3-3)
• (modified) llvm/lib/DebugInfo/DWARF/DWARFContext.cpp (+1-8)
• (modified) llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp (+103-112)
• (modified) llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp (+6-13)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
index b39fb6852d700..6df3f5066e327 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
@@ -103,7 +103,6 @@ class DWARFContext : public DIContext {
     std::unique_ptr<DWARFDebugMacro>
     parseMacroOrMacinfo(MacroSecType SectionType);
 
-    virtual Error doWorkThreadSafely(function_ref<Error()> Work) = 0;
   };
   friend class DWARFContextState;
 
@@ -492,10 +491,6 @@ class DWARFContext : public DIContext {
   /// manually only for DWARF5.
   void setParseCUTUIndexManually(bool PCUTU) { ParseCUTUIndexManually = PCUTU; }
 
-  Error doWorkThreadSafely(function_ref<Error()> Work) {
-    return State->doWorkThreadSafely(Work);
-  }
-
 private:
   void addLocalsForDie(DWARFCompileUnit *CU, DWARFDie Subprogram, DWARFDie Die,
                        std::vector<DILocal> &Result);
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
index 0f7958f28065d..80c27aea89312 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
@@ -566,9 +566,6 @@ class DWARFUnit {
 
   Error tryExtractDIEsIfNeeded(bool CUDieOnly);
 
-  /// clearDIEs - Clear parsed DIEs to keep memory usage low.
-  void clearDIEs(bool KeepCUDie, bool KeepDWODies = false);
-
 private:
   /// Size in bytes of the .debug_info data associated with this compile unit.
   size_t getDebugInfoSize() const {
@@ -584,6 +581,9 @@ class DWARFUnit {
   void extractDIEsToVector(bool AppendCUDie, bool AppendNonCUDIEs,
                            std::vector<DWARFDebugInfoEntry> &DIEs) const;
 
+  /// clearDIEs - Clear parsed DIEs to keep memory usage low.
+  void clearDIEs(bool KeepCUDie);
+
   /// parseDWO - Parses .dwo file for current compile unit. Returns true if
   /// it was actually constructed.
   /// The \p AlternativeLocation specifies an alternative location to get
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index 1d2f379d1509b..e76e518ef8595 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -622,9 +622,7 @@ class ThreadUnsafeDWARFContextState : public DWARFContext::DWARFContextState {
       return getNormalTypeUnitMap();
   }
 
-  Error doWorkThreadSafely(function_ref<Error()> Work) override {
-    return Work();
-  }
+
 };
 
 class ThreadSafeState : public ThreadUnsafeDWARFContextState {
@@ -740,11 +738,6 @@ class ThreadSafeState : public ThreadUnsafeDWARFContextState {
     std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
     return ThreadUnsafeDWARFContextState::getTypeUnitMap(IsDWO);
   }
-
-  Error doWorkThreadSafely(function_ref<Error()> Work) override {
-    std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
-    return ThreadUnsafeDWARFContextState::doWorkThreadSafely(Work);
-  }
 };
 } // namespace
 
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index 8dc4050e2d8a2..bdd04b00f557b 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -496,111 +496,108 @@ void DWARFUnit::extractDIEsIfNeeded(bool CUDieOnly) {
 }
 
 Error DWARFUnit::tryExtractDIEsIfNeeded(bool CUDieOnly) {
-  return Context.doWorkThreadSafely([&]() -> Error {
-    if ((CUDieOnly && !DieArray.empty()) || DieArray.size() > 1)
-      return Error::success(); // Already parsed.
-
-    bool HasCUDie = !DieArray.empty();
-    extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
-
-    if (DieArray.empty())
-      return Error::success();
-
-    // If CU DIE was just parsed, copy several attribute values from it.
-    if (HasCUDie)
-      return Error::success();
-
-    DWARFDie UnitDie(this, &DieArray[0]);
-    if (std::optional<uint64_t> DWOId =
-            toUnsigned(UnitDie.find(DW_AT_GNU_dwo_id)))
-      Header.setDWOId(*DWOId);
-    if (!IsDWO) {
-      assert(AddrOffsetSectionBase == std::nullopt);
-      assert(RangeSectionBase == 0);
-      assert(LocSectionBase == 0);
-      AddrOffsetSectionBase = toSectionOffset(UnitDie.find(DW_AT_addr_base));
-      if (!AddrOffsetSectionBase)
-        AddrOffsetSectionBase =
-            toSectionOffset(UnitDie.find(DW_AT_GNU_addr_base));
-      RangeSectionBase = toSectionOffset(UnitDie.find(DW_AT_rnglists_base), 0);
-      LocSectionBase = toSectionOffset(UnitDie.find(DW_AT_loclists_base), 0);
-    }
+  if ((CUDieOnly && !DieArray.empty()) ||
+      DieArray.size() > 1)
+    return Error::success(); // Already parsed.
 
-    // In general, in DWARF v5 and beyond we derive the start of the unit's
-    // contribution to the string offsets table from the unit DIE's
-    // DW_AT_str_offsets_base attribute. Split DWARF units do not use this
-    // attribute, so we assume that there is a contribution to the string
-    // offsets table starting at offset 0 of the debug_str_offsets.dwo section.
-    // In both cases we need to determine the format of the contribution,
-    // which may differ from the unit's format.
-    DWARFDataExtractor DA(Context.getDWARFObj(), StringOffsetSection,
-                          IsLittleEndian, 0);
-    if (IsDWO || getVersion() >= 5) {
-      auto StringOffsetOrError =
-          IsDWO ? determineStringOffsetsTableContributionDWO(DA)
-                : determineStringOffsetsTableContribution(DA);
-      if (!StringOffsetOrError) {
-        return createStringError(errc::invalid_argument,
-                                 "invalid reference to or invalid content in "
-                                 ".debug_str_offsets[.dwo]: " +
-                                     toString(StringOffsetOrError.takeError()));
-      }
+  bool HasCUDie = !DieArray.empty();
+  extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
 
-      StringOffsetsTableContribution = *StringOffsetOrError;
-    }
+  if (DieArray.empty())
+    return Error::success();
 
-    // DWARF v5 uses the .debug_rnglists and .debug_rnglists.dwo sections to
-    // describe address ranges.
-    if (getVersion() >= 5) {
-      // In case of DWP, the base offset from the index has to be added.
-      if (IsDWO) {
-        uint64_t ContributionBaseOffset = 0;
-        if (auto *IndexEntry = Header.getIndexEntry())
-          if (auto *Contrib = IndexEntry->getContribution(DW_SECT_RNGLISTS))
-            ContributionBaseOffset = Contrib->getOffset();
-        setRangesSection(
-            &Context.getDWARFObj().getRnglistsDWOSection(),
-            ContributionBaseOffset +
-                DWARFListTableHeader::getHeaderSize(Header.getFormat()));
-      } else
-        setRangesSection(&Context.getDWARFObj().getRnglistsSection(),
-                         toSectionOffset(UnitDie.find(DW_AT_rnglists_base),
-                                         DWARFListTableHeader::getHeaderSize(
-                                             Header.getFormat())));
-    }
+  // If CU DIE was just parsed, copy several attribute values from it.
+  if (HasCUDie)
+    return Error::success();
+
+  DWARFDie UnitDie(this, &DieArray[0]);
+  if (std::optional<uint64_t> DWOId =
+          toUnsigned(UnitDie.find(DW_AT_GNU_dwo_id)))
+    Header.setDWOId(*DWOId);
+  if (!IsDWO) {
+    assert(AddrOffsetSectionBase == std::nullopt);
+    assert(RangeSectionBase == 0);
+    assert(LocSectionBase == 0);
+    AddrOffsetSectionBase = toSectionOffset(UnitDie.find(DW_AT_addr_base));
+    if (!AddrOffsetSectionBase)
+      AddrOffsetSectionBase =
+          toSectionOffset(UnitDie.find(DW_AT_GNU_addr_base));
+    RangeSectionBase = toSectionOffset(UnitDie.find(DW_AT_rnglists_base), 0);
+    LocSectionBase = toSectionOffset(UnitDie.find(DW_AT_loclists_base), 0);
+  }
+
+  // In general, in DWARF v5 and beyond we derive the start of the unit's
+  // contribution to the string offsets table from the unit DIE's
+  // DW_AT_str_offsets_base attribute. Split DWARF units do not use this
+  // attribute, so we assume that there is a contribution to the string
+  // offsets table starting at offset 0 of the debug_str_offsets.dwo section.
+  // In both cases we need to determine the format of the contribution,
+  // which may differ from the unit's format.
+  DWARFDataExtractor DA(Context.getDWARFObj(), StringOffsetSection,
+                        IsLittleEndian, 0);
+  if (IsDWO || getVersion() >= 5) {
+    auto StringOffsetOrError =
+        IsDWO ? determineStringOffsetsTableContributionDWO(DA)
+              : determineStringOffsetsTableContribution(DA);
+    if (!StringOffsetOrError)
+      return createStringError(errc::invalid_argument,
+                               "invalid reference to or invalid content in "
+                               ".debug_str_offsets[.dwo]: " +
+                                   toString(StringOffsetOrError.takeError()));
+
+    StringOffsetsTableContribution = *StringOffsetOrError;
+  }
 
+  // DWARF v5 uses the .debug_rnglists and .debug_rnglists.dwo sections to
+  // describe address ranges.
+  if (getVersion() >= 5) {
+    // In case of DWP, the base offset from the index has to be added.
     if (IsDWO) {
-      // If we are reading a package file, we need to adjust the location list
-      // data based on the index entries.
-      StringRef Data = Header.getVersion() >= 5
-                           ? Context.getDWARFObj().getLoclistsDWOSection().Data
-                           : Context.getDWARFObj().getLocDWOSection().Data;
+      uint64_t ContributionBaseOffset = 0;
       if (auto *IndexEntry = Header.getIndexEntry())
-        if (const auto *C = IndexEntry->getContribution(
-                Header.getVersion() >= 5 ? DW_SECT_LOCLISTS : DW_SECT_EXT_LOC))
-          Data = Data.substr(C->getOffset(), C->getLength());
-
-      DWARFDataExtractor DWARFData(Data, IsLittleEndian, getAddressByteSize());
-      LocTable =
-          std::make_unique<DWARFDebugLoclists>(DWARFData, Header.getVersion());
-      LocSectionBase = DWARFListTableHeader::getHeaderSize(Header.getFormat());
-    } else if (getVersion() >= 5) {
-      LocTable = std::make_unique<DWARFDebugLoclists>(
-          DWARFDataExtractor(Context.getDWARFObj(),
-                             Context.getDWARFObj().getLoclistsSection(),
-                             IsLittleEndian, getAddressByteSize()),
-          getVersion());
-    } else {
-      LocTable = std::make_unique<DWARFDebugLoc>(DWARFDataExtractor(
-          Context.getDWARFObj(), Context.getDWARFObj().getLocSection(),
-          IsLittleEndian, getAddressByteSize()));
-    }
+        if (auto *Contrib = IndexEntry->getContribution(DW_SECT_RNGLISTS))
+          ContributionBaseOffset = Contrib->getOffset();
+      setRangesSection(
+          &Context.getDWARFObj().getRnglistsDWOSection(),
+          ContributionBaseOffset +
+              DWARFListTableHeader::getHeaderSize(Header.getFormat()));
+    } else
+      setRangesSection(&Context.getDWARFObj().getRnglistsSection(),
+                       toSectionOffset(UnitDie.find(DW_AT_rnglists_base),
+                                       DWARFListTableHeader::getHeaderSize(
+                                           Header.getFormat())));
+  }
 
-    // Don't fall back to DW_AT_GNU_ranges_base: it should be ignored for
-    // skeleton CU DIE, so that DWARF users not aware of it are not broken.
+  if (IsDWO) {
+    // If we are reading a package file, we need to adjust the location list
+    // data based on the index entries.
+    StringRef Data = Header.getVersion() >= 5
+                         ? Context.getDWARFObj().getLoclistsDWOSection().Data
+                         : Context.getDWARFObj().getLocDWOSection().Data;
+    if (auto *IndexEntry = Header.getIndexEntry())
+      if (const auto *C = IndexEntry->getContribution(
+              Header.getVersion() >= 5 ? DW_SECT_LOCLISTS : DW_SECT_EXT_LOC))
+        Data = Data.substr(C->getOffset(), C->getLength());
+
+    DWARFDataExtractor DWARFData(Data, IsLittleEndian, getAddressByteSize());
+    LocTable =
+        std::make_unique<DWARFDebugLoclists>(DWARFData, Header.getVersion());
+    LocSectionBase = DWARFListTableHeader::getHeaderSize(Header.getFormat());
+  } else if (getVersion() >= 5) {
+    LocTable = std::make_unique<DWARFDebugLoclists>(
+        DWARFDataExtractor(Context.getDWARFObj(),
+                           Context.getDWARFObj().getLoclistsSection(),
+                           IsLittleEndian, getAddressByteSize()),
+        getVersion());
+  } else {
+    LocTable = std::make_unique<DWARFDebugLoc>(DWARFDataExtractor(
+        Context.getDWARFObj(), Context.getDWARFObj().getLocSection(),
+        IsLittleEndian, getAddressByteSize()));
+  }
 
-    return Error::success();
-  });
+  // Don't fall back to DW_AT_GNU_ranges_base: it should be ignored for
+  // skeleton CU DIE, so that DWARF users not aware of it are not broken.
+  return Error::success();
 }
 
 bool DWARFUnit::parseDWO(StringRef DWOAlternativeLocation) {
@@ -655,21 +652,15 @@ bool DWARFUnit::parseDWO(StringRef DWOAlternativeLocation) {
   return true;
 }
 
-void DWARFUnit::clearDIEs(bool KeepCUDie, bool KeepDWODies) {
-  assert(!Context.doWorkThreadSafely([&] {
-    if (!KeepDWODies && DWO) {
-      DWO->clearDIEs(KeepCUDie, KeepDWODies);
-    }
-    // Do not use resize() + shrink_to_fit() to free memory occupied by dies.
-    // shrink_to_fit() is a *non-binding* request to reduce capacity() to
-    // size(). It depends on the implementation whether the request is
-    // fulfilled. Create a new vector with a small capacity and assign it to the
-    // DieArray to have previous contents freed.
-    DieArray = (KeepCUDie && !DieArray.empty())
-                   ? std::vector<DWARFDebugInfoEntry>({DieArray[0]})
-                   : std::vector<DWARFDebugInfoEntry>();
-    return Error::success();
-  }));
+void DWARFUnit::clearDIEs(bool KeepCUDie) {
+  // Do not use resize() + shrink_to_fit() to free memory occupied by dies.
+  // shrink_to_fit() is a *non-binding* request to reduce capacity() to size().
+  // It depends on the implementation whether the request is fulfilled.
+  // Create a new vector with a small capacity and assign it to the DieArray to
+  // have previous contents freed.
+  DieArray = (KeepCUDie && !DieArray.empty())
+                 ? std::vector<DWARFDebugInfoEntry>({DieArray[0]})
+                 : std::vector<DWARFDebugInfoEntry>();
 }
 
 Expected<DWARFAddressRangesVector>
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index 1f70d273a9d9d..7a0256f10ea60 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -656,11 +656,6 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
       DWARFDie Die = getDie(*CU);
       CUInfo CUI(DICtx, dyn_cast<DWARFCompileUnit>(CU.get()));
       handleDie(Out, CUI, Die);
-      // Release the line table, once we're done.
-      DICtx.clearLineTableForUnit(CU.get());
-      // Free any DIEs that were allocated by the DWARF parser.
-      // If/when they're needed by other CU's, they'll be recreated.
-      CU->clearDIEs(/*KeepCUDie=*/false, /*KeepDWODIEs=*/false);
     }
   } else {
     // LLVM Dwarf parser is not thread-safe and we need to parse all DWARF up
@@ -673,7 +668,12 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
     for (const auto &CU : DICtx.compile_units())
       CU->getAbbreviations();
 
+    // Now parse all DIEs in case we have cross compile unit references in a
+    // thread pool.
     DefaultThreadPool pool(hardware_concurrency(NumThreads));
+    for (const auto &CU : DICtx.compile_units())
+      pool.async([&CU]() { CU->getUnitDIE(false /*CUDieOnly*/); });
+    pool.wait();
 
     // Now convert all DWARF to GSYM in a thread pool.
     std::mutex LogMutex;
@@ -681,15 +681,11 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
       DWARFDie Die = getDie(*CU);
       if (Die) {
         CUInfo CUI(DICtx, dyn_cast<DWARFCompileUnit>(CU.get()));
-        pool.async([this, CUI, &CU, &LogMutex, &Out, Die]() mutable {
+        pool.async([this, CUI, &LogMutex, &Out, Die]() mutable {
           std::string storage;
           raw_string_ostream StrStream(storage);
           OutputAggregator ThreadOut(Out.GetOS() ? &StrStream : nullptr);
           handleDie(ThreadOut, CUI, Die);
-          DICtx.clearLineTableForUnit(CU.get());
-          // Free any DIEs that were allocated by the DWARF parser.
-          // If/when they're needed by other CU's, they'll be recreated.
-          CU->clearDIEs(/*KeepCUDie=*/false, /*KeepDWODIEs=*/false);
           // Print ThreadLogStorage lines into an actual stream under a lock
           std::lock_guard<std::mutex> guard(LogMutex);
           if (Out.GetOS()) {
@@ -701,9 +697,6 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
     }
     pool.wait();
   }
-  // Now get rid of all the DIEs that may have been recreated
-  for (const auto &CU : DICtx.compile_units())
-    CU->clearDIEs(/*KeepCUDie=*/false, /*KeepDWODIEs=*/false);
   size_t FunctionsAddedCount = Gsym.getNumFunctionInfos() - NumBefore;
   Out << "Loaded " << FunctionsAddedCount << " functions from DWARF.\n";
   return Error::success();

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[danilaml]

LGTM!