This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold][patch] Small code refactoring
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