[PATCH] D45571: [ELF] - Speedup MergeInputSection::splitStrings

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola added a comment.

I just noticed that hash_short will read at most 64 bytes of the string.

This could cause a pathological case if many symbols have a common prefix, no?

One interesting thing about the current setup is that we first read the bytes in the string looking for the 0 that terminates the string. We then read them again to hash them. At least with a simple hash like djb (what is implemented in hashGnu) it should not be too hard to read each byte once. Would you mind giving that a try?



================
Comment at: ELF/InputSection.cpp:903
   for (size_t I = 0; I != Size; I += EntSize)
     Pieces.emplace_back(I, xxHash64(toStringRef(Data.slice(I, EntSize))),
                         !IsAlloc);
----------------
grimar wrote:
> espindola wrote:
> > For consistency we should probably also replace this use of xxHash64.
> I just did not find a good test for that place. I tried to do that change, but for Scylla, there was no difference, so decided to not do that in this patch to keep the focus on the main place.
I don't think there is ever a case where a non SHF_STRING SHF_MERGE section shows up in the profile.


https://reviews.llvm.org/D45571