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@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits