[lld] r257017 - ELF: Improve performance of string table construction.

[Rafael Espíndola via llvm-commits]

On 15 January 2016 at 18:05, Rui Ueyama <ruiu at google.com> wrote:
> On Thu, Jan 14, 2016 at 7:25 AM, Rafael Espíndola
> <rafael.espindola at gmail.com> wrote:
>>
>> > In total, this patch improves link time of lld itself about 12%
>> > (3.50 seconds -> 3.08 seconds.)
>>
>> That is quite slow, is in a debug build that you are linking? What is
>> the size of the string table if you use discard_locals? I wonder if we
>> are keeping more symbols than gold or bfd.
>
>
> That is a debug build. I haven't tried discard_locals yet,

Using a no-asserts build of lld to link a Release+Asserts build of lld
with all arches, what I got was:

r257017:
* strtab is 3916859 bytes
* link takes 0.317558552 seconds

r257016:
* strtab is 3812950 bytes (1.027* smaller)
* link takes 0.353217270 seconds (1.11* slower)

r257016 with StrTabBuilder.finalize commented out as a timing hack.
* link time is 0.331792363 (1.04X slower)

Which is strange, so I got about 3% instead of 0.3% but about the same
slowdown. The link itself is about 10X faster than what you reported.

>> Have you tried just not calling StrTabBuilder.finalize() similar to
>> what we do for SHF_STRINGS sections when not optimizing? That way we
>> would have fast linking at -O0 and smaller tables at -O2.
>
>
> and neither StrTabBuilder.finalize(). But I don't want to use
> StrTabBuilder.finalize() because the API has the problem as I described in
> this patch. Look for "Why two phases?" and read the comment. Also, I think
> StringTableBuilder is slower than this implementation even if we don't call
> finalize() because StringTableBuilder copies strings, while this
> implementation does not (it keeps only StringRefs).

Any implementation that removes duplicated strings will have to be a
bit more expensive. The question is if it is still noticeable when not
tail merging. Given that the timing with the hack was still doing
unnecessary hash lookups, I would say we should at least revisit this.
I will open a bug to track it.

Cheers,
Rafael