[PATCH] [libstdc++] Refactor/cleanup of atomic wait implementation

Jonathan Wakely jwakely@redhat.com
Tue Apr 20 14:25:36 GMT 2021


On 20/04/21 12:41 +0100, Jonathan Wakely wrote:
>On 20/04/21 12:04 +0100, Jonathan Wakely wrote:
>>On 19/04/21 12:23 -0700, Thomas Rodgers wrote:
>>>From: Thomas Rodgers <rodgert@twrodgers.com>
>>>
>>>This patch address jwakely's feedback from 2021-04-15.
>>>
>>>This is a substantial rewrite of the atomic wait/notify (and timed wait
>>>counterparts) implementation.
>>>
>>>The previous __platform_wait looped on EINTR however this behavior is
>>>not required by the standard. A new _GLIBCXX_HAVE_PLATFORM_WAIT macro
>>>now controls whether wait/notify are implemented using a platform
>>>specific primitive or with a platform agnostic mutex/condvar. This
>>>patch only supplies a definition for linux futexes. A future update
>>>could add support __ulock_wait/wake on Darwin, for instance.
>>>
>>>The members of __waiters were lifted to a new base class. The members
>>>are now arranged such that overall sizeof(__waiters_base) fits in two
>>>cache lines (on platforms with at least 64 byte cache lines). The
>>>definition will also use destructive_interference_size for this if it
>>>is available.
>>>
>>>The __waiters type is now specific to untimed waits. Timed waits have a
>>>corresponding __timed_waiters type. Much of the code has been moved from
>>>the previous __atomic_wait() free function to the __waiter_base template
>>>and a __waiter derived type is provided to implement the un-timed wait
>>>operations. A similar change has been made to the timed wait
>>>implementation.
>>>
>>>The __atomic_spin code has been extended to take a spin policy which is
>>>invoked after the initial busy wait loop. The default policy is to
>>>return from the spin. The timed wait code adds a timed backoff spinning
>>>policy. The code from <thread> which implements this_thread::sleep_for,
>>>sleep_until has been moved to a new <bits/std_thread_sleep.h> header
>>
>>The commit msg wasn't updated for the latest round of changes
>>(this_thread_sleep, __waiters_pool_base etc).
>>
>>>which allows the thread sleep code to be consumed without pulling in the
>>>whole of <thread>.
>>>
>>>The entry points into the wait/notify code have been restructured to
>>>support either -
>>> * Testing the current value of the atomic stored at the given address
>>>   and waiting on a notification.
>>> * Applying a predicate to determine if the wait was satisfied.
>>>The entry points were renamed to make it clear that the wait and wake
>>>operations operate on addresses. The first variant takes the expected
>>>value and a function which returns the current value that should be used
>>>in comparison operations, these operations are named with a _v suffix
>>>(e.g. 'value'). All atomic<_Tp> wait/notify operations use the first
>>>variant. Barriers, latches and semaphores use the predicate variant.
>>>
>>>This change also centralizes what it means to compare values for the
>>>purposes of atomic<T>::wait rather than scattering through individual
>>>predicates.
>>>
>>>This change also centralizes the repetitive code which adjusts for
>>>different user supplied clocks (this should be moved elsewhere
>>>and all such adjustments should use a common implementation).
>>>
>>>This change also removes the hashing of the pointer and uses
>>>the pointer value directly for indexing into the waiters table.
>>>
>>>libstdc++-v3/ChangeLog:
>>>	* include/Makefile.am: Add new <bits/std_thread_sleep.h> header.
>>
>>The name needs updating to correspond to the latest version of the
>>patch.
>>
>>>	* include/Makefile.in: Regenerate.
>>>	* include/bits/atomic_base.h: Adjust all calls
>>>	to __atomic_wait/__atomic_notify for new call signatures.
>>>	* include/bits/atomic_wait.h: Extensive rewrite.
>>>	* include/bits/atomic_timed_wait.h: Likewise.
>>>	* include/bits/semaphore_base.h: Adjust all calls
>>>	to __atomic_wait/__atomic_notify for new call signatures.
>>>	* include/bits/this_thread_sleep.h: New file.
>>>	* include/std/atomic: Likewise.
>>>	* include/std/barrier: Likewise.
>>>	* include/std/latch: Likewise.
>>
>>include/std/thread is missing from the changelog entry. You can use
>>the 'git gcc-verify' alias to check your commit log will be accepted
>>by the server-side hook:
>>
>>'gcc-verify' is aliased to '!f() { "`git rev-parse --show-toplevel`/contrib/gcc-changelog/git_check_commit.py" $@; } ; f'
>>
>>
>>>	* testsuite/29_atomics/atomic/wait_notify/bool.cc: Simplify
>>>	test.
>>>	* testsuite/29_atomics/atomic/wait_notify/generic.cc: Likewise.
>>>	* testsuite/29_atomics/atomic/wait_notify/pointers.cc: Likewise.
>>>	* testsuite/29_atomics/atomic_flag/wait_notify.cc: Likewise.
>>>	* testsuite/29_atomics/atomic_float/wait_notify.cc: Likewise.
>>>	* testsuite/29_atomics/atomic_integral/wait_notify.cc: Likewise.
>>>	* testsuite/29_atomics/atomic_ref/wait_notify.cc: Likewise.
>>
>>>-    struct __timed_waiters : __waiters
>>>+    struct __timed_waiters : __waiter_pool_base
>>
>>Should this be __timed_waiter_pool for consistency with
>>__waiter_pool_base and __waiter_pool?
>>
>>
>>>-    inline void
>>>-    __thread_relax() noexcept
>>>-    {
>>>-#if defined __i386__ || defined __x86_64__
>>>-      __builtin_ia32_pause();
>>>-#elif defined _GLIBCXX_USE_SCHED_YIELD
>>>-      __gthread_yield();
>>>-#endif
>>>-    }
>>>+    template<typename _Tp>
>>>+      struct __waiter_base
>>>+      {
>>>+	using __waiter_type = _Tp;
>>>
>>>-    inline void
>>>-    __thread_yield() noexcept
>>>-    {
>>>-#if defined _GLIBCXX_USE_SCHED_YIELD
>>>-     __gthread_yield();
>>>-#endif
>>>-    }
>>
>>This chunk of the patch doesn't apply, because it's based on an old
>>version of trunk (before r11-7248).
>
>I managed to bodge the patch so it applies, see attached.

The attached patch is what I've pushed to trunk and gcc-11, which
addresses all my comments from today.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 71638 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20210420/f050f39e/attachment-0001.bin>


More information about the Libstdc++ mailing list