<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Oct 22, 2016 at 2:08 AM, Eugene Leviant <span dir="ltr"><<a href="mailto:evgeny.leviant@gmail.com" target="_blank">evgeny.leviant@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">evgeny777 added inline comments.<br>
<br>
<br>
================<br>
<span class="gmail-">Comment at: ELF/InputFiles.h:51<br>
+// for each object file we link.<br>
+template <class ELFT> class DIHelper {<br>
+public:<br>
----------------<br>
</span><span class="gmail-">ruiu wrote:<br>
> I think this is too smart, at least as the initial implementation. This function is run only when an error occurs, and for error cases, we don't pursue performance. Instead I'd like to keep it simple as possible. So, could you please remove this class and define a non-member function? I think it is okay to parse debug info for each error message.<br>
> could you please remove this class<br>
<br>
</span>I really really wouldn't like to do this. The main reason is that LLVM DWARF parser is very slow (main reason why lldb is so slow). If you want the numbers, here they are:<br>
<br>
command (reports about 3K errors):<br>
```<br>
ld.lld LoopConvertCheck.cpp.o -o a.out<br>
```<br>
time w/o caching: 3m31.165s<br>
time with caching: 0m1.659s<br></blockquote><div><br></div><div>Thank you for the numbers. It is way too slower than I expected. I'm convinced. (But why is this so slow in the first place?)</div><div><br></div><div>There seems a room to simplify this patch before submitting. I'd do</div><div><br></div><div> - Revert changes to Error.c and use the existing error() or warn() functions.</div><div> - Merge DIHelper an DIHelperImpl because the latter is the only derived class of the former.</div><div> - Don't make DIHelper functions virtual.</div><div> - Initialize DwarfLine member in the DIHelperImpl ctor and remove cache check from getLineInfo.</div><div> - Return a string "<filename>(<lineno>)" from getLineInfo so that we can remove #include <DWARFContext.h> from Relocations.cpp.</div><div><br></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 release build of lld on i7-2600K / SSD<br>
Debug build had worked over 15 minutes, before I terminated it with Ctrl-C<br>
<br>
May be it's better to simplify DIHelper somehow?<br>
<br>
<br>
<a href="https://reviews.llvm.org/D25826" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D25826</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>