[libcxx-commits] [libcxx] [libc++] Speed up classic locale (PR #70631)

[via libcxx-commits]

philnik777 wrote:

> > As Nikolas mentioned, this is an ABI break. The speedup is interesting, but we'd need to either find a way to make this not an ABI break, or to only enable the optimization under an ABI flag.
> > Is there any reason why we're not storing the classic locale pointer you're adding to the `locale` class inside the `__imp` class instead? I think that would allow not breaking the ABI, since that's a pimpl idiom.
> 
> I don't understand the problem space well, so I am open to suggestions re ABI. What exactly is breaking ABI here? New methods? Inline methods? The static field?

The ABI break is actually that you remove functions from the dylib. Adding new ones isn't a huge problem.

> What is ABI flag? How bad is this? What are other options? Were there similar precedents?

An ABI flag is a macro that is only defined if it's OK to break the ABI. They are listed [here](https://github.com/llvm/llvm-project/blob/e734bc53de8173a1b54493f3e0c8f70575341a9e/libcxx/include/__config#L98-L171).

> Re __imp/pimpl, the original motivation was inlining. __imp is not in the header, so we can get access to it from any inline methods. If there are no good options that preserve inlining, I guess we can give up on it. It contributes to this speed somewhat:
> 
> ```
> Ostream_number/0/real_time/threads:1                   184.0n ± 1%   135.0n ± 1%  -26.63% (p=0.000 n=24)
> ```
> 
> But for this, more important one, it's probably in the noise:
> 
> ```
> Ostream_number/0/real_time/threads:72                17279.0n ± 4%   308.0n ± 1%  -98.22% (p=0.000 n=24)
> ```

I'm a bit confused about this. The statements here seem contradictory. Could you try to reword this?

https://github.com/llvm/llvm-project/pull/70631