On Wed, Aug 28, 2024 at 9:12 AM Oliver Schädlich via Std-Proposals <
std-proposals_at_[hidden]> wrote:

> I recently have presented my idea for an improvement of shared_ptr<> in
> combination with
> atomic<shared_ptr<>> here, but I was misunderstood. My idea was simply
> that shared_ptr<>
> should add a new assignement-operator with a atomic<shared_ptr<>> as its
> parameter.
>
I believe I understood that `operator=` was the form-factor you hit on
first, yes. But that's not the right form-factor.
I very strongly oppose adding any new overloaded operator to `atomic` which
does not have any named-member-function version, for two reasons:
- Good multithreaded code IMNSHO makes all atomic operations explicit (e.g.
`int b = a.load(); a.store(b)`).
- You need somewhere to hang the `memory_order` argument, anyway.

With current C++ load()ing from a atomic<shared_ptr<>> is extremely slow,
> even when you
> have a lock-free solution since the cacheline-flipping between the cores
> is rather expensive.
> I've written my own shared_obj<>- (like shared_ptr<>) and
> tshared_obj<>-classes (like atomic
> <shared_ptr<>>) and with this optimization I get a speedup of factor 5,000
> when 32 threads
> are contending on a single thared_obj<>, i.e. constantly copying it's
> pointer to a shared_obj<>
> object while the tshared_obj is rarely updated.
>
> This is the code:
>
> template<typename T>
> shared_obj<T> &shared_obj<T>::operator =( tshared_obj<T> const &ts )
> noexcept
> {
> using namespace std;
> // important optimization for RCU-like patterns:
> // we're managing the same object like the thread-safe object-pointer ?
> if( (size_t)m_ctrl == ts.m_dca.first().load( memory_order_relaxed ) )
> [[likely]]
> // yes: nothing to do
> return *this;
>
FYI, this would not be correct for `std::shared_ptr`. With
`std::shared_ptr` it is totally possible to have two shared_ptrs owning the
same control block yet with different values:
    std::shared_ptr<int[]> a = std::make_shared<int[]>(2);
    std::shared_ptr<int> b = std::shared_ptr<int>(a, &a[0]);
    std::shared_ptr<int> c = std::shared_ptr<int>(a, &a[1]);
    std::atomic<std::shared_ptr<int>> at;
    at.store(b);
    c = at.load(); // Replace this line with the new API, i.e.
at.load_into(c); or c = at;
    assert(c == b); // This had better be true after the load!

So you need to look at both halves of the shared_ptr. I believe this
requires a double-wide atomic load to avoid tearing-esque problems.

> // erase our object pointer
> this->~shared_obj();
>
As Jonathan said, technically, what you want here is
    *this = nullptr;
Also, the following line repoints `m_ctrl`, but where do you actually
repoint the object pointer to point to the same object as `*ts`?

> // copy() increments the reference count for ourself
> m_ctrl = ts.copy();
> return *this;
> }
>

–Arthur