[lld] 03909c4 - [ELF] Remove StringRefZ

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2022-01-19T20:09:41-08:00
New Revision: 03909c4400b585c1991e59359b4155d72a4f19ca

URL: https://github.com/llvm/llvm-project/commit/03909c4400b585c1991e59359b4155d72a4f19ca
DIFF: https://github.com/llvm/llvm-project/commit/03909c4400b585c1991e59359b4155d72a4f19ca.diff

LOG: [ELF] Remove StringRefZ

StringRefZ does not improve performance. Non-local symbols always have eagerly
computed nameSize. Most local symbols's lengths will be updated in either:

* shouldKeepInSymtab
* SymbolTableBaseSection::addSymbol

Its benefit is offsetted by strlen in every call site (sums up to 5KiB code in a
release x86-64 build), so using StringRefZ may be slower.

In a -s link (uncommon) there is minor speedup, like ~0.3% for clang and chrome.

Reviewed By: alexander-shaposhnikov

Differential Revision: https://reviews.llvm.org/D117644

Added: 
    

Modified: 
    lld/ELF/InputFiles.cpp
    lld/ELF/Symbols.h

Removed: 
    


################################################################################
diff  --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 64e64ff63b506..ca16a64e50836 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -1067,7 +1067,7 @@ template <class ELFT> void ObjFile<ELFT>::initializeSymbols() {
       sourceFile = CHECK(eSym.getName(stringTable), this);
     if (LLVM_UNLIKELY(stringTable.size() <= eSym.st_name))
       fatal(toString(this) + ": invalid symbol name offset");
-    StringRefZ name = stringTable.data() + eSym.st_name;
+    StringRef name(stringTable.data() + eSym.st_name);
 
     symbols[i] = reinterpret_cast<Symbol *>(locals + i);
     if (eSym.st_shndx == SHN_UNDEF || sec == &InputSection::discarded)

diff  --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 2198bc29d19fe..cd5d4b280e793 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -42,20 +42,6 @@ class SharedSymbol;
 class Symbol;
 class Undefined;
 
-// This is a StringRef-like container that doesn't run strlen().
-//
-// ELF string tables contain a lot of null-terminated strings. Most of them
-// are not necessary for the linker because they are names of local symbols,
-// and the linker doesn't use local symbol names for name resolution. So, we
-// use this class to represents strings read from string tables.
-struct StringRefZ {
-  StringRefZ(const char *s) : data(s), size(-1) {}
-  StringRefZ(StringRef s) : data(s.data()), size(s.size()) {}
-
-  const char *data;
-  const uint32_t size;
-};
-
 // Some index properties of a symbol are stored separately in this auxiliary
 // struct to decrease sizeof(SymbolUnion) in the majority of cases.
 struct SymbolAux {
@@ -87,7 +73,8 @@ class Symbol {
 
 protected:
   const char *nameData;
-  mutable uint32_t nameSize;
+  // 32-bit size saves space.
+  uint32_t nameSize;
 
 public:
   // A symAux index used to access GOT/PLT entry indexes. This is allocated in
@@ -179,11 +166,7 @@ class Symbol {
   // all input files have been added.
   bool isUndefWeak() const { return isWeak() && isUndefined(); }
 
-  StringRef getName() const {
-    if (nameSize == (uint32_t)-1)
-      nameSize = strlen(nameData);
-    return {nameData, nameSize};
-  }
+  StringRef getName() const { return {nameData, nameSize}; }
 
   void setName(StringRef s) {
     nameData = s.data();
@@ -196,10 +179,7 @@ class Symbol {
   //
   // For @@, the name has been truncated by insert(). For @, the name has been
   // truncated by Symbol::parseSymbolVersion().
-  const char *getVersionSuffix() const {
-    (void)getName();
-    return nameData + nameSize;
-  }
+  const char *getVersionSuffix() const { return nameData + nameSize; }
 
   uint32_t getGotIdx() const {
     return auxIdx == uint32_t(-1) ? uint32_t(-1) : symAux[auxIdx].gotIdx;
@@ -266,10 +246,11 @@ class Symbol {
   inline size_t getSymbolSize() const;
 
 protected:
-  Symbol(Kind k, InputFile *file, StringRefZ name, uint8_t binding,
+  Symbol(Kind k, InputFile *file, StringRef name, uint8_t binding,
          uint8_t stOther, uint8_t type)
-      : file(file), nameData(name.data), nameSize(name.size), binding(binding),
-        type(type), stOther(stOther), symbolKind(k), visibility(stOther & 3),
+      : file(file), nameData(name.data()), nameSize(name.size()),
+        binding(binding), type(type), stOther(stOther), symbolKind(k),
+        visibility(stOther & 3),
         isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind),
         exportDynamic(isExportDynamic(k, visibility)), inDynamicList(false),
         canInline(false), referenced(false), traced(false),
@@ -348,7 +329,7 @@ class Symbol {
 // Represents a symbol that is defined in the current output file.
 class Defined : public Symbol {
 public:
-  Defined(InputFile *file, StringRefZ name, uint8_t binding, uint8_t stOther,
+  Defined(InputFile *file, StringRef name, uint8_t binding, uint8_t stOther,
           uint8_t type, uint64_t value, uint64_t size, SectionBase *section)
       : Symbol(DefinedKind, file, name, binding, stOther, type), value(value),
         size(size), section(section) {}
@@ -383,7 +364,7 @@ class Defined : public Symbol {
 // section. (Therefore, the later passes don't see any CommonSymbols.)
 class CommonSymbol : public Symbol {
 public:
-  CommonSymbol(InputFile *file, StringRefZ name, uint8_t binding,
+  CommonSymbol(InputFile *file, StringRef name, uint8_t binding,
                uint8_t stOther, uint8_t type, uint64_t alignment, uint64_t size)
       : Symbol(CommonKind, file, name, binding, stOther, type),
         alignment(alignment), size(size) {}
@@ -396,7 +377,7 @@ class CommonSymbol : public Symbol {
 
 class Undefined : public Symbol {
 public:
-  Undefined(InputFile *file, StringRefZ name, uint8_t binding, uint8_t stOther,
+  Undefined(InputFile *file, StringRef name, uint8_t binding, uint8_t stOther,
             uint8_t type, uint32_t discardedSecIdx = 0)
       : Symbol(UndefinedKind, file, name, binding, stOther, type),
         discardedSecIdx(discardedSecIdx) {}