[PATCH] D26130: [ELF] - Implemented --symbol-ordering-file option.

[Davide Italiano via llvm-commits]

davide added a comment.

In https://reviews.llvm.org/D26130#584374, @grimar wrote:

> In https://reviews.llvm.org/D26130#583948, @ruiu wrote:
>
> > Well, again, you really need to run a long sequence of computations to get meaningful numbers for benchmark. `clang -help` isn't useful for that purpose.
>
>
> Yes, Rui, please don't think I forgot about that opinion :) clang -help looks good because of very stable results. As I pointed I observe the same amount of faults for the same binary on 2 different PC. So it defenetely (IMO) proves that patch can be used to reduce them during startup.
>
> In https://reviews.llvm.org/D26130#583961, @davide wrote:
>
> > In https://reviews.llvm.org/D26130#583948, @ruiu wrote:
> >
> > > Well, again, you really need to run a long sequence of computations to get meaningful numbers for benchmark. `clang -help` isn't useful for that purpose.
> >
> >
> > Agree (as already pointed out in the other thread review). I discussed this a bit with Rafael and I see his point of view, i.e. `clang -help` is definitely a sanity check, and the number of page faults being decreased is a good thing, but I'd really love this to be tested on a startup-time sensitive application.
> >  I mentioned browsers, in particular Firefox, because the Mozilla people spent quite a bit of time thinking about the problem, so you should be able to reuse all their infrastructure (tests etc..). Firefox should link out-the-box with lld, and if it doesn't .. well there's more work to to there =)
>
>
> I did not yet try firefox with this patch. But as I wrote in another thread, I was unable to obtain a boost using --sections-ordering-file on clang and firefox previously. Though sections file for firefox produced by icegrind was 5 megabytes in size, what is much more than 300 entries that works with this patch.
>  So the noticable result using --symbol-ordering-file and Rafael's method of generating it is probably  a step forward that was worth to share.
>
> Will try to test firefox using this patch.
>
> (about linking firefox with lld. When last time I did that, it produced next error for me during launch:
>
> > umb at ubuntu:~/firefox/clean/mozilla-central$ /home/umb/firefox/clean/mozilla-central/obj-x86_64-pc-linux-gnu/dist/bin/firefox -no-remote -profile /home/umb/firefox/clean/mozilla-central/obj-x86_64-pc-linux-gnu/tmp/scratch0
> >  XPCOMGlueLoad error for file /home/umb/firefox/clean/mozilla-central/obj-x86_64-pc-linux-gnu/dist/bin/libxul.so:
> >  /home/umb/firefox/clean/mozilla-central/obj-x86_64-pc-linux-gnu/dist/bin/libxul.so: undefined symbol: gtk_main_do_event
> >  Couldn't load XPCOM.
>
> so I had to link firefox with gold and then relink libxul.so with lld. That should have being fine I think since that is the single core library of firefox and sections file was targeted for it.
>  And anyways latest tests I performed on vanilla gold to check the option itself, and result was the same).


If lld mislinks firefox, we shouldn't silently ignore it and try to link with gold, but open a bug instead (and/or try to fix).


https://reviews.llvm.org/D26130