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

[Rui Ueyama via llvm-commits]

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.


>
> > -  std::stable_sort(Symbols.begin(), Symbols.end(),
> > -                   [&](const Entry &L, const Entry &R) {
> > -                     return L.Hash % NBuckets < R.Hash % NBuckets;
> > -                   });
> > +  for (SymbolTableEntry &Ent : llvm::make_range(Mid, V.end())) {
>
> And here.
>
> > +    Symbol *B = Ent.Sym;
> > +    uint32_t Hash = hashGnu(B->getName());
> > +    uint32_t BucketIdx = Hash % NBuckets;
> > +    Symbols.push_back({B, Ent.StrTabOffset, Hash, BucketIdx});
> > +  }
> > +
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171201/180ace8e/attachment.html>