[llvm] b5c862e - [JITLink] Store Sections in a DenseMap with the section name as key.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 17 20:22:00 PDT 2023
Author: Lang Hames
Date: 2023-03-17T20:21:54-07:00
New Revision: b5c862e15caf4d8aa34bbc6ee25af8da9a9405a4
URL: https://github.com/llvm/llvm-project/commit/b5c862e15caf4d8aa34bbc6ee25af8da9a9405a4
DIFF: https://github.com/llvm/llvm-project/commit/b5c862e15caf4d8aa34bbc6ee25af8da9a9405a4.diff
LOG: [JITLink] Store Sections in a DenseMap with the section name as key.
This speeds up section lookup by name.
This change was motivated by poor performance of a testcase while trying to fix
the NoAlloc lifetime patch that was originally landed as 2cc64df0bd6. The
NoAlloc lifetime patch causes ELF non-SHF_ALLOC sections to be given a JITLink
Section (previously they were skipped), and the
llvm/test/ExecutionEngine/JITLink/X86/ELF_shndex.s testcase creates > 64k
non-SHF_ALLOC sections, each of which now needs to be checked to ensure that its
name does not clash. Moving to a DenseMap allows us to implement this check
efficiently.
Added:
Modified:
llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_associative_dead_strip.test
Removed:
################################################################################
diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
index 1123b35a4cd76..47adf0156965d 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
@@ -826,7 +826,7 @@ class SectionRange {
class LinkGraph {
private:
- using SectionList = std::vector<std::unique_ptr<Section>>;
+ using SectionMap = DenseMap<StringRef, std::unique_ptr<Section>>;
using ExternalSymbolSet = DenseSet<Symbol *>;
using BlockSet = DenseSet<Block *>;
@@ -865,7 +865,7 @@ class LinkGraph {
}
static iterator_range<Section::const_block_iterator>
- getSectionConstBlocks(Section &S) {
+ getSectionConstBlocks(const Section &S) {
return S.blocks();
}
@@ -875,15 +875,27 @@ class LinkGraph {
}
static iterator_range<Section::const_symbol_iterator>
- getSectionConstSymbols(Section &S) {
+ getSectionConstSymbols(const Section &S) {
return S.symbols();
}
+ struct GetSectionMapEntryValue {
+ Section &operator()(SectionMap::value_type &KV) const { return *KV.second; }
+ };
+
+ struct GetSectionMapEntryConstValue {
+ const Section &operator()(const SectionMap::value_type &KV) const {
+ return *KV.second;
+ }
+ };
+
public:
using external_symbol_iterator = ExternalSymbolSet::iterator;
- using section_iterator = pointee_iterator<SectionList::iterator>;
- using const_section_iterator = pointee_iterator<SectionList::const_iterator>;
+ using section_iterator =
+ mapped_iterator<SectionMap::iterator, GetSectionMapEntryValue>;
+ using const_section_iterator =
+ mapped_iterator<SectionMap::const_iterator, GetSectionMapEntryConstValue>;
template <typename OuterItrT, typename InnerItrT, typename T,
iterator_range<InnerItrT> getInnerRange(
@@ -933,18 +945,17 @@ class LinkGraph {
};
using defined_symbol_iterator =
- nested_collection_iterator<const_section_iterator,
- Section::symbol_iterator, Symbol *,
- getSectionSymbols>;
+ nested_collection_iterator<section_iterator, Section::symbol_iterator,
+ Symbol *, getSectionSymbols>;
using const_defined_symbol_iterator =
nested_collection_iterator<const_section_iterator,
Section::const_symbol_iterator, const Symbol *,
getSectionConstSymbols>;
- using block_iterator = nested_collection_iterator<const_section_iterator,
- Section::block_iterator,
- Block *, getSectionBlocks>;
+ using block_iterator =
+ nested_collection_iterator<section_iterator, Section::block_iterator,
+ Block *, getSectionBlocks>;
using const_block_iterator =
nested_collection_iterator<const_section_iterator,
@@ -1041,14 +1052,9 @@ class LinkGraph {
/// Create a section with the given name, protection flags, and alignment.
Section &createSection(StringRef Name, orc::MemProt Prot) {
- assert(llvm::none_of(Sections,
- [&](std::unique_ptr<Section> &Sec) {
- return Sec->getName() == Name;
- }) &&
- "Duplicate section name");
+ assert(!Sections.count(Name) && "Duplicate section name");
std::unique_ptr<Section> Sec(new Section(Name, Prot, Sections.size()));
- Sections.push_back(std::move(Sec));
- return *Sections.back();
+ return *Sections.insert(std::make_pair(Name, std::move(Sec))).first->second;
}
/// Create a content block.
@@ -1207,29 +1213,39 @@ class LinkGraph {
}
iterator_range<section_iterator> sections() {
- return make_range(section_iterator(Sections.begin()),
- section_iterator(Sections.end()));
+ return make_range(
+ section_iterator(Sections.begin(), GetSectionMapEntryValue()),
+ section_iterator(Sections.end(), GetSectionMapEntryValue()));
+ }
+
+ iterator_range<const_section_iterator> sections() const {
+ return make_range(
+ const_section_iterator(Sections.begin(),
+ GetSectionMapEntryConstValue()),
+ const_section_iterator(Sections.end(), GetSectionMapEntryConstValue()));
}
- SectionList::size_type sections_size() const { return Sections.size(); }
+ size_t sections_size() const { return Sections.size(); }
/// Returns the section with the given name if it exists, otherwise returns
/// null.
Section *findSectionByName(StringRef Name) {
- for (auto &S : sections())
- if (S.getName() == Name)
- return &S;
- return nullptr;
+ auto I = Sections.find(Name);
+ if (I == Sections.end())
+ return nullptr;
+ return I->second.get();
}
iterator_range<block_iterator> blocks() {
- return make_range(block_iterator(Sections.begin(), Sections.end()),
- block_iterator(Sections.end(), Sections.end()));
+ auto Secs = sections();
+ return make_range(block_iterator(Secs.begin(), Secs.end()),
+ block_iterator(Secs.end(), Secs.end()));
}
iterator_range<const_block_iterator> blocks() const {
- return make_range(const_block_iterator(Sections.begin(), Sections.end()),
- const_block_iterator(Sections.end(), Sections.end()));
+ auto Secs = sections();
+ return make_range(const_block_iterator(Secs.begin(), Secs.end()),
+ const_block_iterator(Secs.end(), Secs.end()));
}
iterator_range<external_symbol_iterator> external_symbols() {
@@ -1241,14 +1257,15 @@ class LinkGraph {
}
iterator_range<defined_symbol_iterator> defined_symbols() {
- return make_range(defined_symbol_iterator(Sections.begin(), Sections.end()),
- defined_symbol_iterator(Sections.end(), Sections.end()));
+ auto Secs = sections();
+ return make_range(defined_symbol_iterator(Secs.begin(), Secs.end()),
+ defined_symbol_iterator(Secs.end(), Secs.end()));
}
iterator_range<const_defined_symbol_iterator> defined_symbols() const {
- return make_range(
- const_defined_symbol_iterator(Sections.begin(), Sections.end()),
- const_defined_symbol_iterator(Sections.end(), Sections.end()));
+ auto Secs = sections();
+ return make_range(const_defined_symbol_iterator(Secs.begin(), Secs.end()),
+ const_defined_symbol_iterator(Secs.end(), Secs.end()));
}
/// Make the given symbol external (must not already be external).
@@ -1447,11 +1464,10 @@ class LinkGraph {
/// Remove a section. The section reference is defunct after calling this
/// function and should no longer be used.
void removeSection(Section &Sec) {
- auto I = llvm::find_if(Sections, [&Sec](const std::unique_ptr<Section> &S) {
- return S.get() == &Sec;
- });
- assert(I != Sections.end() && "Section does not appear in this graph");
- Sections.erase(I);
+ assert(Sections.count(Sec.getName()) && "Section not found");
+ assert(Sections.find(Sec.getName())->second.get() == &Sec &&
+ "Section map entry invalid");
+ Sections.erase(Sec.getName());
}
/// Accessor for the AllocActions object for this graph. This can be used to
@@ -1474,7 +1490,7 @@ class LinkGraph {
unsigned PointerSize;
support::endianness Endianness;
GetEdgeKindNameFunction GetEdgeKindName = nullptr;
- SectionList Sections;
+ DenseMap<StringRef, std::unique_ptr<Section>> Sections;
ExternalSymbolSet ExternalSymbols;
ExternalSymbolSet AbsoluteSymbols;
orc::shared::AllocActions AAs;
diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_associative_dead_strip.test b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_associative_dead_strip.test
index dd2e4bc7c9055..ad6b5a06abfca 100644
--- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_associative_dead_strip.test
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_associative_dead_strip.test
@@ -6,9 +6,10 @@
# Check a comdat child block connected by associative selection type is dead strip when
# parent block is dead.
#
-# CHECK: section parent:
+# CHECK: Link graph
+# CHECK-DAG: section parent:
# CHECK-EMPTY:
-# CHECK-NEXT: section child:
+# CHECK-DAG: section child:
# CHECK-EMPTY:
--- !COFF
More information about the llvm-commits
mailing list