Re: [PATCH v6 3/4] Reduce CAS in malloc spinlocks

[Wilco Dijkstra <Wilco.Dijkstra@arm.com>]

Hi DJ,

>        size_t n = narenas;
>        if (__glibc_unlikely (n <= narenas_limit - 1))
>          {
> +          if (atomic_load_relaxed (&narenas) != n)
> +           {
> +              atomic_spin_nop ();
> +              goto repeat;
> +           }
>            if (catomic_compare_and_exchange_bool_acq (&narenas, n + 1, n))
>              goto repeat;

Before we consider optimizing it, we should first simplify it. All this wants
to do is a relaxed atomic add, then check the maximum arenas and
treat the case of having too many arenas in the same way as failure to
create another arena (ie. just atomically decrement again). Ie. no CAS
loop required, and there is nothing to optimize either.

> Should target-specific atomics optimizations be "hidden" somewhere in
> the atomics implementation?  Just because x86 may benefit from a
> pre-read doesn't mean that all targets will, and if x86 generally
> benefits, it should update its implementation of the atomics to do that
> at a lower level.

We have been removing secret optimizations hidden behind atomics and just
use standard atomics everywhere. These micro optimizations are often counter-
productive - it's far better to do them at a higher similar to the single-thread
path in malloc/free.

For these there is no evidence they are heavily contended - if anything the
extra code will just slow things down. The cost is in the CAS itself, we could
remove it from the multithreaded malloc path by splitting the free list into a
local one (only accessed when you have the malloc lock) and a concurrent one.

Cheers,
Wilco