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

[Rafael Avila de Espindola via llvm-commits]

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.


> -  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