[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

Tue Aug 14 06:13:32 PDT 2018

hans added a comment.

In https://reviews.llvm.org/D50652#1197775, @rnk wrote:

> I'd prefer not to do this, since internal_linkage generates smaller, more debuggable code by default.

IIUC, this is what we had before though, so it's a conservative "revert to safety" approach until a better solution can be found.

> I think the symbol table size increase may be specific to MachO, and it may be possible to work around this by changing ld64 to pool strings for symbols by default. I don't know enough about MachO to say if this is possible.

I looked some more at my local build. The sum of object file sizes didn't increase very much, the big growth is for the executable, and there it's mostly the string table as your analysis showed.

As far as I understand, string pooling should work fine for Mach-O; LLVM does it for object files already. And yes, ld64 doesn't do it. I looked at this symbol:

  __ZNSt3__1eqERKNS_21__tree_const_iteratorIN7testing11ExpectationEPNS_11__tree_nodeIS2_PvEElEES9_

If I understand correctly that function would previously have been inlined everywhere, but now it occurs 20+ times in the symbol table of base_unittests, and yes it's repeated 20+ times in the string table too :-/

> I also think the symbol table size problem may be limited to "large" users of C++: people with 500+ object files using libc++ in a DSO. I'm more comfortable imposing a cost on these users to ask them to opt in to some setting that disables internal_linkage and always_inline than I am leaving always_inline on by default, which adversely affects all users.

Sure, if there's a macro we can use to opt in to get the old inlining behaviour, or maybe even better getting linkonce_odr linkage, I'd be a happy camper. But the cost of just growing the executables this much seems like a real problem for our build infrastructure.

Repository:
  rCXX libc++

https://reviews.llvm.org/D50652