From a5d7c4563d949c66a4e47f4e6456a107bd4a9968 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 31 Mar 2024 06:47:54 -0700 Subject: [PATCH 1/9] `primitives.hpp`: Include `__msvc_threads_core.hpp`. This will allow us to move the definition of `_Cnd_internal_imp_t` into `__msvc_threads_core.hpp`. This is a safe transformation because only 3 files include `primitives.hpp` and they already drag in `__msvc_threads_core.hpp` (`sharedmutex.cpp` directly; `cond.cpp` and `mutex.cpp` via `xthreads.h`). --- stl/src/primitives.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/stl/src/primitives.hpp b/stl/src/primitives.hpp index cd888ab514e..ad0e6af1110 100644 --- a/stl/src/primitives.hpp +++ b/stl/src/primitives.hpp @@ -3,6 +3,7 @@ #pragma once +#include <__msvc_threads_core.hpp> #include #include From 97402c1413a3e625507331e788b0df9ee9b4acc9 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 31 Mar 2024 07:03:07 -0700 Subject: [PATCH 2/9] In `_Cnd_internal_imp_t`, rename its `_Aligned_storage_t` data member `cv` to `_Cv_storage`. This will allow us to move the definition into `stl/inc/__msvc_threads_core.hpp`. The new name is consistent with how `_Mtx_internal_imp_t` names its `_Aligned_storage_t` data member `_Cs_storage`. --- stl/src/primitives.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/src/primitives.hpp b/stl/src/primitives.hpp index ad0e6af1110..054c1eb02b2 100644 --- a/stl/src/primitives.hpp +++ b/stl/src/primitives.hpp @@ -50,11 +50,11 @@ namespace Concurrency { extern "C" { struct _Cnd_internal_imp_t { - std::_Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment> cv; + std::_Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment> _Cv_storage; [[nodiscard]] Concurrency::details::stl_condition_variable_win7* _get_cv() noexcept { // get pointer to implementation - return reinterpret_cast(&cv); + return reinterpret_cast(&_Cv_storage); } }; From c1c7ef0766079fb23da50c954ea99248e2da24fa Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 31 Mar 2024 07:20:31 -0700 Subject: [PATCH 3/9] Change definition: `_Cnd_internal_imp_t::_get_cv` => `Concurrency::details::_Get_cond_var` Change the member function `_Cnd_internal_imp_t::_get_cv` into the non-member function `Concurrency::details::_Get_cond_var`, renamed to be more descriptive. Drop the unnecessary comment `// get pointer to implementation`. Now we don't need to qualify `stl_condition_variable_win7`, defined immediately above. We need to take a parameter `::_Cnd_internal_imp_t* _Cond`, qualified for clarity. We need `inline` for the non-member function. --- stl/src/primitives.hpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/stl/src/primitives.hpp b/stl/src/primitives.hpp index 054c1eb02b2..38dad9087b6 100644 --- a/stl/src/primitives.hpp +++ b/stl/src/primitives.hpp @@ -44,6 +44,9 @@ namespace Concurrency { CONDITION_VARIABLE m_condition_variable = CONDITION_VARIABLE_INIT; }; + [[nodiscard]] inline stl_condition_variable_win7* _Get_cond_var(::_Cnd_internal_imp_t* _Cond) noexcept { + return reinterpret_cast(&_Cond->_Cv_storage); + } } // namespace details } // namespace Concurrency @@ -51,11 +54,6 @@ extern "C" { struct _Cnd_internal_imp_t { std::_Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment> _Cv_storage; - - [[nodiscard]] Concurrency::details::stl_condition_variable_win7* _get_cv() noexcept { - // get pointer to implementation - return reinterpret_cast(&_Cv_storage); - } }; } // extern "C" From 7f328edecec620b09ff4328380618ee22660a254 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 31 Mar 2024 07:22:01 -0700 Subject: [PATCH 4/9] Replace calls: `cond->_get_cv()` => `Concurrency::details::_Get_cond_var(cond)` --- stl/src/cond.cpp | 13 +++++++------ stl/src/sharedmutex.cpp | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/stl/src/cond.cpp b/stl/src/cond.cpp index 38ae9c20a9c..4cb255335fa 100644 --- a/stl/src/cond.cpp +++ b/stl/src/cond.cpp @@ -14,7 +14,7 @@ extern "C" { _CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(const _Cnd_t cond) noexcept { // initialize condition variable in situ - new (cond->_get_cv()) Concurrency::details::stl_condition_variable_win7; + new (Concurrency::details::_Get_cond_var(cond)) Concurrency::details::stl_condition_variable_win7; } _CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t) noexcept {} // destroy condition variable in situ @@ -54,7 +54,7 @@ _CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t mtx) noexcept { // set owner _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx) noexcept { // wait until signaled const auto cs = &mtx->_Critical_section; _Mtx_clear_owner(mtx); - cond->_get_cv()->wait(cs); + Concurrency::details::_Get_cond_var(cond)->wait(cs); _Mtx_reset_owner(mtx); return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds } @@ -66,13 +66,14 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_timedwait( const auto cs = &mtx->_Critical_section; if (target == nullptr) { // no target time specified, wait on mutex _Mtx_clear_owner(mtx); - cond->_get_cv()->wait(cs); + Concurrency::details::_Get_cond_var(cond)->wait(cs); _Mtx_reset_owner(mtx); } else { // target time specified, wait for it _timespec64 now; _Timespec64_get_sys(&now); _Mtx_clear_owner(mtx); - if (!cond->_get_cv()->wait_for(cs, _Xtime_diff_to_millis2(target, &now))) { // report timeout + if (!Concurrency::details::_Get_cond_var(cond)->wait_for( + cs, _Xtime_diff_to_millis2(target, &now))) { // report timeout _Timespec64_get_sys(&now); if (_Xtime_diff_to_millis2(target, &now) == 0) { res = _Thrd_result::_Timedout; @@ -84,12 +85,12 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_timedwait( } _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_signal(const _Cnd_t cond) noexcept { // release one waiting thread - cond->_get_cv()->notify_one(); + Concurrency::details::_Get_cond_var(cond)->notify_one(); return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds } _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_broadcast(const _Cnd_t cond) noexcept { // release all waiting threads - cond->_get_cv()->notify_all(); + Concurrency::details::_Get_cond_var(cond)->notify_all(); return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds } diff --git a/stl/src/sharedmutex.cpp b/stl/src/sharedmutex.cpp index 73dbf6b2888..ef5e53f7450 100644 --- a/stl/src/sharedmutex.cpp +++ b/stl/src/sharedmutex.cpp @@ -50,7 +50,7 @@ _Thrd_result __stdcall _Cnd_timedwait_for(const _Cnd_t cond, const _Mtx_t mtx, c mtx->_Thread_id = -1; --mtx->_Count; - if (!cond->_get_cv()->wait_for(cs, target_ms)) { // report timeout + if (!Concurrency::details::_Get_cond_var(cond)->wait_for(cs, target_ms)) { // report timeout const auto end_ms = GetTickCount64(); if (end_ms - start_ms >= target_ms) { res = _Thrd_result::_Timedout; From 60b1db5257bb79307a7890e90c0a300e4227772a Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 31 Mar 2024 07:26:39 -0700 Subject: [PATCH 5/9] Move the definition of `_Cnd_internal_imp_t` from `primitives.hpp` to `__msvc_threads_core.hpp`. This is still within `extern "C"`. Change `std::` => `_STD`. We no longer need the wacky `/clr` forward declaration workaround. --- stl/inc/__msvc_threads_core.hpp | 9 ++++----- stl/src/primitives.hpp | 8 -------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/stl/inc/__msvc_threads_core.hpp b/stl/inc/__msvc_threads_core.hpp index 27cc751b442..89693d76051 100644 --- a/stl/inc/__msvc_threads_core.hpp +++ b/stl/inc/__msvc_threads_core.hpp @@ -71,12 +71,11 @@ _INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = alignof(void*); using _Mtx_t = _Mtx_internal_imp_t*; -#ifdef _M_CEE // avoid warning LNK4248: unresolved typeref token for '_Cnd_internal_imp_t'; image may not run -using _Cnd_t = void*; -#else // ^^^ defined(_M_CEE) / !defined(_M_CEE) vvv -struct _Cnd_internal_imp_t; +struct _Cnd_internal_imp_t { + _STD _Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment> _Cv_storage; +}; + using _Cnd_t = _Cnd_internal_imp_t*; -#endif // ^^^ !defined(_M_CEE) ^^^ } // extern "C" #pragma pop_macro("new") diff --git a/stl/src/primitives.hpp b/stl/src/primitives.hpp index 38dad9087b6..ff9b5937256 100644 --- a/stl/src/primitives.hpp +++ b/stl/src/primitives.hpp @@ -49,11 +49,3 @@ namespace Concurrency { } } // namespace details } // namespace Concurrency - -extern "C" { - -struct _Cnd_internal_imp_t { - std::_Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment> _Cv_storage; -}; - -} // extern "C" From 22e890455ce09c3ec5309d92d4f0e297f6d6982a Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 31 Mar 2024 07:33:40 -0700 Subject: [PATCH 6/9] Directly store `_Cnd_internal_imp_t _Cnd_storage`. We can do this because its only data member is exactly `_Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment>`, so the representation is unchanged. Then we can simplify the `_Mycnd()` member function. This follows the same pattern as `_Mutex_base`'s `_Mtx_internal_imp_t _Mtx_storage` and `_Mymtx()`. --- stl/inc/condition_variable | 6 +++--- stl/inc/mutex | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index 9878204252f..93f84507535 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -220,10 +220,10 @@ public: private: shared_ptr _Myptr; - _Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment> _Cnd_storage; + _Cnd_internal_imp_t _Cnd_storage; - _NODISCARD _Cnd_t _Mycnd() noexcept { // get pointer to _Cnd_internal_imp_t inside _Cnd_storage - return reinterpret_cast<_Cnd_t>(&_Cnd_storage); + _NODISCARD _Cnd_t _Mycnd() noexcept { + return &_Cnd_storage; } template diff --git a/stl/inc/mutex b/stl/inc/mutex index 42544d2204b..49291830219 100644 --- a/stl/inc/mutex +++ b/stl/inc/mutex @@ -637,10 +637,10 @@ public: } private: - _Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment> _Cnd_storage; + _Cnd_internal_imp_t _Cnd_storage; - _Cnd_t _Mycnd() noexcept { // get pointer to _Cnd_internal_imp_t inside _Cnd_storage - return reinterpret_cast<_Cnd_t>(&_Cnd_storage); + _Cnd_t _Mycnd() noexcept { + return &_Cnd_storage; } }; From 810e6abad736342aec4a5b7f2cd9e9928b8857b9 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 31 Mar 2024 07:40:33 -0700 Subject: [PATCH 7/9] Move `_Cnd_internal_imp_size`/`_Cnd_internal_imp_alignment` within `_Cnd_internal_imp_t`. This changes `_INLINE_VAR` to `static`. The comment `// Size and alignment for _Cnd_internal_imp_t` is now unnecessary and can be dropped. --- stl/inc/__msvc_threads_core.hpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/stl/inc/__msvc_threads_core.hpp b/stl/inc/__msvc_threads_core.hpp index 89693d76051..322259f395e 100644 --- a/stl/inc/__msvc_threads_core.hpp +++ b/stl/inc/__msvc_threads_core.hpp @@ -58,20 +58,19 @@ struct _Mtx_internal_imp_t { int _Count{}; }; -// Size and alignment for _Cnd_internal_imp_t +using _Mtx_t = _Mtx_internal_imp_t*; + +struct _Cnd_internal_imp_t { #if defined(_CRT_WINDOWS) // for Windows-internal code -_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 2 * sizeof(void*); + static constexpr size_t _Cnd_internal_imp_size = 2 * sizeof(void*); #elif defined(_WIN64) // ordinary 64-bit code -_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 72; + static constexpr size_t _Cnd_internal_imp_size = 72; #else // vvv ordinary 32-bit code vvv -_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 40; + static constexpr size_t _Cnd_internal_imp_size = 40; #endif // ^^^ ordinary 32-bit code ^^^ -_INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = alignof(void*); + static constexpr size_t _Cnd_internal_imp_alignment = alignof(void*); -using _Mtx_t = _Mtx_internal_imp_t*; - -struct _Cnd_internal_imp_t { _STD _Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment> _Cv_storage; }; From 996908b3bf7a8283f9f1c93844815c682c7b5395 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 31 Mar 2024 07:44:03 -0700 Subject: [PATCH 8/9] Fuse `_Critical_section_align` and `_Cnd_internal_imp_alignment` into their only uses. --- stl/inc/__msvc_threads_core.hpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/stl/inc/__msvc_threads_core.hpp b/stl/inc/__msvc_threads_core.hpp index 322259f395e..78c581794ae 100644 --- a/stl/inc/__msvc_threads_core.hpp +++ b/stl/inc/__msvc_threads_core.hpp @@ -47,12 +47,10 @@ struct _Mtx_internal_imp_t { #endif // ^^^ !defined(_WIN64) ^^^ #endif // ^^^ public STL ^^^ - static constexpr size_t _Critical_section_align = alignof(void*); - int _Type{}; union { _Stl_critical_section _Critical_section{}; - _STD _Aligned_storage_t<_Critical_section_size, _Critical_section_align> _Cs_storage; + _STD _Aligned_storage_t<_Critical_section_size, alignof(void*)> _Cs_storage; }; long _Thread_id{}; int _Count{}; @@ -69,9 +67,7 @@ struct _Cnd_internal_imp_t { static constexpr size_t _Cnd_internal_imp_size = 40; #endif // ^^^ ordinary 32-bit code ^^^ - static constexpr size_t _Cnd_internal_imp_alignment = alignof(void*); - - _STD _Aligned_storage_t<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment> _Cv_storage; + _STD _Aligned_storage_t<_Cnd_internal_imp_size, alignof(void*)> _Cv_storage; }; using _Cnd_t = _Cnd_internal_imp_t*; From be28e7c0f1a26eb4cde7a6174d641eab765a87a9 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 31 Mar 2024 07:48:22 -0700 Subject: [PATCH 9/9] Follow `_Cnd_internal_imp_t`'s simpler pattern in `_Mtx_internal_imp_t`. This expresses "16 or 8 bytes" as `2 * sizeof(void*)`. --- stl/inc/__msvc_threads_core.hpp | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/stl/inc/__msvc_threads_core.hpp b/stl/inc/__msvc_threads_core.hpp index 78c581794ae..94cc7346131 100644 --- a/stl/inc/__msvc_threads_core.hpp +++ b/stl/inc/__msvc_threads_core.hpp @@ -33,19 +33,13 @@ struct _Stl_critical_section { }; struct _Mtx_internal_imp_t { -#if defined(_CRT_WINDOWS) || defined(UNDOCKED_WINDOWS_UCRT) -#ifdef _WIN64 - static constexpr size_t _Critical_section_size = 16; -#else // ^^^ defined(_WIN64) / !defined(_WIN64) vvv - static constexpr size_t _Critical_section_size = 8; -#endif // ^^^ !defined(_WIN64) ^^^ -#else // ^^^ Windows private STL / public STL vvv -#ifdef _WIN64 +#if defined(_CRT_WINDOWS) || defined(UNDOCKED_WINDOWS_UCRT) // for Windows-internal code + static constexpr size_t _Critical_section_size = 2 * sizeof(void*); +#elif defined(_WIN64) // ordinary 64-bit code static constexpr size_t _Critical_section_size = 64; -#else // ^^^ defined(_WIN64) / !defined(_WIN64) vvv +#else // vvv ordinary 32-bit code vvv static constexpr size_t _Critical_section_size = 36; -#endif // ^^^ !defined(_WIN64) ^^^ -#endif // ^^^ public STL ^^^ +#endif // ^^^ ordinary 32-bit code ^^^ int _Type{}; union {