Skip to content

Commit 1d9c7c4

Browse files
committed
Increase performance of llvm-gsymutil by up to 200%.
llvm-gsymutil was maintaining an address ranges collection behind a mutex and having the multi-threaded code access this and hold the mutex was causing slowdown when converting DWARF to GSYM. This patch does the following: - removes the "Ranges" variable from the GsymCreator and any functions and places that used it - clients don't try to detect if a function has been added for an address range, we now remove any inferior copies of information in the GsymCreator::finalize() routine as was done before, we just have more items to remove, though performance is greator due to less mutex thread locking - after I started adding all of the inferior funtion info objects the previous patch that tried to remove infrior debug info had bugs in it, so I replace the removeIfBinary() function in GsymCreator with a more efficient and easier to debug way to do things which copies items from the GsymCreator::Funcs into a new vector of FunctionInfo objects and then replaces GsymCreator::Funcs at the end. - Sorting of FunctionInfo objects has been modified to also compare InlineInfo objects. We found cases where LTO was ruining inline function address ranges and we ended up with a variety of FunctionInfo objects for the same range that had varying amounts of valid debug info. This patch now ensure that two function info objects with different inline info for the same function address range, the best one will be picked to ensure the greatest fidelity. - If we detect that a DW_TAG_subprogram has inline functions and after parsing it, we don't end up with any valid inline information, we set the optional to std::nullopt to avoid emitting empty inline information and wasting space. My tests show a 200% perf increase on M1 macs and a 100% performance increase on linux machines for the same complex large DWARF input binary. Differential Revision: https://reviews.llvm.org/D156773
1 parent b21c24c commit 1d9c7c4

File tree

11 files changed

+771
-160
lines changed

11 files changed

+771
-160
lines changed

llvm/include/llvm/DebugInfo/GSYM/FunctionInfo.h

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -202,21 +202,27 @@ inline bool operator==(const FunctionInfo &LHS, const FunctionInfo &RHS) {
202202
inline bool operator!=(const FunctionInfo &LHS, const FunctionInfo &RHS) {
203203
return !(LHS == RHS);
204204
}
205-
/// This sorting will order things consistently by address range first, but then
206-
/// followed by inlining being valid and line tables. We might end up with a
207-
/// FunctionInfo from debug info that will have the same range as one from the
208-
/// symbol table, but we want to quickly be able to sort and use the best version
209-
/// when creating the final GSYM file.
205+
/// This sorting will order things consistently by address range first, but
206+
/// then followed by increasing levels of debug info like inline information
207+
/// and line tables. We might end up with a FunctionInfo from debug info that
208+
/// will have the same range as one from the symbol table, but we want to
209+
/// quickly be able to sort and use the best version when creating the final
210+
/// GSYM file. This function compares the inline information as we have seen
211+
/// cases where LTO can generate a wide array of differing inline information,
212+
/// mostly due to messing up the address ranges for inlined functions, so the
213+
/// inline information with the most entries will appeear last. If the inline
214+
/// information match, either by both function infos not having any or both
215+
/// being exactly the same, we will then compare line tables. Comparing line
216+
/// tables allows the entry with the most line entries to appear last. This
217+
/// ensures we are able to save the FunctionInfo with the most debug info into
218+
/// the GSYM file.
210219
inline bool operator<(const FunctionInfo &LHS, const FunctionInfo &RHS) {
211220
// First sort by address range
212221
if (LHS.Range != RHS.Range)
213222
return LHS.Range < RHS.Range;
214-
215-
// Then sort by inline
216-
if (LHS.Inline.has_value() != RHS.Inline.has_value())
217-
return RHS.Inline.has_value();
218-
219-
return LHS.OptLineTable < RHS.OptLineTable;
223+
if (LHS.Inline == RHS.Inline)
224+
return LHS.OptLineTable < RHS.OptLineTable;
225+
return LHS.Inline < RHS.Inline;
220226
}
221227

222228
raw_ostream &operator<<(raw_ostream &OS, const FunctionInfo &R);

llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ class GsymCreator {
142142
std::vector<llvm::gsym::FileEntry> Files;
143143
std::vector<uint8_t> UUID;
144144
std::optional<AddressRanges> ValidTextRanges;
145-
AddressRanges Ranges;
146145
std::optional<uint64_t> BaseAddress;
146+
bool IsSegment = false;
147147
bool Finalized = false;
148148
bool Quiet;
149149

@@ -282,6 +282,15 @@ class GsymCreator {
282282
llvm::support::endianness ByteOrder,
283283
uint64_t SegmentSize) const;
284284

285+
/// Let this creator know that this is a segment of another GsymCreator.
286+
///
287+
/// When we have a segment, we know that function infos will be added in
288+
/// ascending address range order without having to be finalized. We also
289+
/// don't need to sort and unique entries during the finalize function call.
290+
void setIsSegment() {
291+
IsSegment = true;
292+
}
293+
285294
public:
286295
GsymCreator(bool Quiet = false);
287296

@@ -379,17 +388,6 @@ class GsymCreator {
379388
/// object.
380389
size_t getNumFunctionInfos() const;
381390

382-
/// Check if an address has already been added as a function info.
383-
///
384-
/// FunctionInfo data can come from many sources: debug info, symbol tables,
385-
/// exception information, and more. Symbol tables should be added after
386-
/// debug info and can use this function to see if a symbol's start address
387-
/// has already been added to the GsymReader. Calling this before adding
388-
/// a function info from a source other than debug info avoids clients adding
389-
/// many redundant FunctionInfo objects from many sources only for them to be
390-
/// removed during the finalize() call.
391-
bool hasFunctionInfoForAddress(uint64_t Addr) const;
392-
393391
/// Set valid .text address ranges that all functions must be contained in.
394392
void SetValidTextRanges(AddressRanges &TextRanges) {
395393
ValidTextRanges = TextRanges;

llvm/include/llvm/DebugInfo/GSYM/InlineInfo.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,17 @@ struct InlineInfo {
164164
/// \returns An error object that indicates success or failure or the
165165
/// encoding process.
166166
llvm::Error encode(FileWriter &O, uint64_t BaseAddr) const;
167+
168+
/// Compare InlineInfo objects.
169+
///
170+
/// When comparing InlineInfo objects the item with the most inline functions
171+
/// wins. If we have two FunctionInfo objects that both have the same address
172+
/// range and both have valid InlineInfo objects, we want the one with the
173+
/// most inline functions to win so we save the most information possible
174+
/// to the GSYM file. We have seen cases where LTO messes up the inline
175+
/// function information for the same address range, so this helps ensure we
176+
/// get the most descriptive information we can for an address range.
177+
bool operator<(const InlineInfo &RHS) const;
167178
};
168179

169180
inline bool operator==(const InlineInfo &LHS, const InlineInfo &RHS) {

llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,6 @@ static void parseInlineInfo(GsymCreator &Gsym, raw_ostream &Log, CUInfo &CUI,
215215
if (Tag == dwarf::DW_TAG_inlined_subroutine) {
216216
// create new InlineInfo and append to parent.children
217217
InlineInfo II;
218-
DWARFAddressRange FuncRange =
219-
DWARFAddressRange(FI.startAddress(), FI.endAddress());
220218
Expected<DWARFAddressRangesVector> RangesOrError = Die.getAddressRanges();
221219
if (RangesOrError) {
222220
for (const DWARFAddressRange &Range : RangesOrError.get()) {
@@ -421,6 +419,23 @@ void DwarfTransformer::handleDie(raw_ostream &OS, CUInfo &CUI, DWARFDie Die) {
421419
FI.Inline->Name = *NameIndex;
422420
FI.Inline->Ranges.insert(FI.Range);
423421
parseInlineInfo(Gsym, OS, CUI, Die, 0, FI, *FI.Inline);
422+
// Make sure we at least got some valid inline info other than just
423+
// the top level function. If we didn't then remove the inline info
424+
// from the function info. We have seen cases where LTO tries to modify
425+
// the DWARF for functions and it messes up the address ranges for
426+
// the inline functions so it is no longer valid.
427+
//
428+
// By checking if there are any valid children on the top level inline
429+
// information object, we will know if we got anything valid from the
430+
// debug info.
431+
if (FI.Inline->Children.empty()) {
432+
if (!Gsym.isQuiet()) {
433+
OS << "warning: DIE contains inline function information that has "
434+
"no valid ranges, removing inline information:\n";
435+
Die.dump(OS, 0, DIDumpOptions::getForSingleDIE());
436+
}
437+
FI.Inline = std::nullopt;
438+
}
424439
}
425440
Gsym.addFunctionInfo(std::move(FI));
426441
}

0 commit comments

Comments
 (0)