From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1814) id 797873857702; Fri, 16 Feb 2024 23:47:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 797873857702 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1708127275; bh=tL//9dZxYEpSIdPqXx5OspxViu4AkqHMf6PWsxcIPFc=; h=From:To:Subject:Date:From; b=afmwpNOJfni8uE54udFcQVKbmU2BP0fqifJ5qFNY0/R/wznjyH+C9DdOedBBpJEqA hmzSKy9TW1y/jmzgVL5rmN/Gm2+TwG+DwdEIrrqNvzw0twq5czVEMX/7xLziwqOP8G NV/eRU3ttXOhxLA0Oj9fhBdZAxQt1ryxZjkw8tCY= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jonathan Yong To: gcc-cvs@gcc.gnu.org Subject: [gcc r14-9042] libgcc: fix Win32 CV abnormal spurious wakeups in timed wait [PR113850] X-Act-Checkin: gcc X-Git-Author: Matteo Italia X-Git-Refname: refs/heads/master X-Git-Oldrev: cd503b0616462445381a8232fb753239d319af76 X-Git-Newrev: 05ad8fb55a55f1e201fd781c84682a7c0bbd4d97 Message-Id: <20240216234755.797873857702@sourceware.org> Date: Fri, 16 Feb 2024 23:47:55 +0000 (GMT) List-Id: https://gcc.gnu.org/g:05ad8fb55a55f1e201fd781c84682a7c0bbd4d97 commit r14-9042-g05ad8fb55a55f1e201fd781c84682a7c0bbd4d97 Author: Matteo Italia 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 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 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 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. */