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

Jonathan Wakely jwakely@redhat.com
Tue Apr 20 11:04:55 GMT 2021


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).



More information about the Libstdc++ mailing list