[PATCH] D56325: Sort symbols in .bss by size.

Sat Jan 19 09:55:49 PST 2019

grimar added a comment.

In D56325#1363572 <https://reviews.llvm.org/D56325#1363572>, @victoryang wrote:

> In D56325#1362861 <https://reviews.llvm.org/D56325#1362861>, @grimar wrote:
>
> > In D56325#1362094 <https://reviews.llvm.org/D56325#1362094>, @victoryang wrote:
> >
> > > Another data point: Sorting the data section for Chrome on Android, data section dirty pages went down from 6308KB to 6280KB (arm32).
> >
> >
> > Or 0.5%..
>
>
> 0.5% may not sound a lot, but that's already a helpful amount for some systems, where you have to constantly make tradeoffs that sacrifices some functionalities to free up memory.

Given all the results you provided, now this patch looks like a micro-optimization for some particular set of code.
In other words, based on results, it seems useless for non-micro apps to me. At the same time, it brings additional logic to the linker that is assumed to be used for all kind of applications,
and that code adds not big but still an additional complication that we will have to support.

Please understand my position right. I am an android user and I am happy with it, it's a good OS IMO, but this patch is going to change apps all over the world and the best result for them shown
in this thread seems to be "there is no regression". Having no regression is sure great, but doing micro-optimizations for particular apps and adding more code to linker for no benefits for 99% users
(I guess) is probably not.

I would be happy to hear other opinions, am I missing something? I do not want to be a stopper person for a good change to the linker, but is it a really good one?

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

https://reviews.llvm.org/D56325