[libcxx-commits] [libcxx] r365273 - Fix PR27658 - Make ~mutex trivial when possible.
Nico Weber via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Jul 7 07:11:29 PDT 2019
Do you know if this optimization works on Windows too?
On Sat, Jul 6, 2019 at 9:21 PM Eric Fiselier via libcxx-commits <
libcxx-commits at lists.llvm.org> wrote:
> Author: ericwf
> Date: Sat Jul 6 18:20:54 2019
> New Revision: 365273
>
> URL: http://llvm.org/viewvc/llvm-project?rev=365273&view=rev
> Log:
> Fix PR27658 - Make ~mutex trivial when possible.
>
> Currently std::mutex has a constexpr constructor, but a non-trivial
> destruction.
>
> The constexpr constructor is required to ensure the construction of a
> mutex with static storage duration happens at compile time, during
> constant initialization, and not during dynamic initialization.
> This means that static mutex's are always initialized and can be used
> safely during dynamic initialization without the "static initialization
> order fiasco".
>
> A trivial destructor is important for similar reasons. If a mutex is
> used during dynamic initialization it might also be used during program
> termination. If a static mutex has a non-trivial destructor it will be
> invoked during termination. This can introduce the "static
> deinitialization order fiasco".
>
> Additionally, function-local statics emit a guard variable around
> non-trivially destructible types. This results in horrible codegen and
> adds a runtime cost to every call to that function. non-local static's
> also result in slightly worse codegen but it's not as big of a problem.
>
> Example codegen can be found here: https://goo.gl/3CSzbM
>
> Note: This optimization is not safe with every pthread implementation.
> Some implementations allocate on the first call to pthread_mutex_lock
> and free the allocation in pthread_mutex_destroy.
>
> Also, changing the triviality of the destructor is not an ABI break.
> At least to the best of my knowledge :-)
>
> Added:
> libcxx/trunk/src/mutex_destructor.cpp
> Modified:
> libcxx/trunk/include/__config
> libcxx/trunk/include/__mutex_base
> libcxx/trunk/src/CMakeLists.txt
> libcxx/trunk/src/mutex.cpp
>
> libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp
>
> Modified: libcxx/trunk/include/__config
> URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=365273&r1=365272&r2=365273&view=diff
>
> ==============================================================================
> --- libcxx/trunk/include/__config (original)
> +++ libcxx/trunk/include/__config Sat Jul 6 18:20:54 2019
> @@ -1097,6 +1097,14 @@ _LIBCPP_FUNC_VIS extern "C" void __sanit
> _LIBCPP_HAS_NO_THREADS is defined.
> #endif
>
> +// The Apple, glibc, and Bionic implementation of pthreads implements
> +// pthread_mutex_destroy as nop for regular mutexes.
> +// TODO(EricWF): Enable this optimization on Apple and Bionic platforms
> after
> +// speaking to their respective stakeholders.
> +#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) && defined(__GLIBC__)
> +# define _LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION
> +#endif
> +
> // Systems that use capability-based security (FreeBSD with Capsicum,
> // Nuxi CloudABI) may only provide local filesystem access (using *at()).
> // Functions like open(), rename(), unlink() and stat() should not be
>
> Modified: libcxx/trunk/include/__mutex_base
> URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__mutex_base?rev=365273&r1=365272&r2=365273&view=diff
>
> ==============================================================================
> --- libcxx/trunk/include/__mutex_base (original)
> +++ libcxx/trunk/include/__mutex_base Sat Jul 6 18:20:54 2019
> @@ -36,28 +36,24 @@ _LIBCPP_BEGIN_NAMESPACE_STD
> # endif
> #endif // _LIBCPP_THREAD_SAFETY_ANNOTATION
>
> +
> class _LIBCPP_TYPE_VIS
> _LIBCPP_THREAD_SAFETY_ANNOTATION(capability("mutex")) mutex
> {
> -#ifndef _LIBCPP_CXX03_LANG
> __libcpp_mutex_t __m_ = _LIBCPP_MUTEX_INITIALIZER;
> -#else
> - __libcpp_mutex_t __m_;
> -#endif
>
> public:
> _LIBCPP_INLINE_VISIBILITY
> -#ifndef _LIBCPP_CXX03_LANG
> - constexpr mutex() = default;
> + _LIBCPP_CONSTEXPR mutex() = default;
> +
> + mutex(const mutex&) = delete;
> + mutex& operator=(const mutex&) = delete;
> +
> +#if defined(_LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION)
> + ~mutex() = default;
> #else
> - mutex() _NOEXCEPT {__m_ =
> (__libcpp_mutex_t)_LIBCPP_MUTEX_INITIALIZER;}
> -#endif
> ~mutex() _NOEXCEPT;
> +#endif
>
> -private:
> - mutex(const mutex&);// = delete;
> - mutex& operator=(const mutex&);// = delete;
> -
> -public:
> void lock() _LIBCPP_THREAD_SAFETY_ANNOTATION(acquire_capability());
> bool try_lock() _NOEXCEPT
> _LIBCPP_THREAD_SAFETY_ANNOTATION(try_acquire_capability(true));
> void unlock() _NOEXCEPT
> _LIBCPP_THREAD_SAFETY_ANNOTATION(release_capability());
>
> Modified: libcxx/trunk/src/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/CMakeLists.txt?rev=365273&r1=365272&r2=365273&view=diff
>
> ==============================================================================
> --- libcxx/trunk/src/CMakeLists.txt (original)
> +++ libcxx/trunk/src/CMakeLists.txt Sat Jul 6 18:20:54 2019
> @@ -22,6 +22,7 @@ set(LIBCXX_SOURCES
> locale.cpp
> memory.cpp
> mutex.cpp
> + mutex_destructor.cpp
> new.cpp
> optional.cpp
> random.cpp
>
> Modified: libcxx/trunk/src/mutex.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/mutex.cpp?rev=365273&r1=365272&r2=365273&view=diff
>
> ==============================================================================
> --- libcxx/trunk/src/mutex.cpp (original)
> +++ libcxx/trunk/src/mutex.cpp Sat Jul 6 18:20:54 2019
> @@ -25,10 +25,7 @@ const defer_lock_t defer_lock = {};
> const try_to_lock_t try_to_lock = {};
> const adopt_lock_t adopt_lock = {};
>
> -mutex::~mutex() _NOEXCEPT
> -{
> - __libcpp_mutex_destroy(&__m_);
> -}
> +// ~mutex is defined elsewhere
>
> void
> mutex::lock()
>
> Added: libcxx/trunk/src/mutex_destructor.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/mutex_destructor.cpp?rev=365273&view=auto
>
> ==============================================================================
> --- libcxx/trunk/src/mutex_destructor.cpp (added)
> +++ libcxx/trunk/src/mutex_destructor.cpp Sat Jul 6 18:20:54 2019
> @@ -0,0 +1,51 @@
> +//===--------------------- mutex_destructor.cpp
> ---------------------------===//
> +//
> +// Part of the LLVM Project, under the Apache License v2.0 with LLVM
> Exceptions.
> +// See https://llvm.org/LICENSE.txt for license information.
> +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
> +//
>
> +//===----------------------------------------------------------------------===//
> +
> +// Define ~mutex.
> +//
> +// On some platforms ~mutex has been made trivial and the definition is
> only
> +// provided for ABI compatibility.
> +//
> +// In order to avoid ODR violations within libc++ itself, we need to
> ensure
> +// that *nothing* sees the non-trivial mutex declaration. For this reason
> +// we re-declare the entire class in this file instead of using
> +// _LIBCPP_BUILDING_LIBRARY to change the definition in the headers.
> +
> +#include "__config"
> +#include "__threading_support"
> +
> +#if !defined(_LIBCPP_HAS_NO_THREADS)
> +#if _LIBCPP_ABI_VERSION == 1 ||
> !defined(_LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION)
> +#define NEEDS_MUTEX_DESTRUCTOR
> +#endif
> +#endif
> +
> +_LIBCPP_BEGIN_NAMESPACE_STD
> +
> +#ifdef NEEDS_MUTEX_DESTRUCTOR
> +class _LIBCPP_TYPE_VIS mutex
> +{
> + __libcpp_mutex_t __m_ = _LIBCPP_MUTEX_INITIALIZER;
> +
> +public:
> + _LIBCPP_ALWAYS_INLINE _LIBCPP_INLINE_VISIBILITY
> + constexpr mutex() = default;
> + mutex(const mutex&) = delete;
> + mutex& operator=(const mutex&) = delete;
> + ~mutex() noexcept;
> +};
> +
> +
> +mutex::~mutex() _NOEXCEPT
> +{
> + __libcpp_mutex_destroy(&__m_);
> +}
> +
> +#endif // !_LIBCPP_HAS_NO_THREADS
> +_LIBCPP_END_NAMESPACE_STD
> +
>
> Modified:
> libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp?rev=365273&r1=365272&r2=365273&view=diff
>
> ==============================================================================
> ---
> libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp
> (original)
> +++
> libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.mutex.requirements.mutex/thread.mutex.class/default.pass.cpp
> Sat Jul 6 18:20:54 2019
> @@ -21,8 +21,8 @@
>
> int main(int, char**)
> {
> -
> static_assert(std::is_nothrow_default_constructible<std::mutex>::value, "");
> - std::mutex m;
> -
> + static_assert(std::is_nothrow_default_constructible<std::mutex>::value,
> "");
> + std::mutex m;
> + ((void)m);
> return 0;
> }
>
>
> _______________________________________________
> libcxx-commits mailing list
> libcxx-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190707/11a12b90/attachment-0001.html>
More information about the libcxx-commits
mailing list