[libcxx] r292184 - [Test patch] Inline hot functions in libcxx shared_ptr
Author: hxy9243
Date: Mon Jan 16 20:46:33 2017
New Revision: 292184
URL: http://llvm.org/viewvc/llvm-project?rev=292184=rev
Log:
[Test patch] Inline hot functions in libcxx shared_ptr
Moves hot functions such as atomic add into the memory header file
so that they can be inlined, which brings performance benefits.
Patch by Kevin Hu, Aditya Kumar, Sebastian Pop
Differential Revision: https://reviews.llvm.org/D24991
Modified:
libcxx/trunk/include/memory
libcxx/trunk/src/memory.cpp
Modified: libcxx/trunk/include/memory
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/memory?rev=292184=292183=292184=diff
==
--- libcxx/trunk/include/memory (original)
+++ libcxx/trunk/include/memory Mon Jan 16 20:46:33 2017
@@ -3681,6 +3681,39 @@ uninitialized_move_n(_InputIt __first, _
#endif // _LIBCPP_STD_VER > 14
+// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
+// should be sufficient for thread safety.
+// See https://llvm.org/bugs/show_bug.cgi?id=22803
+#if defined(__clang__) && __has_builtin(__atomic_add_fetch) \
+ && defined(__ATOMIC_RELAXED) \
+ && defined(__ATOMIC_ACQ_REL)
+# define _LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT
+#elif !defined(__clang__) && defined(_GNUC_VER) && _GNUC_VER >= 407
+# define _LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT
+#endif
+
+template
+inline _LIBCPP_INLINE_VISIBILITY _Tp
+__libcpp_atomic_refcount_increment(_Tp& __t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT) &&
!defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(&__t, 1, __ATOMIC_RELAXED);
+#else
+return __t += 1;
+#endif
+}
+
+template
+inline _LIBCPP_INLINE_VISIBILITY _Tp
+__libcpp_atomic_refcount_decrement(_Tp& __t) _NOEXCEPT
+{
+#if defined(_LIBCPP_HAS_BUILTIN_ATOMIC_SUPPORT) &&
!defined(_LIBCPP_HAS_NO_THREADS)
+return __atomic_add_fetch(&__t, -1, __ATOMIC_ACQ_REL);
+#else
+return __t -= 1;
+#endif
+}
+
class _LIBCPP_EXCEPTION_ABI bad_weak_ptr
: public std::exception
{
@@ -3717,8 +3750,23 @@ public:
explicit __shared_count(long __refs = 0) _NOEXCEPT
: __shared_owners_(__refs) {}
-void __add_shared() _NOEXCEPT;
-bool __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+_LIBCPP_FUNC_VIS void __add_shared() _NOEXCEPT;
+_LIBCPP_FUNC_VIS bool __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+ __libcpp_atomic_refcount_increment(__shared_owners_);
+}
+_LIBCPP_INLINE_VISIBILITY
+bool __release_shared() _NOEXCEPT {
+ if (__libcpp_atomic_refcount_decrement(__shared_owners_) == -1) {
+__on_zero_shared();
+return true;
+ }
+ return false;
+}
+#endif
_LIBCPP_INLINE_VISIBILITY
long use_count() const _NOEXCEPT {
return __libcpp_relaxed_load(&__shared_owners_) + 1;
@@ -3739,9 +3787,25 @@ protected:
virtual ~__shared_weak_count();
public:
-void __add_shared() _NOEXCEPT;
-void __add_weak() _NOEXCEPT;
-void __release_shared() _NOEXCEPT;
+#ifdef _LIBCPP_BUILDING_MEMORY
+_LIBCPP_FUNC_VIS void __add_shared() _NOEXCEPT;
+_LIBCPP_FUNC_VIS void __add_weak() _NOEXCEPT;
+_LIBCPP_FUNC_VIS void __release_shared() _NOEXCEPT;
+#else
+_LIBCPP_INLINE_VISIBILITY
+void __add_shared() _NOEXCEPT {
+ __shared_count::__add_shared();
+}
+_LIBCPP_INLINE_VISIBILITY
+void __add_weak() _NOEXCEPT {
+ __libcpp_atomic_refcount_increment(__shared_weak_owners_);
+}
+_LIBCPP_INLINE_VISIBILITY
+void __release_shared() _NOEXCEPT {
+ if (__shared_count::__release_shared())
+__release_weak();
+}
+#endif
void __release_weak() _NOEXCEPT;
_LIBCPP_INLINE_VISIBILITY
long use_count() const _NOEXCEPT {return __shared_count::use_count();}
Modified: libcxx/trunk/src/memory.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/memory.cpp?rev=292184=292183=292184=diff
==
--- libcxx/trunk/src/memory.cpp (original)
+++ libcxx/trunk/src/memory.cpp Mon Jan 16 20:46:33 2017
@@ -17,28 +17,6 @@
_LIBCPP_BEGIN_NAMESPACE_STD
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-} // namespace
-
const allocator_arg_t allocator_arg = allocator_arg_t();
bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
@@ -53,16 +31,20 @@ __shared_count::~__shared_count()
{
}
+__shared_weak_count::~__shared_weak_count()
+{
+}
+
void
__shared_count::__add_shared()
[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.
hxy9243 added a comment. Ping. @mclow.lists, @EricWF, any ideas on this patch? Thanks very much! https://reviews.llvm.org/D24991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.
hxy9243 added a comment. In https://reviews.llvm.org/D24991#586248, @kubabrecka wrote: > In https://reviews.llvm.org/D24991#586219, @sebpop wrote: > > > I just ran ninja check-all with and without this patch and there are no > > regressions in compiler-rt on an x86_64-linux machine. > > > The TSan interceptors (and testcases) are Darwin-only at this point. I'll > run the tests on my machine. Any updates on the testing? Thanks very much! https://reviews.llvm.org/D24991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.
hxy9243 accepted this revision. hxy9243 added a comment. This revision is now accepted and ready to land. Looks good to me. Notice that the performance gain can only be observed when compiled with the updated C++ header files. https://reviews.llvm.org/D24991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.
hxy9243 added a comment. Thanks for pointing out. It's true that it may cause ABI breakage. It would be nice to keep compatibility while getting the performance benefits from inlining. I've tested the patch with google-benchmark/util_smartptr_libcxx shipped with libcxx on x86_64 server, and attached the results as following: BASE libcxx r283113: $ taskset -c 23 ./util_smartptr.libcxx.out Run on (24 X 1200 MHz CPU s) 2016-10-12 13:52:03 ***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead. Benchmark Time CPU Iterations BM_SharedPtrCreateDestroy 54 ns 54 ns 12388755 BM_SharedPtrIncDecRef 37 ns 37 ns 19021739 BM_WeakPtrIncDecRef 38 ns 38 ns 18421053 libcxx with patch: $ taskset -c 23 ./util_smartptr.libcxx.out Run on (24 X 1200 MHz CPU s) 2016-10-12 13:48:38 ***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead. Benchmark Time CPU Iterations BM_SharedPtrCreateDestroy 44 ns 44 ns 14730639 BM_SharedPtrIncDecRef 18 ns 18 ns 3889 BM_WeakPtrIncDecRef 30 ns 30 ns 23648649 Repository: rL LLVM https://reviews.llvm.org/D24991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.
hxy9243 updated this revision to Diff 73293.
hxy9243 added a comment.
Addresses comments from @halyavin, rename "atomic_support.h" to
"__atomic_support" to avoid collisions with application headers.
Repository:
rL LLVM
https://reviews.llvm.org/D24991
Files:
libcxx/include/__atomic_support
libcxx/include/memory
libcxx/src/include/atomic_support.h
libcxx/src/memory.cpp
libcxx/src/mutex.cpp
Index: libcxx/src/mutex.cpp
===
--- libcxx/src/mutex.cpp
+++ libcxx/src/mutex.cpp
@@ -12,7 +12,7 @@
#include "limits"
#include "system_error"
#include "cassert"
-#include "include/atomic_support.h"
+#include "__atomic_support"
_LIBCPP_BEGIN_NAMESPACE_STD
#ifndef _LIBCPP_HAS_NO_THREADS
Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -13,85 +13,17 @@
#include "mutex"
#include "thread"
#endif
-#include "include/atomic_support.h"
+#include "__atomic_support"
_LIBCPP_BEGIN_NAMESPACE_STD
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-} // namespace
-
const allocator_arg_t allocator_arg = allocator_arg_t();
bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
-const char*
-bad_weak_ptr::what() const _NOEXCEPT
-{
-return "bad_weak_ptr";
-}
+__shared_count::~__shared_count() {}
-__shared_count::~__shared_count()
-{
-}
-
-void
-__shared_count::__add_shared() _NOEXCEPT
-{
-increment(__shared_owners_);
-}
-
-bool
-__shared_count::__release_shared() _NOEXCEPT
-{
-if (decrement(__shared_owners_) == -1)
-{
-__on_zero_shared();
-return true;
-}
-return false;
-}
-
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
-void
-__shared_weak_count::__add_shared() _NOEXCEPT
-{
-__shared_count::__add_shared();
-}
-
-void
-__shared_weak_count::__add_weak() _NOEXCEPT
-{
-increment(__shared_weak_owners_);
-}
-
-void
-__shared_weak_count::__release_shared() _NOEXCEPT
-{
-if (__shared_count::__release_shared())
-__release_weak();
-}
+__shared_weak_count::~__shared_weak_count() {}
void
__shared_weak_count::__release_weak() _NOEXCEPT
Index: libcxx/src/include/atomic_support.h
===
--- libcxx/src/include/atomic_support.h
+++ /dev/null
@@ -1,158 +0,0 @@
-//===--===
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===
-
-#ifndef ATOMIC_SUPPORT_H
-#define ATOMIC_SUPPORT_H
-
-#include "__config"
-#include "memory" // for __libcpp_relaxed_load
-
-#if defined(__clang__) && __has_builtin(__atomic_load_n) \
- && __has_builtin(__atomic_store_n)\
- && __has_builtin(__atomic_add_fetch) \
- && __has_builtin(__atomic_compare_exchange_n) \
- && defined(__ATOMIC_RELAXED) \
- && defined(__ATOMIC_CONSUME) \
- && defined(__ATOMIC_ACQUIRE) \
- && defined(__ATOMIC_RELEASE) \
- && defined(__ATOMIC_ACQ_REL) \
- && defined(__ATOMIC_SEQ_CST)
-# define _LIBCPP_HAS_ATOMIC_BUILTINS
-#elif !defined(__clang__) && defined(_GNUC_VER) && _GNUC_VER >= 407
-# define _LIBCPP_HAS_ATOMIC_BUILTINS
-#endif
-
-#if !defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-# if defined(_MSC_VER) && !defined(__clang__)
-_LIBCPP_WARNING("Building libc++ without __atomic builtins is unsupported")
-# else
-# warning Building libc++ without __atomic builtins is unsupported
-# endif
-#endif
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-namespace {
-
-#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-
-enum __libcpp_atomic_order {
-_AO_Relaxed = __ATOMIC_RELAXED,
-_AO_Consume = __ATOMIC_CONSUME,
-_AO_Acquire = __ATOMIC_ACQUIRE,
-_AO_Release = __ATOMIC_RELEASE,
-_AO_Acq_Rel = __ATOMIC_ACQ_REL,
-_AO_Seq = __ATOMIC_SEQ_CST
-};
-
-template
-inline _LIBCPP_INLINE_VISIBILITY
-void __libcpp_atomic_store(_ValueType* __dest, _FromType __val,
- int __order = _AO_Seq)
-{
-__atomic_store_n(__dest, __val, __order);
-}
-
-template
[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.
hxy9243 created this revision.
hxy9243 added reviewers: sebpop, hiraditya, wmi.
hxy9243 added a subscriber: cfe-commits.
hxy9243 set the repository for this revision to rL LLVM.
This patch moves some existing functions from the memory.cpp to the memory
header file, so that they could be properly inlined, which gives potential
optimization opportunities and performance benefits.
Repository:
rL LLVM
https://reviews.llvm.org/D24991
Files:
libcxx/include/atomic_support.h
libcxx/include/memory
libcxx/src/include/atomic_support.h
libcxx/src/memory.cpp
libcxx/src/mutex.cpp
Index: libcxx/src/mutex.cpp
===
--- libcxx/src/mutex.cpp
+++ libcxx/src/mutex.cpp
@@ -12,7 +12,7 @@
#include "limits"
#include "system_error"
#include "cassert"
-#include "include/atomic_support.h"
+#include "atomic_support.h"
_LIBCPP_BEGIN_NAMESPACE_STD
#ifndef _LIBCPP_HAS_NO_THREADS
Index: libcxx/src/memory.cpp
===
--- libcxx/src/memory.cpp
+++ libcxx/src/memory.cpp
@@ -13,85 +13,17 @@
#include "mutex"
#include "thread"
#endif
-#include "include/atomic_support.h"
+#include "atomic_support.h"
_LIBCPP_BEGIN_NAMESPACE_STD
-namespace
-{
-
-// NOTE: Relaxed and acq/rel atomics (for increment and decrement respectively)
-// should be sufficient for thread safety.
-// See https://llvm.org/bugs/show_bug.cgi?id=22803
-template
-inline T
-increment(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, 1, _AO_Relaxed);
-}
-
-template
-inline T
-decrement(T& t) _NOEXCEPT
-{
-return __libcpp_atomic_add(, -1, _AO_Acq_Rel);
-}
-
-} // namespace
-
const allocator_arg_t allocator_arg = allocator_arg_t();
bad_weak_ptr::~bad_weak_ptr() _NOEXCEPT {}
-const char*
-bad_weak_ptr::what() const _NOEXCEPT
-{
-return "bad_weak_ptr";
-}
+__shared_count::~__shared_count() {}
-__shared_count::~__shared_count()
-{
-}
-
-void
-__shared_count::__add_shared() _NOEXCEPT
-{
-increment(__shared_owners_);
-}
-
-bool
-__shared_count::__release_shared() _NOEXCEPT
-{
-if (decrement(__shared_owners_) == -1)
-{
-__on_zero_shared();
-return true;
-}
-return false;
-}
-
-__shared_weak_count::~__shared_weak_count()
-{
-}
-
-void
-__shared_weak_count::__add_shared() _NOEXCEPT
-{
-__shared_count::__add_shared();
-}
-
-void
-__shared_weak_count::__add_weak() _NOEXCEPT
-{
-increment(__shared_weak_owners_);
-}
-
-void
-__shared_weak_count::__release_shared() _NOEXCEPT
-{
-if (__shared_count::__release_shared())
-__release_weak();
-}
+__shared_weak_count::~__shared_weak_count() {}
void
__shared_weak_count::__release_weak() _NOEXCEPT
Index: libcxx/src/include/atomic_support.h
===
--- libcxx/src/include/atomic_support.h
+++ /dev/null
@@ -1,158 +0,0 @@
-//===--===
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===
-
-#ifndef ATOMIC_SUPPORT_H
-#define ATOMIC_SUPPORT_H
-
-#include "__config"
-#include "memory" // for __libcpp_relaxed_load
-
-#if defined(__clang__) && __has_builtin(__atomic_load_n) \
- && __has_builtin(__atomic_store_n)\
- && __has_builtin(__atomic_add_fetch) \
- && __has_builtin(__atomic_compare_exchange_n) \
- && defined(__ATOMIC_RELAXED) \
- && defined(__ATOMIC_CONSUME) \
- && defined(__ATOMIC_ACQUIRE) \
- && defined(__ATOMIC_RELEASE) \
- && defined(__ATOMIC_ACQ_REL) \
- && defined(__ATOMIC_SEQ_CST)
-# define _LIBCPP_HAS_ATOMIC_BUILTINS
-#elif !defined(__clang__) && defined(_GNUC_VER) && _GNUC_VER >= 407
-# define _LIBCPP_HAS_ATOMIC_BUILTINS
-#endif
-
-#if !defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-# if defined(_MSC_VER) && !defined(__clang__)
-_LIBCPP_WARNING("Building libc++ without __atomic builtins is unsupported")
-# else
-# warning Building libc++ without __atomic builtins is unsupported
-# endif
-#endif
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-namespace {
-
-#if defined(_LIBCPP_HAS_ATOMIC_BUILTINS) && !defined(_LIBCPP_HAS_NO_THREADS)
-
-enum __libcpp_atomic_order {
-_AO_Relaxed = __ATOMIC_RELAXED,
-_AO_Consume = __ATOMIC_CONSUME,
-_AO_Acquire = __ATOMIC_ACQUIRE,
-_AO_Release = __ATOMIC_RELEASE,
-_AO_Acq_Rel = __ATOMIC_ACQ_REL,
-_AO_Seq = __ATOMIC_SEQ_CST
-};
-
-template
-inline
