Skip to content

Commit

Permalink
[ELF] Remove StringRefZ
Browse files Browse the repository at this point in the history
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
  • Loading branch information
MaskRay committed Jan 20, 2022
1 parent e39dae8 commit 03909c4
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 31 deletions.
2 changes: 1 addition & 1 deletion lld/ELF/InputFiles.cpp
Expand Up @@ -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)
Expand Down
41 changes: 11 additions & 30 deletions lld/ELF/Symbols.h
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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) {}
Expand Down Expand Up @@ -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) {}
Expand All @@ -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) {}
Expand Down

0 comments on commit 03909c4

Please sign in to comment.