[llvm] Reduce llvm-gsymutil memory usage (PR #91023)
Kevin Frei via llvm-commits
llvm-commits at lists.llvm.org
Mon May 13 11:55:01 PDT 2024
https://github.com/kevinfrei updated https://github.com/llvm/llvm-project/pull/91023
>From f4b8d258fa95967a17b6851ffd6a1eb096625280 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Tue, 30 Apr 2024 09:15:12 -0700
Subject: [PATCH 1/5] Release line tables when we're done with them
---
llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index 601686fdd3dd5..9376515e3712f 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -587,6 +587,8 @@ 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());
}
} else {
// LLVM Dwarf parser is not thread-safe and we need to parse all DWARF up
@@ -612,7 +614,7 @@ 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, &LogMutex, &Out, Die]() mutable {
+ pool.async([this, CUI, &CU, &LogMutex, &Out, Die]() mutable {
std::string storage;
raw_string_ostream StrStream(storage);
OutputAggregator ThreadOut(Out.GetOS() ? &StrStream : nullptr);
@@ -623,6 +625,9 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
StrStream.flush();
Out << storage;
}
+ // Release the line table, once we're done.
+ DICtx.clearLineTableForUnit(CU.get());
+
Out.Merge(ThreadOut);
});
}
>From 6073460eb46725c9821b39091e67a6d6a172eb2f Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Tue, 30 Apr 2024 14:54:53 -0700
Subject: [PATCH 2/5] Finished without crashing
---
llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h | 2 ++
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp | 5 +++++
llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp | 5 ++++-
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
index 80c27aea89312..52a8730a46c2c 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
@@ -566,6 +566,8 @@ class DWARFUnit {
Error tryExtractDIEsIfNeeded(bool CUDieOnly);
+ void freeDIEs();
+
private:
/// Size in bytes of the .debug_info data associated with this compile unit.
size_t getDebugInfoSize() const {
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index bdd04b00f557b..4786208c79fc2 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -495,6 +495,11 @@ void DWARFUnit::extractDIEsIfNeeded(bool CUDieOnly) {
Context.getRecoverableErrorHandler()(std::move(e));
}
+void DWARFUnit::freeDIEs() {
+ if (DieArray.capacity())
+ std::vector<DWARFDebugInfoEntry>().swap(DieArray);
+}
+
Error DWARFUnit::tryExtractDIEsIfNeeded(bool CUDieOnly) {
if ((CUDieOnly && !DieArray.empty()) ||
DieArray.size() > 1)
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index 9376515e3712f..7204cefae1b84 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -625,8 +625,9 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
StrStream.flush();
Out << storage;
}
- // Release the line table, once we're done.
+ // Release the line table and DIEs once we're done.
DICtx.clearLineTableForUnit(CU.get());
+ CU->freeDIEs();
Out.Merge(ThreadOut);
});
@@ -634,6 +635,8 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
}
pool.wait();
}
+ for (const auto &CU : DICtx.compile_units())
+ CU->freeDIEs();
size_t FunctionsAddedCount = Gsym.getNumFunctionInfos() - NumBefore;
Out << "Loaded " << FunctionsAddedCount << " functions from DWARF.\n";
return Error::success();
>From 2cdc496d617009e3c063885686ea87798896f01f Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Wed, 1 May 2024 14:58:26 -0700
Subject: [PATCH 3/5] Added locking
---
llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h | 10 +++--
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp | 41 ++++++++++++++-----
llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp | 13 +++---
3 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
index 52a8730a46c2c..9614aab8bb9b5 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
@@ -22,6 +22,7 @@
#include "llvm/DebugInfo/DWARF/DWARFLocationExpression.h"
#include "llvm/DebugInfo/DWARF/DWARFUnitIndex.h"
#include "llvm/Support/DataExtractor.h"
+#include "llvm/Support/RWMutex.h"
#include <cassert>
#include <cstddef>
#include <cstdint>
@@ -257,6 +258,9 @@ class DWARFUnit {
std::shared_ptr<DWARFUnit> DWO;
+ mutable llvm::sys::RWMutex m_cu_die_array_mutex;
+ mutable llvm::sys::RWMutex m_all_die_array_mutex;
+
protected:
friend dwarf_linker::parallel::CompileUnit;
@@ -566,7 +570,8 @@ class DWARFUnit {
Error tryExtractDIEsIfNeeded(bool CUDieOnly);
- void freeDIEs();
+ /// clearDIEs - Clear parsed DIEs to keep memory usage low.
+ void clearDIEs(bool KeepCUDie);
private:
/// Size in bytes of the .debug_info data associated with this compile unit.
@@ -583,9 +588,6 @@ 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/DWARFUnit.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index 4786208c79fc2..b3fdb4e017803 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -495,26 +495,43 @@ void DWARFUnit::extractDIEsIfNeeded(bool CUDieOnly) {
Context.getRecoverableErrorHandler()(std::move(e));
}
-void DWARFUnit::freeDIEs() {
- if (DieArray.capacity())
- std::vector<DWARFDebugInfoEntry>().swap(DieArray);
-}
-
Error DWARFUnit::tryExtractDIEsIfNeeded(bool CUDieOnly) {
- if ((CUDieOnly && !DieArray.empty()) ||
- DieArray.size() > 1)
- return Error::success(); // Already parsed.
-
- bool HasCUDie = !DieArray.empty();
- extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
+ // read-lock:
+ {
+ llvm::sys::ScopedReader Lock(m_cu_die_array_mutex);
+ if ((CUDieOnly && !DieArray.empty()) ||
+ DieArray.size() > 1)
+ return Error::success(); // Already parsed.
+ }
+ bool HasCUDie = false;
+ // write-lock:
+ {
+ llvm::sys::ScopedWriter Lock(m_cu_die_array_mutex);
+ if ((CUDieOnly && !DieArray.empty()) ||
+ DieArray.size() > 1)
+ return Error::success(); // Already parsed.
+ HasCUDie = !DieArray.empty();
+ extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
+ }
+ // read-lock:
+{
+ llvm::sys::ScopedReader Lock(m_all_die_array_mutex);
if (DieArray.empty())
return Error::success();
// If CU DIE was just parsed, copy several attribute values from it.
if (HasCUDie)
return Error::success();
+}
+ // write-lock:
+llvm::sys::ScopedWriter Lock(m_all_die_array_mutex);
+ 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)))
@@ -663,6 +680,8 @@ void DWARFUnit::clearDIEs(bool KeepCUDie) {
// 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.
+ llvm::sys::ScopedWriter CULock(m_cu_die_array_mutex);
+ llvm::sys::ScopedWriter AllLock(m_all_die_array_mutex);
DieArray = (KeepCUDie && !DieArray.empty())
? std::vector<DWARFDebugInfoEntry>({DieArray[0]})
: std::vector<DWARFDebugInfoEntry>();
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index 7204cefae1b84..bd4c24f3ee28e 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -619,24 +619,27 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
raw_string_ostream StrStream(storage);
OutputAggregator ThreadOut(Out.GetOS() ? &StrStream : nullptr);
handleDie(ThreadOut, 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(false);
// Print ThreadLogStorage lines into an actual stream under a lock
std::lock_guard<std::mutex> guard(LogMutex);
if (Out.GetOS()) {
StrStream.flush();
Out << storage;
}
- // Release the line table and DIEs once we're done.
- DICtx.clearLineTableForUnit(CU.get());
- CU->freeDIEs();
-
Out.Merge(ThreadOut);
+
});
}
}
pool.wait();
}
+ // Now get rid of all the DIEs that may have been recreated
for (const auto &CU : DICtx.compile_units())
- CU->freeDIEs();
+ CU->clearDIEs(false);
size_t FunctionsAddedCount = Gsym.getNumFunctionInfos() - NumBefore;
Out << "Loaded " << FunctionsAddedCount << " functions from DWARF.\n";
return Error::success();
>From 5a40af3aaf9bd7385afe4cdf5f69f460352771a1 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Fri, 3 May 2024 09:26:24 -0700
Subject: [PATCH 4/5] Code formatting
---
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp | 28 ++++++++------------
llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp | 4 ++-
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index b3fdb4e017803..cc79d9ec7130c 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -496,42 +496,36 @@ void DWARFUnit::extractDIEsIfNeeded(bool CUDieOnly) {
}
Error DWARFUnit::tryExtractDIEsIfNeeded(bool CUDieOnly) {
- // read-lock:
{
llvm::sys::ScopedReader Lock(m_cu_die_array_mutex);
- if ((CUDieOnly && !DieArray.empty()) ||
- DieArray.size() > 1)
+ if ((CUDieOnly && !DieArray.empty()) || DieArray.size() > 1)
return Error::success(); // Already parsed.
}
bool HasCUDie = false;
- // write-lock:
{
llvm::sys::ScopedWriter Lock(m_cu_die_array_mutex);
- if ((CUDieOnly && !DieArray.empty()) ||
- DieArray.size() > 1)
+ if ((CUDieOnly && !DieArray.empty()) || DieArray.size() > 1)
return Error::success(); // Already parsed.
HasCUDie = !DieArray.empty();
extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
}
+ {
+ llvm::sys::ScopedReader Lock(m_all_die_array_mutex);
+ if (DieArray.empty())
+ return Error::success();
- // read-lock:
-{
- llvm::sys::ScopedReader Lock(m_all_die_array_mutex);
+ // If CU DIE was just parsed, copy several attribute values from it.
+ if (HasCUDie)
+ return Error::success();
+ }
+ llvm::sys::ScopedWriter Lock(m_all_die_array_mutex);
if (DieArray.empty())
return Error::success();
// If CU DIE was just parsed, copy several attribute values from it.
if (HasCUDie)
return Error::success();
-}
- // write-lock:
-llvm::sys::ScopedWriter Lock(m_all_die_array_mutex);
- 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)))
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index bd4c24f3ee28e..4a1ed91a7244f 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -589,6 +589,9 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
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(false);
}
} else {
// LLVM Dwarf parser is not thread-safe and we need to parse all DWARF up
@@ -631,7 +634,6 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
Out << storage;
}
Out.Merge(ThreadOut);
-
});
}
}
>From da79a50621bad3415fd3f8b28f8cf505978d6cc5 Mon Sep 17 00:00:00 2001
From: Kevin Frei <freik at meta.com>
Date: Mon, 13 May 2024 11:53:08 -0700
Subject: [PATCH 5/5] Updated from code review feedback
---
llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h | 4 ++--
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp | 15 +++++++--------
llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp | 6 +++---
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
index 9614aab8bb9b5..4e0601d1e042e 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
@@ -258,8 +258,8 @@ class DWARFUnit {
std::shared_ptr<DWARFUnit> DWO;
- mutable llvm::sys::RWMutex m_cu_die_array_mutex;
- mutable llvm::sys::RWMutex m_all_die_array_mutex;
+ mutable llvm::sys::RWMutex CUDieArrayMutex;
+ mutable llvm::sys::RWMutex AllDieArrayMutex;
protected:
friend dwarf_linker::parallel::CompileUnit;
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index cc79d9ec7130c..bd9c49bcd6b90 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -497,35 +497,34 @@ void DWARFUnit::extractDIEsIfNeeded(bool CUDieOnly) {
Error DWARFUnit::tryExtractDIEsIfNeeded(bool CUDieOnly) {
{
- llvm::sys::ScopedReader Lock(m_cu_die_array_mutex);
+ llvm::sys::ScopedReader Lock(CUDieArrayMutex);
if ((CUDieOnly && !DieArray.empty()) || DieArray.size() > 1)
return Error::success(); // Already parsed.
}
bool HasCUDie = false;
{
- llvm::sys::ScopedWriter Lock(m_cu_die_array_mutex);
+ llvm::sys::ScopedWriter Lock(CUDieArrayMutex);
if ((CUDieOnly && !DieArray.empty()) || DieArray.size() > 1)
return Error::success(); // Already parsed.
HasCUDie = !DieArray.empty();
extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
}
{
- llvm::sys::ScopedReader Lock(m_all_die_array_mutex);
+ llvm::sys::ScopedReader Lock(AllDieArrayMutex);
if (DieArray.empty())
return Error::success();
- // If CU DIE was just parsed, copy several attribute values from it.
if (HasCUDie)
return Error::success();
}
- llvm::sys::ScopedWriter Lock(m_all_die_array_mutex);
+ llvm::sys::ScopedWriter Lock(AllDieArrayMutex);
if (DieArray.empty())
return Error::success();
- // If CU DIE was just parsed, copy several attribute values from it.
if (HasCUDie)
return Error::success();
+ // If CU DIE was just parsed, copy several attribute values from it.
DWARFDie UnitDie(this, &DieArray[0]);
if (std::optional<uint64_t> DWOId =
toUnsigned(UnitDie.find(DW_AT_GNU_dwo_id)))
@@ -674,8 +673,8 @@ void DWARFUnit::clearDIEs(bool KeepCUDie) {
// 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.
- llvm::sys::ScopedWriter CULock(m_cu_die_array_mutex);
- llvm::sys::ScopedWriter AllLock(m_all_die_array_mutex);
+ llvm::sys::ScopedWriter CULock(CUDieArrayMutex);
+ llvm::sys::ScopedWriter AllLock(AllDieArrayMutex);
DieArray = (KeepCUDie && !DieArray.empty())
? std::vector<DWARFDebugInfoEntry>({DieArray[0]})
: std::vector<DWARFDebugInfoEntry>();
diff --git a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
index 4a1ed91a7244f..e1b30648b6a77 100644
--- a/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
+++ b/llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp
@@ -591,7 +591,7 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
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(false);
+ CU->clearDIEs(/*KeepCUDie=*/false);
}
} else {
// LLVM Dwarf parser is not thread-safe and we need to parse all DWARF up
@@ -626,7 +626,7 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
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(false);
+ CU->clearDIEs(/*KeepCUDie=*/false);
// Print ThreadLogStorage lines into an actual stream under a lock
std::lock_guard<std::mutex> guard(LogMutex);
if (Out.GetOS()) {
@@ -641,7 +641,7 @@ Error DwarfTransformer::convert(uint32_t NumThreads, OutputAggregator &Out) {
}
// Now get rid of all the DIEs that may have been recreated
for (const auto &CU : DICtx.compile_units())
- CU->clearDIEs(false);
+ CU->clearDIEs(/*KeepCUDie=*/false);
size_t FunctionsAddedCount = Gsym.getNumFunctionInfos() - NumBefore;
Out << "Loaded " << FunctionsAddedCount << " functions from DWARF.\n";
return Error::success();
More information about the llvm-commits
mailing list