[PATCH] D25826: [ELF] Show error location for 'undefined symbol' errors

[Rui Ueyama via llvm-commits]

On Sat, Oct 22, 2016 at 2:08 AM, Eugene Leviant <evgeny.leviant at gmail.com>
wrote:

> evgeny777 added inline comments.
>
>
> ================
> Comment at: ELF/InputFiles.h:51
> +// for each object file we link.
> +template <class ELFT> class DIHelper {
> +public:
> ----------------
> ruiu wrote:
> > 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.
> > could you please remove this class
>
> 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:
>
> command (reports about 3K errors):
> ```
> ld.lld LoopConvertCheck.cpp.o -o a.out
> ```
> time w/o caching: 3m31.165s
> time with caching: 0m1.659s
>

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?)

There seems a room to simplify this patch before submitting. I'd do

 - Revert changes to Error.c and use the existing error() or warn()
functions.
 - Merge DIHelper an DIHelperImpl because the latter is the only derived
class of the former.
 - Don't make DIHelper functions virtual.
 - Initialize DwarfLine member in the DIHelperImpl ctor and remove cache
check from getLineInfo.
 - Return a string "<filename>(<lineno>)" from getLineInfo so that we can
remove #include <DWARFContext.h> from Relocations.cpp.

This is release build of lld on i7-2600K / SSD
> Debug build had worked over 15 minutes, before I terminated it with Ctrl-C
>
> May be it's better to simplify DIHelper somehow?
>
>
> https://reviews.llvm.org/D25826
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161024/376dd46e/attachment.html>