<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Dec 1, 2017 at 2:08 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"><div class="HOEnZb"><div class="h5">Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> writes:<br>
<br>
> On Fri, Dec 1, 2017 at 1:41 PM, Rafael Avila de Espindola <<br>
> <a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
><br>
>> 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>
>> 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>
>><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>
>> I would leave something like<br>
>><br>
>> auto HashedSyms = llvm::make_range(Mid, V.end();<br>
>><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>
>> And use it here.<br>
><br>
><br>
> llvm::range doesn't have `size()`, so you can't use it here.<br>
<br>
</div></div>You can use end() - begin(), no?<br></blockquote><div><br></div><div>I tried that but it didn't look beautiful, so I'd keep it as is . </div></div></div></div>