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

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Feb 23, 2023 at 10:28 AM Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>
> 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.

I didn't pursue it further since I couldn't show how much it would
improve performance.



-- 
H.J.