public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-8337] libgcc: fix Win32 CV abnormal spurious wakeups in timed wait [PR113850]
@ 2024-02-16 23:49 Jonathan Yong
  0 siblings, 0 replies; only message in thread
From: Jonathan Yong @ 2024-02-16 23:49 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:b5def8fd08976fc2514426ecb1f263f13ba3e2af

commit r13-8337-gb5def8fd08976fc2514426ecb1f263f13ba3e2af
Author: Matteo Italia <matteo@mitalia.net>
Date:   Fri Feb 9 15:04:20 2024 +0100

    libgcc: fix Win32 CV abnormal spurious wakeups in timed wait [PR113850]
    
    Fix a typo in __gthr_win32_abs_to_rel_time that caused it to return a
    relative time in seconds instead of milliseconds. As a consequence,
    __gthr_win32_cond_timedwait called SleepConditionVariableCS with a
    1000x shorter timeout; this caused ~1000x more spurious wakeups in
    CV timed waits such as std::condition_variable::wait_for or wait_until,
    resulting generally in much higher CPU usage.
    
    This can be demonstrated by this sample program:
    
    ```
    
    int main() {
        std::condition_variable cv;
        std::mutex mx;
        bool pass = false;
    
        auto thread_fn = [&](bool timed) {
            int wakeups = 0;
            using sc = std::chrono::system_clock;
            auto before = sc::now();
            std::unique_lock<std::mutex> ml(mx);
            if (timed) {
                cv.wait_for(ml, std::chrono::seconds(2), [&]{
                    ++wakeups;
                    return pass;
                });
            } else {
                cv.wait(ml, [&]{
                    ++wakeups;
                    return pass;
                });
            }
            printf("pass: %d; wakeups: %d; elapsed: %d ms\n", pass, wakeups,
                    int((sc::now() - before) / std::chrono::milliseconds(1)));
            pass = false;
        };
    
        {
            // timed wait, let expire
            std::thread t(thread_fn, true);
            t.join();
        }
    
        {
            // timed wait, wake up explicitly after 1 second
            std::thread t(thread_fn, true);
            std::this_thread::sleep_for(std::chrono::seconds(1));
            {
                std::unique_lock<std::mutex> ml(mx);
                pass = true;
            }
            cv.notify_all();
            t.join();
        }
    
        {
            // non-timed wait, wake up explicitly after 1 second
            std::thread t(thread_fn, false);
            std::this_thread::sleep_for(std::chrono::seconds(1));
            {
                std::unique_lock<std::mutex> ml(mx);
                pass = true;
            }
            cv.notify_all();
            t.join();
        }
        return 0;
    }
    ```
    
    On builds based on non-affected threading models (e.g. POSIX on Linux,
    or winpthreads or MCF on Win32) the output is something like
    ```
    pass: 0; wakeups: 2; elapsed: 2000 ms
    pass: 1; wakeups: 2; elapsed: 991 ms
    pass: 1; wakeups: 2; elapsed: 996 ms
    ```
    
    while with the Win32 threading model we get
    ```
    pass: 0; wakeups: 1418; elapsed: 2000 ms
    pass: 1; wakeups: 479; elapsed: 988 ms
    pass: 1; wakeups: 2; elapsed: 992 ms
    ```
    (notice the huge number of wakeups in the timed wait cases only).
    
    This commit fixes the conversion, adjusting the final division by
    NSEC100_PER_SEC to use NSEC100_PER_MSEC instead (already defined in the
    file and not used in any other place, so probably just a typo).
    
    libgcc/ChangeLog:
    
            PR libgcc/113850
            * config/i386/gthr-win32-cond.c (__gthr_win32_abs_to_rel_time):
            fix absolute timespec to relative milliseconds count
            conversion (it incorrectly returned seconds instead of
            milliseconds); this avoids spurious wakeups in
            __gthr_win32_cond_timedwait
    
    (cherry picked from commit 05ad8fb55a55f1e201fd781c84682a7c0bbd4d97)

Diff:
---
 libgcc/config/i386/gthr-win32-cond.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgcc/config/i386/gthr-win32-cond.c b/libgcc/config/i386/gthr-win32-cond.c
index 75fb253bee14..c6e9776d30cb 100644
--- a/libgcc/config/i386/gthr-win32-cond.c
+++ b/libgcc/config/i386/gthr-win32-cond.c
@@ -78,7 +78,7 @@ __gthr_win32_abs_to_rel_time (const __gthread_time_t *abs_time)
   if (abs_time_nsec100 < now.nsec100)
     return 0;
 
-  return (DWORD) CEIL_DIV (abs_time_nsec100 - now.nsec100, NSEC100_PER_SEC);
+  return (DWORD) CEIL_DIV (abs_time_nsec100 - now.nsec100, NSEC100_PER_MSEC);
 }
 
 /* Check the sizes of the local version of the Win32 types.  */

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-02-16 23:49 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-16 23:49 [gcc r13-8337] libgcc: fix Win32 CV abnormal spurious wakeups in timed wait [PR113850] Jonathan Yong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).