<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 26, 2016, at 3:59 PM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" class="">dberlin@dberlin.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Jan 26, 2016 at 3:35 PM, Mehdi Amini <span dir="ltr" class=""><<a href="mailto:mehdi.amini@apple.com" target="_blank" class="">mehdi.amini@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="">The hash should (almost?) never change.<div class="">I thought about this solution, and I plan on evaluating it a well (it applies to constants as well). </div><div class="">However there is a tradeoff of memory vs speed doing this.</div><div class="">For instance when doing LTO, we load a lot of metadata when loading and linking individual Modules, which put a lot of stress on the hash tables (both uniquing and grow). But then during optimization and CodeGen it shouldn’t and I’m not sure we want to pay the price of the memory overhead then.</div><div class=""><br class=""></div></div></blockquote><div class="">This naturally raises the questions of whether you really need hash tables to get what you want, or would some other more memory-efficient data structure be better for uniquing :)</div></div></div></div></div></blockquote><div><br class=""></div><div>I think this is a great point.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""></div><div class="">While DenseMap is pretty good,  you do end up with a lot of empty buckets (or pay a high probing price).<br class=""></div><div class=""><br class=""></div><div class="">Whereas, depending on the situation and structure, you can often pay a much lower cost (and have better behavior on hash misses). Obviously, in most cases, DenseMap should be the choice because it's consistent and who cares, but in specific situations if you are placing huge stress on it during say, uniquing, there are often better ways both in time and space (various compressed tries, ternary search trees, etc).</div></div></div></div></div></blockquote><div><br class=""></div><div>Debug Info metadata are putting a lot of stress both on CPU time as it shows up on the profile quite frequently, and on the memory (even it is far better now than in the past following all the work driven by Duncan last year).</div><div>An LTO build of llvm-tblgen consumes at peak 130MB without debug info, and 740MB with debug info. Since you’re familiar with GCC are you aware of the </div><div><br class=""></div><div>I’ll look forward talking with you in more details about the various tradeoff and what would be the best data structure going forward!</div><div><br class=""></div><div>— </div><div>Mehdi</div><div><br class=""></div><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""></div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""></div><div class="">Note also that a ThinLTO link of `opt` right now involves loading ~30000 bitcode files, so it is roughly 30s CPU time overall for the link time for each millisecond saved for each file loaded :)</div><div class=""><br class=""></div><div class="">Let me know if you have any other suggestion!</div><div class=""><br class=""></div><div class="">— </div><span class="HOEnZb"><font color="#888888" class=""><div class="">Mehdi</div><div class=""><br class=""></div></font></span><div class=""><div class=""><br class=""><div class=""><blockquote type="cite" class=""><span class=""><div class="">On Jan 26, 2016, at 1:57 PM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" target="_blank" class="">dberlin@dberlin.org</a>> wrote:</div><br class=""></span><div class=""><div class="h5"><div class=""><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="">How often does the hash change?<br class=""><div class="">If it's really speed important,  you could:</div><div class="">A. store the hash value</div><div class="">B. update it when fields change.</div><div class=""><br class=""></div></div><div class="gmail_extra" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br class=""><div class="gmail_quote">On Tue, Jan 26, 2016 at 1:23 AM, Mehdi AMINI via llvm-commits<span class=""> </span><span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span><span class=""> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">joker.eph created this revision.<br class="">joker.eph added a reviewer: dexonsmith.<br class="">joker.eph added a subscriber: llvm-commits.<br class=""><br class="">This patch changes the computation of the hash key for DISubprogram to<br class="">be computed on a small subset of the fields. The hash is computed a<br class="">lot faster, but there might be more collision in the table.<br class="">However by carefully selecting the fields, colisions should be rare.<br class=""><br class="">Using `opt` to load the IR for FastISelEmitter.cpp.o, this patch<br class="">improves DISubprogram::getImpl() runtime from 28ms to 15ms.<br class=""><br class=""><a href="http://reviews.llvm.org/D16571" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D16571</a><br class=""><br class="">Files:<br class=""> <span class=""> </span>lib/IR/LLVMContextImpl.h<br class=""><br class="">Index: lib/IR/LLVMContextImpl.h<br class="">===================================================================<br class="">--- lib/IR/LLVMContextImpl.h<br class="">+++ lib/IR/LLVMContextImpl.h<br class="">@@ -519,10 +519,7 @@<br class="">           <span class=""> </span>Variables == RHS->getRawVariables();<br class="">   }<br class="">   unsigned getHashValue() const {<br class="">-    return hash_combine(Scope, Name, LinkageName, File, Line, Type,<br class="">-                        IsLocalToUnit, IsDefinition, ScopeLine, ContainingType,<br class="">-                        Virtuality, VirtualIndex, Flags, IsOptimized,<br class="">-                        TemplateParams, Declaration, Variables);<br class="">+    return hash_combine(Scope, File, Type, Line);<br class="">   }<br class=""> };<br class=""><br class=""><br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></blockquote></div></div></div></div></div></blockquote></div><br class=""></div></div></div></blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></body></html>