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