[PATCH] D40697: Cache modulo values for the .gnu.hash section.

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 1 14:08:02 PST 2017


Rui Ueyama <ruiu at google.com> writes:

> On Fri, Dec 1, 2017 at 1:41 PM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
>
>> Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:
>>
>> > ruiu updated this revision to Diff 125067.
>> > ruiu added a comment.
>> >
>> > - clang-format
>> >
>> > It seems this patch actually makes that test case 2% faster, which
>> > isn't a negligible improvement.
>>
>> Please benchmark more than clang. There is a set of different benchmarks
>> in https://s3-us-west-2.amazonaws.com/linker-tests/lld-speed-test.tar.xz.
>>
>>
>> >    // Write hash buckets. Hash buckets contain indices in the following
>> >    // hash value table.
>> > @@ -1792,21 +1792,22 @@
>> >    if (Mid == V.end())
>> >      return;
>> >
>> > -  for (SymbolTableEntry &Ent : llvm::make_range(Mid, V.end())) {
>> > -    Symbol *B = Ent.Sym;
>> > -    Symbols.push_back({B, Ent.StrTabOffset, hashGnu(B->getName())});
>> > -  }
>> > -
>>
>> I would leave something like
>>
>> auto HashedSyms = llvm::make_range(Mid, V.end();
>>
>>
>> >    // We chose load factor 4 for the on-disk hash table. For each hash
>> >    // collision, the dynamic linker will compare a uint32_t hash value.
>> >    // Since the integer comparison is quite fast, we believe we can make
>> >    // the load factor even larger. 4 is just a conservative choice.
>> > -  NBuckets = std::max<size_t>(Symbols.size() / 4, 1);
>> > +  NBuckets = std::max<size_t>((V.end() - Mid) / 4, 1);
>>
>> And use it here.
>
>
> llvm::range doesn't have `size()`, so you can't use it here.

You can use end() - begin(), no?

Cheers,
Rafael




More information about the llvm-commits mailing list