[PATCH] D116390: [ELF][LTO] Cache symbol table of lazy BitcodeFile

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added a reviewer: tejohnson.
Herald added subscribers: arichardson, inglorion, emaste.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Similar to D62188 <https://reviews.llvm.org/D62188>: a BitcodeFile's symbol table may be iterated twice, once in
--start-lib (lazy) state, and once in the non-lazy state. This patch
make `parseLazy` save `symbols[i]` so that the non-lazy state does not need to
re-insert to the global symbol table. Avoiding a redundant `saver.save` likely
results in some memory saving.

`Maximum resident set size (kbytes)` for a large --thinlto-index-only link:

- without the patch: 10164000
- with the patch: 10095716 (0.6% decrease)

Note: we can remove `saver.save` if `BitcodeCompiler::add` does not transfer the ownership
of `f.obj` in `checkError(ltoObj->add(std::move(f.obj), resols));`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116390

Files:
  lld/ELF/InputFiles.cpp


Index: lld/ELF/InputFiles.cpp
===================================================================
--- lld/ELF/InputFiles.cpp
+++ lld/ELF/InputFiles.cpp
@@ -1680,34 +1680,42 @@
 }
 
 template <class ELFT>
-static Symbol *createBitcodeSymbol(const std::vector<bool> &keptComdats,
-                                   const lto::InputFile::Symbol &objSym,
-                                   BitcodeFile &f) {
-  StringRef name = saver.save(objSym.getName());
+static void
+createBitcodeSymbol(Symbol *&sym, const std::vector<bool> &keptComdats,
+                    const lto::InputFile::Symbol &objSym, BitcodeFile &f) {
   uint8_t binding = objSym.isWeak() ? STB_WEAK : STB_GLOBAL;
   uint8_t type = objSym.isTLS() ? STT_TLS : STT_NOTYPE;
   uint8_t visibility = mapVisibility(objSym.getVisibility());
   bool canOmitFromDynSym = objSym.canBeOmittedFromSymbolTable();
 
+  StringRef name;
+  if (sym) {
+    name = sym->getName();
+  } else {
+    name = saver.save(objSym.getName());
+    sym = symtab->insert(name);
+  }
+
   int c = objSym.getComdatIndex();
   if (objSym.isUndefined() || (c != -1 && !keptComdats[c])) {
     Undefined newSym(&f, name, binding, visibility, type);
     if (canOmitFromDynSym)
       newSym.exportDynamic = false;
-    Symbol *ret = symtab->addSymbol(newSym);
-    ret->referenced = true;
-    return ret;
+    sym->resolve(newSym);
+    sym->referenced = true;
+    return;
   }
 
-  if (objSym.isCommon())
-    return symtab->addSymbol(
-        CommonSymbol{&f, name, binding, visibility, STT_OBJECT,
-                     objSym.getCommonAlignment(), objSym.getCommonSize()});
-
-  Defined newSym(&f, name, binding, visibility, type, 0, 0, nullptr);
-  if (canOmitFromDynSym)
-    newSym.exportDynamic = false;
-  return symtab->addSymbol(newSym);
+  if (objSym.isCommon()) {
+    sym->resolve(CommonSymbol{&f, name, binding, visibility, STT_OBJECT,
+                              objSym.getCommonAlignment(),
+                              objSym.getCommonSize()});
+  } else {
+    Defined newSym(&f, name, binding, visibility, type, 0, 0, nullptr);
+    if (canOmitFromDynSym)
+      newSym.exportDynamic = false;
+    sym->resolve(newSym);
+  }
 }
 
 template <class ELFT> void BitcodeFile::parse() {
@@ -1719,10 +1727,11 @@
             .second);
   }
 
-  symbols.assign(obj->symbols().size(), nullptr);
-  for (auto it : llvm::enumerate(obj->symbols()))
-    symbols[it.index()] =
-        createBitcodeSymbol<ELFT>(keptComdats, it.value(), *this);
+  symbols.resize(obj->symbols().size());
+  for (auto it : llvm::enumerate(obj->symbols())) {
+    Symbol *&sym = symbols[it.index()];
+    createBitcodeSymbol<ELFT>(sym, keptComdats, it.value(), *this);
+  }
 
   for (auto l : obj->getDependentLibraries())
     addDependentLibrary(l, this);
@@ -1730,9 +1739,11 @@
 
 void BitcodeFile::parseLazy() {
   SymbolTable &symtab = *elf::symtab;
-  for (const lto::InputFile::Symbol &sym : obj->symbols())
-    if (!sym.isUndefined())
-      symtab.addSymbol(LazyObject{*this, saver.save(sym.getName())});
+  symbols.resize(obj->symbols().size());
+  for (auto it : llvm::enumerate(obj->symbols()))
+    if (!it.value().isUndefined())
+      symbols[it.index()] =
+          symtab.addSymbol(LazyObject{*this, saver.save(it.value().getName())});
 }
 
 void BinaryFile::parse() {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116390.396601.patch
Type: text/x-patch
Size: 3318 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211230/fd08cb7f/attachment.bin>