<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 20, 2015, at 12:19 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="Apple-interchange-newline">On 2015 Jan 20, at 11:28, Frederic Riss <<a href="mailto:friss@apple.com" class="">friss@apple.com</a>> wrote:<br class=""><br class="">Hi dblaikie, echristo,<br class=""><br class="">First of all, although this patch passes the testsuite, I don't<br class="">think it is ready to go in, at least not without a thorough<br class="">discussion of its downsides.<br class=""><br class="">What the patch does is move away from using unique_ptr<>s to<br class="">track DIE object memory and allocate most of them using a<br class="">BumpAllocator. Making releasing the memory for the DIE tree<br class="">basically free. (DIEBlocks and DIELocs are still allocated using<br class="">the standard allocator, but they were already using separate<br class="">bookkeeping, so this doesn't complicate the patch).<br class=""><br class="">I'm going down this road because the teardown of the DIE tree<br class="">uses a lot of CPU time, especially in dsymutil's usecase. In<br class="">my prototype, the DIE destructors take ~40 seconds to run for<br class="">DIE trees worth of 700MB of debug infomration. That's quite a<br class="">lot especially considering that dsymutil's runtime on this kind<br class="">of data should be around 2 minutes. I really need to find an<br class="">acceptale solution for dsymutil's usecase.<br class=""><br class="">This patch achieves the performance gains that I expect, but there<br class="">is one big downside: it limits the number of attributes a DIE can<br class="">accept to a fixed maximum (well technically, if you remove the<br class="">assert in the code, it will work, but it will leak the attribute<br class="">vectors going over the limit).<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Is there a way to explicitly walk through and clear these vectors?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Or is that already too expensive (in most cases it'll be a no-op,</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">but you'll still need to do the walk)?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">(IIRC, `SmallVector<>::clear()` deallocates.)</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>I’d need to try it to be sure, but I have the feeling it would be expensive.</div><div>I seem to remember something about the average size of (encoded) DIEs</div><div>being 17 bytes (I haven’t checked that fact, but the order of magnitude </div><div>feels right). So for 700MB of encoded debug information that would </div><div>represent 43 million DIEs. That’s a huge tree to walk.</div><div><br class=""></div><div>Maybe it would however be possible to track these separately. Instead of</div><div>the assert, introduce some bookkeeping code that would remember the</div><div>DIEs that need their attributes freed. This is ugly in the sense that it needs</div><div>access to the bookkeeping structures from the addAttribute method.</div><div>Maybe the ugliness can be alleviated by having all the DIE operations go</div><div>through some new DIEAllocator object. Instead of</div><div>DIE->addValue(dwarf::DW_AT_something, Val);</div><div>you’d need to write</div><div>DIEAlloc.addValue(DIE, dwarf::DW_AT_something, Val);</div><div><br class=""></div><div>Thoughts?</div><div><br class=""></div><div>Fred</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="">Imposing this limit on users seems too strong as a constraint.<span class="Apple-converted-space"> </span><br class="">Especially considering not all frontends are in-tree. I can imagine<br class="">more flexible solutions involving complicated allocation strategies,<br class="">but I wanted to ask for opinions before I work on something that I<br class="">fear would end up much more complicated than this patch. Maybe<br class="">there's an easier solution that I haven't seen, happy to hear<br class="">suggestions.<br class=""><br class="">BTW, the runtime difference is definitely measurable on a clang<br class="">debug build also, although the gained seconds do not represent<br class="">40% of the runtime like in dsymutil's case. On my machine, a clang<br class="">debug build goes from:<br class=""><br class="">real 14m38.308s<br class="">user 45m26.856s<br class="">sys 3m53.519s<br class=""><br class="">without the patch, to:<br class=""><br class="">real 14m19.762s<br class="">user 44m14.438s<br class="">sys 3m45.520s<br class=""><br class="">Shaving more than one minute of user time and 15 to 20 seconds from<br class="">wall-clock time.<br class=""><br class=""><a href="http://reviews.llvm.org/D7072" class="">http://reviews.llvm.org/D7072</a><br class=""><br class="">Files:<br class="">include/llvm/CodeGen/DIE.h<br class="">lib/CodeGen/AsmPrinter/DIE.cpp<br class="">lib/CodeGen/AsmPrinter/DIEHash.cpp<br class="">lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp<br class="">lib/CodeGen/AsmPrinter/DwarfCompileUnit.h<br class="">lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br class="">lib/CodeGen/AsmPrinter/DwarfFile.cpp<br class="">lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br class="">lib/CodeGen/AsmPrinter/DwarfUnit.h<br class="">unittests/CodeGen/DIEHashTest.cpp<br class=""><br class="">EMAIL PREFERENCES<br class="">http://reviews.llvm.org/settings/panel/emailpreferences/<br class=""><D7072.18447.patch>_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div></blockquote></div><br class=""></body></html>