https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113850

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jonathan Yong
<jy...@gcc.gnu.org>:

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

commit r13-8337-gb5def8fd08976fc2514426ecb1f263f13ba3e2af
Author: Matteo Italia <mat...@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)

Reply via email to