<div dir="ltr"><div dir="ltr">On Tue, Feb 5, 2019 at 2:35 PM Rui Ueyama via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">ruiu added inline comments.<br>
<br>
<br>
================<br>
Comment at: wasm/SymbolTable.cpp:89-90<br>
     Inserted = true;<br>
+    if (TraceSymbols.count(Name) != 0)<br>
+      Sym->Traced = true;<br>
   }<br>
----------------<br>
Any operation that we do for each symbol can be super expensive, as the number of symbols can be in the order of millions. In particular, lld should do more than one hash table lookup for each symbol. </blockquote><div><br></div><div>s/should/should not/</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This is one of reasons why lld is so fast.<br>
<br>
So, this new hash table shouldn't be added. Take a look at ELF/SymbolTable.cpp. We implemented this feature without adding a new hash table lookup.<br>
<br>
<br>
Repository:<br>
  rLLD LLVM Linker<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D57725/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D57725/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D57725" rel="noreferrer" target="_blank">https://reviews.llvm.org/D57725</a><br>
<br>
<br>
<br>
</blockquote></div></div>