Pack symbols a bit more

[• espindola]

Summary

Before this patch:

Symbol 56
Defined 80
Undefined 56
SharedSymbol 88
LazyArchive 72
LazyObject 56

With this patch

Symbol 48
Defined 72
Undefined 48
SharedSymbol 80
LazyArchive 64
LazyObject 48

The result is that peak allocation when linking chromium (according to heaptrack) goes from 578 to 568 MB.

[emaste]

Nice :)

[ruiu]

Maybe we should remove StringRefZ class and use a char pointer instead? The point of having StringRefZ class is that an instance of StringRefZ is automatically converted to StringRefZ when needed, but I think we no longer use the feature after this patch.

ELF/Symbols.h

53

uint32_t is probably better even though they are likely the same.

[• espindola]

In D46080#1078640, @ruiu wrote:

Maybe we should remove StringRefZ class and use a char pointer instead? The point of having StringRefZ class is that an instance of StringRefZ is automatically converted to StringRefZ when needed, but I think we no longer use the feature after this patch.

It is still convenient for the constructor argument. In some cases we know the size. Without it we have to overload the constructor so that there is a variant with "const char*" and one with StringRef.

[• espindola]

Use uint32_t

[ruiu]

LGTM

It might be worth trying to search for a way to remove StringRefZ class cleanly, but this patch looks good as is. Thank you for doing this!

[• espindola]

330874

ELF/Symbols.h

enum Kind {
     SharedKind,
     UndefinedKind,
     LazyArchiveKind,
     LazyObjectKind,
   };
 
   Kind kind() const { return static_cast<Kind>(SymbolKind); }
 
+  // The file from which this symbol was created.
+  InputFile *File;
+
+protected:
+  const char *NameStart;
+  mutable uint32_t NameSize;
+
+public:
+  uint32_t DynsymIndex = 0;
+  uint32_t GotIndex = -1;
+  uint32_t GotPltIndex = -1;
+  uint32_t PltIndex = -1;
+  uint32_t GlobalDynIndex = -1;
+
+  // Version definition index.
+  uint16_t VersionId;
+
   // Symbol binding. This is not overwritten by replaceSymbol to track
   // changes during resolution. In particular:
   //  - An undefined weak is still weak when it resolves to a shared library.
   //  - An undefined weak will not fetch archive members, but we have to
   //    remember it is weak.
   uint8_t Binding;
 
-  // Version definition index.
-  uint16_t VersionId;
+  // The following fields have the same meaning as the ELF symbol attributes.
+  uint8_t Type;    // symbol type
+  uint8_t StOther; // st_other field value
+
+  const uint8_t SymbolKind;
 
   // Symbol visibility. This is the computed minimum visibility of all
   // observed non-DSO symbols.
   unsigned Visibility : 2;
 
   // True if the symbol was used for linking and thus need to be added to the
   // output file's symbol table. This is true for all symbols except for
   // unreferenced DSO symbols and bitcode symbols that are unreferenced except
public:
   // False if LTO shouldn't inline whatever this symbol points to. If a symbol
   // is overwritten after LTO, LTO shouldn't inline the symbol because it
   // doesn't know the final contents of the symbol.
   unsigned CanInline : 1;
 
   // True if this symbol is specified by --trace-symbol option.
   unsigned Traced : 1;
 
-  // The file from which this symbol was created.
-  InputFile *File;
-
   bool includeInDynsym() const;
   uint8_t computeBinding() const;
   bool isWeak() const { return Binding == llvm::ELF::STB_WEAK; }
 
   bool isUndefined() const { return SymbolKind == UndefinedKind; }
   bool isDefined() const { return SymbolKind == DefinedKind; }
   bool isShared() const { return SymbolKind == SharedKind; }
   bool isLocal() const { return Binding == llvm::ELF::STB_LOCAL; }
 
   bool isLazy() const {
     return SymbolKind == LazyArchiveKind || SymbolKind == LazyObjectKind;
   }
 
   // True if this is an undefined weak symbol.
   bool isUndefWeak() const { return isWeak() && isUndefined(); }
 
-  StringRef getName() const { return Name; }
+  StringRef getName() const {
+    if (NameSize == (uint32_t)-1)
+      NameSize = strlen(NameStart);
+    return StringRef(NameStart, NameSize);
+  }
   void parseSymbolVersion();
 
   bool isInGot() const { return GotIndex != -1U; }
   bool isInPlt() const { return PltIndex != -1U; }
 
   uint64_t getVA(int64_t Addend = 0) const;
 
   uint64_t getGotOffset() const;
   uint64_t getGotVA() const;
   uint64_t getGotPltOffset() const;
   uint64_t getGotPltVA() const;
   uint64_t getPltVA() const;
   uint64_t getSize() const;
   OutputSection *getOutputSection() const;
 
-  uint32_t DynsymIndex = 0;
-  uint32_t GotIndex = -1;
-  uint32_t GotPltIndex = -1;
-  uint32_t PltIndex = -1;
-  uint32_t GlobalDynIndex = -1;
-
 protected:
   Symbol(Kind K, InputFile *File, StringRefZ Name, uint8_t Binding,
          uint8_t StOther, uint8_t Type)
-      : Binding(Binding), File(File), SymbolKind(K), NeedsPltAddr(false),
-        IsInGlobalMipsGot(false), Is32BitMipsGot(false), IsInIplt(false),
-        IsInIgot(false), IsPreemptible(false), Used(!Config->GcSections),
-        Type(Type), StOther(StOther), Name(Name) {}
-
-  const unsigned SymbolKind : 8;
+      : File(File), NameStart(Name.data()), NameSize(Name.rawSize()),
+        Binding(Binding), Type(Type), StOther(StOther), SymbolKind(K),
+        NeedsPltAddr(false), IsInGlobalMipsGot(false), Is32BitMipsGot(false),
+        IsInIplt(false), IsInIgot(false), IsPreemptible(false),
+        Used(!Config->GcSections) {}
 
 public:
   // True the symbol should point to its PLT entry.
   // For SharedSymbol only.
   unsigned NeedsPltAddr : 1;
   // True if this symbol has an entry in the global part of MIPS GOT.
   unsigned IsInGlobalMipsGot : 1;
 
   // True if this symbol is referenced by 32-bit GOT relocations.
   unsigned Is32BitMipsGot : 1;
 
   // True if this symbol is in the Iplt sub-section of the Plt.
   unsigned IsInIplt : 1;
 
   // True if this symbol is in the Igot sub-section of the .got.plt or .got.
   unsigned IsInIgot : 1;
 
   // True if this symbol is preemptible at load time.
   unsigned IsPreemptible : 1;
 
   // True if an undefined or shared symbol is used from a live section.
   unsigned Used : 1;
 
-  // The following fields have the same meaning as the ELF symbol attributes.
-  uint8_t Type;    // symbol type
-  uint8_t StOther; // st_other field value
-
   // The Type field may also have this value. It means that we have not yet seen
   // a non-Lazy symbol with this name, so we don't know what its type is. The
   // Type field is normally set to this value for Lazy symbols unless we saw a
   // weak undefined symbol first, in which case we need to remember the original
   // symbol's type in order to check for TLS mismatches.
   enum { UnknownType = 255 };
 
   bool isSection() const { return Type == llvm::ELF::STT_SECTION; }
   bool isTls() const { return Type == llvm::ELF::STT_TLS; }
   bool isFunc() const { return Type == llvm::ELF::STT_FUNC; }
   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; }
-
-protected:
-  StringRefZ Name;
 };
 
 // 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,
           uint8_t Type, uint64_t Value, uint64_t Size, SectionBase *Section)
       : Symbol(DefinedKind, File, Name, Binding, StOther, Type), Value(Value),

ELF/Symbols.cpp

void Symbol::parseSymbolVersion() {
   size_t Pos = S.find('@');
   if (Pos == 0 || Pos == StringRef::npos)
     return;
   StringRef Verstr = S.substr(Pos + 1);
   if (Verstr.empty())
     return;
 
   // Truncate the symbol name so that it doesn't include the version string.
-  Name = {S.data(), Pos};
+  NameSize = Pos;
 
   // If this is not in this DSO, it is not a definition.
   if (!isDefined())
     return;
 
   // '@@' in a symbol name means the default version.
   // It is usually the most recent one.
   bool IsDefault = (Verstr[0] == '@');

include/lld/Common/Strings.h

public:
   StringRefZ() : Start(nullptr), Size(0) {}
   StringRefZ(const char *S, size_t Size) : Start(S), Size(Size) {}
 
   /*implicit*/ StringRefZ(const char *S) : Start(S), Size(-1) {}
 
   /*implicit*/ StringRefZ(llvm::StringRef S)
       : Start(S.data()), Size(S.size()) {}
 
+  const char *data() const { return Start; }
+  size_t rawSize() const { return Size; };
+
   operator llvm::StringRef() const {
     if (Size == (size_t)-1)
       Size = strlen(Start);
     return {Start, Size};
   }
 
 private:
   const char *Start;