Skip to content

Commit 03909c4

Browse files
committed
[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
1 parent e39dae8 commit 03909c4

File tree

2 files changed

+12
-31
lines changed

2 files changed

+12
-31
lines changed

lld/ELF/InputFiles.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1067,7 +1067,7 @@ template <class ELFT> void ObjFile<ELFT>::initializeSymbols() {
10671067
sourceFile = CHECK(eSym.getName(stringTable), this);
10681068
if (LLVM_UNLIKELY(stringTable.size() <= eSym.st_name))
10691069
fatal(toString(this) + ": invalid symbol name offset");
1070-
StringRefZ name = stringTable.data() + eSym.st_name;
1070+
StringRef name(stringTable.data() + eSym.st_name);
10711071

10721072
symbols[i] = reinterpret_cast<Symbol *>(locals + i);
10731073
if (eSym.st_shndx == SHN_UNDEF || sec == &InputSection::discarded)

lld/ELF/Symbols.h

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -42,20 +42,6 @@ class SharedSymbol;
4242
class Symbol;
4343
class Undefined;
4444

45-
// This is a StringRef-like container that doesn't run strlen().
46-
//
47-
// ELF string tables contain a lot of null-terminated strings. Most of them
48-
// are not necessary for the linker because they are names of local symbols,
49-
// and the linker doesn't use local symbol names for name resolution. So, we
50-
// use this class to represents strings read from string tables.
51-
struct StringRefZ {
52-
StringRefZ(const char *s) : data(s), size(-1) {}
53-
StringRefZ(StringRef s) : data(s.data()), size(s.size()) {}
54-
55-
const char *data;
56-
const uint32_t size;
57-
};
58-
5945
// Some index properties of a symbol are stored separately in this auxiliary
6046
// struct to decrease sizeof(SymbolUnion) in the majority of cases.
6147
struct SymbolAux {
@@ -87,7 +73,8 @@ class Symbol {
8773

8874
protected:
8975
const char *nameData;
90-
mutable uint32_t nameSize;
76+
// 32-bit size saves space.
77+
uint32_t nameSize;
9178

9279
public:
9380
// A symAux index used to access GOT/PLT entry indexes. This is allocated in
@@ -179,11 +166,7 @@ class Symbol {
179166
// all input files have been added.
180167
bool isUndefWeak() const { return isWeak() && isUndefined(); }
181168

182-
StringRef getName() const {
183-
if (nameSize == (uint32_t)-1)
184-
nameSize = strlen(nameData);
185-
return {nameData, nameSize};
186-
}
169+
StringRef getName() const { return {nameData, nameSize}; }
187170

188171
void setName(StringRef s) {
189172
nameData = s.data();
@@ -196,10 +179,7 @@ class Symbol {
196179
//
197180
// For @@, the name has been truncated by insert(). For @, the name has been
198181
// truncated by Symbol::parseSymbolVersion().
199-
const char *getVersionSuffix() const {
200-
(void)getName();
201-
return nameData + nameSize;
202-
}
182+
const char *getVersionSuffix() const { return nameData + nameSize; }
203183

204184
uint32_t getGotIdx() const {
205185
return auxIdx == uint32_t(-1) ? uint32_t(-1) : symAux[auxIdx].gotIdx;
@@ -266,10 +246,11 @@ class Symbol {
266246
inline size_t getSymbolSize() const;
267247

268248
protected:
269-
Symbol(Kind k, InputFile *file, StringRefZ name, uint8_t binding,
249+
Symbol(Kind k, InputFile *file, StringRef name, uint8_t binding,
270250
uint8_t stOther, uint8_t type)
271-
: file(file), nameData(name.data), nameSize(name.size), binding(binding),
272-
type(type), stOther(stOther), symbolKind(k), visibility(stOther & 3),
251+
: file(file), nameData(name.data()), nameSize(name.size()),
252+
binding(binding), type(type), stOther(stOther), symbolKind(k),
253+
visibility(stOther & 3),
273254
isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind),
274255
exportDynamic(isExportDynamic(k, visibility)), inDynamicList(false),
275256
canInline(false), referenced(false), traced(false),
@@ -348,7 +329,7 @@ class Symbol {
348329
// Represents a symbol that is defined in the current output file.
349330
class Defined : public Symbol {
350331
public:
351-
Defined(InputFile *file, StringRefZ name, uint8_t binding, uint8_t stOther,
332+
Defined(InputFile *file, StringRef name, uint8_t binding, uint8_t stOther,
352333
uint8_t type, uint64_t value, uint64_t size, SectionBase *section)
353334
: Symbol(DefinedKind, file, name, binding, stOther, type), value(value),
354335
size(size), section(section) {}
@@ -383,7 +364,7 @@ class Defined : public Symbol {
383364
// section. (Therefore, the later passes don't see any CommonSymbols.)
384365
class CommonSymbol : public Symbol {
385366
public:
386-
CommonSymbol(InputFile *file, StringRefZ name, uint8_t binding,
367+
CommonSymbol(InputFile *file, StringRef name, uint8_t binding,
387368
uint8_t stOther, uint8_t type, uint64_t alignment, uint64_t size)
388369
: Symbol(CommonKind, file, name, binding, stOther, type),
389370
alignment(alignment), size(size) {}
@@ -396,7 +377,7 @@ class CommonSymbol : public Symbol {
396377

397378
class Undefined : public Symbol {
398379
public:
399-
Undefined(InputFile *file, StringRefZ name, uint8_t binding, uint8_t stOther,
380+
Undefined(InputFile *file, StringRef name, uint8_t binding, uint8_t stOther,
400381
uint8_t type, uint32_t discardedSecIdx = 0)
401382
: Symbol(UndefinedKind, file, name, binding, stOther, type),
402383
discardedSecIdx(discardedSecIdx) {}

0 commit comments

Comments
 (0)