> It might do, keeping in mind that reading pretty much every existing
object
> file format already requires scanning for string lengths. Certainly
> something to try and evaluate, at least.
In lld strlen does show up in the profile. I haven't benchmarked it
recently enough to remember exactly how much.
So I have spent some time evaluating the various options that have been
discussed on this thread. They are:
1) adding the string table itself (this involves moving all data in the VST
to either the string table or elsewhere in the bitcode, with the exception
of function offsets)
2) representing string references as offset/size
3) removing VST function offsets (and the VST itself)
I am attaching three patches with my prototypes, where t1 implements the
string table, t2 is string table + remove function offsets and t3 is string
table + remove function offsets + offset/size. (They are all against
r299588.) I used these patches to evaluate the perf impact when linking
Chromium with ThinLTO.
Median time elapsed for a no-op incremental link (#runs = 100):
- trunk: 51.71s
- string table: 48.66s (-5.9%)
- string table + remove function offsets: 49.12s (-5.0%)
- string table + remove function offsets + offset/size: 48.10s (-7.0%)
And for a full link (#runs = 13):
- trunk: 605.24s
- string table: 597.88s (-1.3%)
- string table + remove function offsets: 598.99s (-1.1%)
- string table + remove function offsets + offset/size: 598.97s (-1.1%)
It is hard to draw any conclusions from the full ThinLTO links (partly due
to the small #runs) other than that adding the string table is a clear win.
But I think it is clear enough from the incremental results that
offset/size is a win. And as Duncan mentioned it should allow us to more
easily share strings with the metadata string table.
Function offsets do appear to be a win, so it seems likely that we will
want something like them, either in the symbol table or elsewhere. My
conclusion is that there should be no great harm in storing just the
function offsets in the VST for now because, after we add the string table,
the data in the VST will no longer be needed for correctness and any
potential future change that moves the function offsets elsewhere can
simply start ignoring the VST in the reader.
I also measured the impact on file size by taking the total sum of object
file sizes for each of the variants. They are:
- trunk: 1183510504 bytes
- string table: 1151533664 bytes (-2.7%)
- string table + remove function offsets: 1147981736 bytes (-3.0%)
- string table + remove function offsets + offset/size: 1149361632 bytes
(-2.9%)
The decrease in size vs trunk is expected because of sharing of strings
between comdats and globals as well as between modules. But the differences
in sizes between the different options does not seem as important.
So here is what I plan to do:
- Create a new patch that implements string table + offset/size and take
further measurements, to make sure that removing function offsets is not a
confounding factor for performance.
- Clean up the patch and send it for review.
The string table builder code currently supports tail merging. On ELF
at least that is a very modest size saving. If the size is written as
a prefix to the string, that doesn't work. If the size is stored in
the reference (like a stringef) we would be able to merge any
substring (not sure if it is profitable).
I agree that we should store the size in the reference. Tail merging should
help on targets that require IR name mangling because the mangled name
should almost always be a suffix of the unmangled name or vice versa.
Thanks,
t1.diff (67.2 KB)
t2.diff (66.2 KB)
t3.diff (67 KB)