Skip to content

Commit

Permalink
Fix PR27658 - Make ~mutex trivial when possible.
Browse files Browse the repository at this point in the history
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 :-)

llvm-svn: 365273
  • Loading branch information
EricWF committed Jul 7, 2019
1 parent 9e52c43 commit 8baf838
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 20 deletions.
8 changes: 8 additions & 0 deletions libcxx/include/__config
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,14 @@ _LIBCPP_FUNC_VIS extern "C" void __sanitizer_annotate_contiguous_container(
_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
Expand Down
22 changes: 9 additions & 13 deletions libcxx/include/__mutex_base
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
1 change: 1 addition & 0 deletions libcxx/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ set(LIBCXX_SOURCES
locale.cpp
memory.cpp
mutex.cpp
mutex_destructor.cpp
new.cpp
optional.cpp
random.cpp
Expand Down
5 changes: 1 addition & 4 deletions libcxx/src/mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
51 changes: 51 additions & 0 deletions libcxx/src/mutex_destructor.cpp
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

1 comment on commit 8baf838

@pinskia
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

godbolt short link for the goo.gl one: https://godbolt.org/z/YxW44PdjP

Please sign in to comment.