This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [v3] locale cache PR 7076,8761
Nathan Myers writes:
> Looks good to me. One question (though not one to hold up applying
> the patch): why not have a second vector for the caches? Is it just
> to maintain the 3.3 ABI? Adding another member at the end of
> locale::_Impl shouldn't affect the ABI. Nobody can derive from it
> or make an array of them.
Yes, it was to maintain the ABI. The version I had done for mainline
had a second vector. I can switch back to using a separate array. Of
course, check-abi will moan about that as well.
>
> Generally, in inlines that are public or are used in many places,
> I like to see the fast part in the inline, and the slow part
> (allocating memory and initializing objects) in a non-inline
> adjunct. That may not apply in this case, if they are used only
> in one or a couple of places.
I'll buy that in general. In this case (I presume you're talking
about __use_cache?), I think the code will be less readable by the
time you're done. Also, wouldn't the compiler try to inline the aux
function anyway when it's feeling aggressive?
Actually, this raises an interesting question. If a branch of a
conditional is on the rare side of a __builtin_expect, will automatic
inlining still perform inlining into the branch? Should it? There
might be more advantage to loosening the limits elsewhere in a
function.
> Is check-abi complaining about the number of elements in the
> facet_vec member of locale::_Impl? I can't see how that would
> matter.
Yes. I didn't think it mattered either, but wanted to get second
opinions in case I was missing someting.
Jerry