[libcxx] r277357 - Improve shared_ptr dtor performance

[Asiri Rathnayake via cfe-commits]

Hi Ben,

Looks like there's a typo in this patch that is causing a buildbot failure:
http://lab.llvm.org:8011/builders/libcxx-libcxxabi-singlethreaded-x86_64-linux-debian/builds/1128/steps/build.libcxx/logs/stdio

Also seen in our local builders. I will fix it if I get around to it before
you :)

Cheers,

/ Asiri



On Mon, Aug 1, 2016 at 6:51 PM, Ben Craig via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: bcraig
> Date: Mon Aug  1 12:51:26 2016
> New Revision: 277357
>
> URL: http://llvm.org/viewvc/llvm-project?rev=277357&view=rev
> Log:
> Improve shared_ptr dtor performance
>
> If the last destruction is uncontended, skip the atomic store on
> __shared_weak_owners_. This shifts some costs from normal
> shared_ptr usage to weak_ptr uses.
>
> https://reviews.llvm.org/D22470
>
> Modified:
>     libcxx/trunk/src/memory.cpp
>
> Modified: libcxx/trunk/src/memory.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/memory.cpp?rev=277357&r1=277356&r2=277357&view=diff
>
> ==============================================================================
> --- libcxx/trunk/src/memory.cpp (original)
> +++ libcxx/trunk/src/memory.cpp Mon Aug  1 12:51:26 2016
> @@ -96,7 +96,35 @@ __shared_weak_count::__release_shared()
>  void
>  __shared_weak_count::__release_weak() _NOEXCEPT
>  {
> -    if (decrement(__shared_weak_owners_) == -1)
> +    // NOTE: The acquire load here is an optimization of the very
> +    // common case where a shared pointer is being destructed while
> +    // having no other contended references.
> +    //
> +    // BENEFIT: We avoid expensive atomic stores like XADD and STREX
> +    // in a common case.  Those instructions are slow and do nasty
> +    // things to caches.
> +    //
> +    // IS THIS SAFE?  Yes.  During weak destruction, if we see that we
> +    // are the last reference, we know that no-one else is accessing
> +    // us. If someone were accessing us, then they would be doing so
> +    // while the last shared / weak_ptr was being destructed, and
> +    // that's undefined anyway.
> +    //
> +    // If we see anything other than a 0, then we have possible
> +    // contention, and need to use an atomicrmw primitive.
> +    // The same arguments don't apply for increment, where it is legal
> +    // (though inadvisable) to share shared_ptr references between
> +    // threads, and have them all get copied at once.  The argument
> +    // also doesn't apply for __release_shared, because an outstanding
> +    // weak_ptr::lock() could read / modify the shared count.
> +    if (__libcpp_atomic_load(&__shared_weak_owners_, _AO_Aquire) == 0)
> +    {
> +        // no need to do this store, because we are about
> +        // to destroy everything.
> +        //__libcpp_atomic_store(&__shared_weak_owners_, -1, _AO_Release);
> +        __on_zero_shared_weak();
> +    }
> +    else if (decrement(__shared_weak_owners_) == -1)
>          __on_zero_shared_weak();
>  }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160802/760fb46e/attachment.html>