Skip to content
Snippets Groups Projects
  • Matteo Italia's avatar
    05ad8fb5
    libgcc: fix Win32 CV abnormal spurious wakeups in timed wait [PR113850] · 05ad8fb5
    Matteo Italia authored
    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
    05ad8fb5
    History
    libgcc: fix Win32 CV abnormal spurious wakeups in timed wait [PR113850]
    Matteo Italia authored
    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