Re: Proposed patch to skip the last atomic decrements in _Sp_counted_base::_M_release

[Jonathan Wakely <jwakely@redhat.com>]

On 07/12/20 16:28 +0100, Marc Glisse wrote:
>Thanks for doing this!
>
>As someone who recently wrote
>
>  if (count.load(std::memory_order_relaxed) == 1
>      || count.fetch_sub(1, std::memory_order_release) == 1) {
>    std::atomic_thread_fence(std::memory_order_acquire);
>    // destroy here
>  }
>
>for a much simpler reference-counted type without weak references, I am
>happy to see that this can generalize to the more complicated case of
>shared_ptr.
>
>Yes, hardening it may be a bit hard. Some checks with alignof/offsetof 
>could make sense, but it is complicated by the fact that things are 
>happening in the base class that depend on the derived class. Some 
>allocators guarantee a minimum alignment, likely sufficient for your 
>case, but again I am not sure how to test for that exactly.
>
>ISTR that libstdc++ uses a surprisingly strong memory barrier when 
>incrementing references (acq_rel, where I would expect acquire to 
>suffice, although I haven't thought about weak references that much). 
>Did you also experiment with that?

That's because we go through the dispatcher functions in
<ext/atomicity.h> which don't take a memory_order argument so use a
conservative ordering. That could be improved.

The dispatchers are beneficial (especially with glibc 2.32 or later)
because they downgrade the atomic ops to non-atomic equivalents when
the program only has a single thread. The proposed patch loses that
benefit, meaning that the 8-byte load will always be done using an
atomic instruction, even when not needed. That should be improved too.