[ELF] Remove StringRefZ

[MaskRay]

Summary

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

• shouldKeepInSymtab
• SymbolTableBaseSection::addSymbol

Its benefit is offseted by strlen in every call site (sums 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.

[alexander-shaposhnikov]

lld/ELF/Symbols.h

250

it looks like in both cases this code tries to "unpack" a StringRef into nameData and nameSize
(here and inside setName) - maybe just store StringRef itself ?

[MaskRay]

lld/ELF/Symbols.h

250

The idea is to use 32-bit nameSize to save some space...

[arichardson]

lld/ELF/Symbols.h

250

It might be worth adding that as comment above the two fields since I had the same thought.

[MaskRay]

Add a comment

lld/ELF/InputFiles.cpp

if (LLVM_UNLIKELY(eSym.getBinding() != STB_LOCAL))
             ") found at index < .symtab's sh_info (" + Twine(end) + ")");
 
     InputSectionBase *sec = sections[secIdx];
     uint8_t type = eSym.getType();
     if (type == STT_FILE)
       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)
       new (symbols[i]) Undefined(this, name, STB_LOCAL, eSym.st_other, type,
                                  /*discardedSecIdx=*/secIdx);
     else
       new (symbols[i]) Defined(this, name, STB_LOCAL, eSym.st_other, type,
                                eSym.st_value, eSym.st_size, sec);

lld/ELF/Symbols.h

 class Defined;
 class InputFile;
 class LazyArchive;
 class LazyObject;
 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 {
   uint32_t gotIdx = -1;
   uint32_t pltIdx = -1;
   uint32_t tlsDescIdx = -1;
   uint32_t tlsGdIdx = -1;
 };
public:
 
   Kind kind() const { return static_cast<Kind>(symbolKind); }
 
   // The file from which this symbol was created.
   InputFile *file;
 
 protected:
   const char *nameData;
-  mutable uint32_t nameSize;
+  uint32_t nameSize;
 
 public:
   // A symAux index used to access GOT/PLT entry indexes. This is allocated in
   // postScanRelocations().
   uint32_t auxIdx = -1;
   uint32_t dynsymIndex = 0;
 
   // This field is a index to the symbol's version definition.
public:
   bool isLazy() const {
     return symbolKind == LazyArchiveKind || symbolKind == LazyObjectKind;
   }
 
   // True if this is an undefined weak symbol. This only works once
   // 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();
     nameSize = s.size();
   }
 
   void parseSymbolVersion();
 
   // Get the NUL-terminated version suffix ("", "@...", or "@@...").
   //
   // 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;
   }
   uint32_t getPltIdx() const {
     return auxIdx == uint32_t(-1) ? uint32_t(-1) : symAux[auxIdx].pltIdx;
   }
   uint32_t getTlsDescIdx() const {
private:
   template <class LazyT> void resolveLazy(const LazyT &other);
   void resolveShared(const SharedSymbol &other);
 
   int compare(const Symbol *other) const;
 
   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),
         hasVersionSuffix(false), isInIplt(false), gotInIgot(false),
         isPreemptible(false), used(!config->gcSections), folded(false),
         needsTocRestore(false), scriptDefined(false), needsCopy(false),
         needsGot(false), needsPlt(false), needsTlsDesc(false),
         needsTlsGd(false), needsTlsGdToIe(false), needsTlsLd(false),
public:
   bool isGnuIFunc() const { return type == llvm::ELF::STT_GNU_IFUNC; }
   bool isObject() const { return type == llvm::ELF::STT_OBJECT; }
   bool isFile() const { return type == llvm::ELF::STT_FILE; }
 };
 
 // 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) {}
 
   static bool classof(const Symbol *s) { return s->isDefined(); }
 
   uint64_t value;
   uint64_t size;

 // compiler create a regular defined symbol by -fno-common).
 //
 // The linker allows common symbols to be replaced by regular defined
 // symbols. If there are remaining common symbols after name resolution is
 // complete, they are converted to regular defined symbols in a .bss
 // 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) {}
 
   static bool classof(const Symbol *s) { return s->isCommon(); }
 
   uint32_t alignment;
   uint64_t size;
 };
 
 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) {}
 
   static bool classof(const Symbol *s) { return s->kind() == UndefinedKind; }
 
   // The section index if in a discarded section, 0 otherwise.
   uint32_t discardedSecIdx;