[gold][patch] Small code refactoring

[Ian Lance Taylor]

Rafael Espindola <espindola@google.com> writes:

>> Actually, I think you are taking slightly the wrong approach here.  The
>> two cases have different error handling behaviour--that's why they wound
>> up being different.  In the archive case, if it's not an ELF file, we
>> fail immediately.  In the readsyms case, we go on to check for a linker
>> script.
>
> That is still the case with the patch.  The new make_elf_object will
> return null. The archive case will fail an the readsyms
> case will check for an script.

Sure, but the error handling is different.  I think your patch will lead
to two error messages if your make_elf_object2 returns NULL in an
archive.


>> Also, your version calls get_view twice in the normal case.  gold gets a
>> surprising amount of its speed by carefully avoiding these sorts of
>> micro-deoptimizations in the normal case.  I spent a lot of time
>> profiling to find them.
>
> That is true. The get_view is cached, no?

Yes, but get_view is a nontrivial function.  The fact that we avoid a
system call doesn't make it free.


>> One approach here would be to write a predicate is_elf_object which
>> takes read_size and ehdr and returns whether it looks like ELF.  Then if
>> is_elf_object returns true, call make_elf_object.
>
> I can try that, but one of the things I like about my patch is that it
> makes the make_elf_object interface a bit simpler.

In gold, simple is good, but speed always wins.  I'm willing to make the
code very complicated indeed to save 1% of runtime (not that your patch
would cost 1%).

Ian