On 19/04/21 12:23 -0700, Thomas Rodgers wrote:
From: Thomas Rodgers <rodg...@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).

Reply via email to