[PATCH] D65242: [ELF] More dynamic relocation packing

Mon Jul 29 11:31:45 PDT 2019

pcc added a comment.

In D65242#1604869 <https://reviews.llvm.org/D65242#1604869>, @victoryang wrote:

> In D65242#1604825 <https://reviews.llvm.org/D65242#1604825>, @pcc wrote:
>
> > > Linking libmonochrome in Chromium for
> > >  Android, targeting ARM32, I see a .rel.dyn of size 127697 bytes, as
> > >  compared to 150532 bytes without this change.
> >
> > IIRC over 99% of relocations in libmonochrome were relative, so I didn't implement anything for the non-relative relocations, but maybe there are more non-relative relocations now. I can imagine that if say you were building it as a component build that would result in more non-relative relocations than in shipping builds. It may also be more compelling to mention how much this helps for the Android platform which I believe uses non-relative relocations more heavily.
>
>
> Thanks for pointing this out! I've updated the description to include test results on Android.

Thanks for adding those figures. I'm still not sure about your Chromium figures though. I just did a quick build myself and I'm still seeing >99% relative relocations without significant duplication in the non-relative relocations so I'm curious to see what your `args.gn` looks like for Chromium.

>> It may also be worth compressing `.rela.plt` now that we're compressing non-relative relocations.
> 
> Do we expect duplicated symbol lookup in .rela.plt? Honestly I'm not sure, but I'm under the impression that we'd have one PLT entry for each external function, so we won't see duplication?

You may have duplication between `.rela.plt` and `.rela.dyn` if a function is both called and address taken. But if there are many PLT entries then you should expect to see an improvement just from compressing the relocations themselves since right now they're being stored uncompressed.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65242/new/

https://reviews.llvm.org/D65242