public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Jonathan Yong <jyong@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r14-9042] libgcc: fix Win32 CV abnormal spurious wakeups in timed wait [PR113850]
Date: Fri, 16 Feb 2024 23:47:55 +0000 (GMT)	[thread overview]
Message-ID: <20240216234755.797873857702@sourceware.org> (raw)

https://gcc.gnu.org/g:05ad8fb55a55f1e201fd781c84682a7c0bbd4d97

commit r14-9042-g05ad8fb55a55f1e201fd781c84682a7c0bbd4d97
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

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 ecb007a54b2d..650c448f2864 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.  */

                 reply	other threads:[~2024-02-16 23:47 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240216234755.797873857702@sourceware.org \
    --to=jyong@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).