From patchwork Fri May 29 06:17:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Crowe X-Patchwork-Id: 1300416 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=gg3C6Arx; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49YDsY04Qnz9sSn for ; Fri, 29 May 2020 16:18:40 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7CAD1388C01B; Fri, 29 May 2020 06:18:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7CAD1388C01B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1590733090; bh=5MZSWhDm8JM9fNspQeqj+NA/LXQcmiTL0BaIutVXMuM=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=gg3C6ArxsaNO5bHXjvBDSonkIj23U4l8rlYhRADD1WtJ2lwiuhX7Hxrm3G31cHr8a OB8vM+iniuRkLkouQvGSrmUlmAhILDO0ThZT2ageZhKswGEEBFUPrLjrrl4jR1gHdu atVvruDYp7R8HT7br6M5raf2aSze+IxXeQN080LI= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from avasout07.plus.net (avasout07.plus.net [84.93.230.235]) by sourceware.org (Postfix) with ESMTPS id C756F388C00D for ; Fri, 29 May 2020 06:18:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C756F388C00D Received: from deneb ([80.229.24.9]) by smtp with ESMTP id eYLBj2a0H0wwMeYLCj60DH; Fri, 29 May 2020 07:18:07 +0100 X-Clacks-Overhead: "GNU Terry Pratchett" X-CM-Score: 0.00 X-CNFS-Analysis: v=2.3 cv=b/4pHuOx c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=sTwFKg_x9MkA:10 a=feKxga7XeoDSQRxrCx4A:9 Received: from mac by deneb with local (Exim 4.92) (envelope-from ) id 1jeYL2-00063J-7N; Fri, 29 May 2020 07:17:52 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH v5 1/8] libstdc++: Improve async test Date: Fri, 29 May 2020 07:17:27 +0100 Message-Id: <0f48b556ff1c8767e7095886fb4569596d5a41e2.1590732962.git-series.mac@mcrowe.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 X-CMAE-Envelope: MS4wfFiHiF+hBWHt89q93e4krhtcHuaHM6ZTnMwLwD3XIEuzSzRfbuzi7uVv9omoBgeXc7X+YK31iHSe8dhxEMT4QCBylc9llUAPM6pYaKW5wYQo7LgVJVdg lON9DSPAqYaQIgKIdYeU+EANvWw4IAJ5LT8= X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Mike Crowe via Gcc-patches From: Mike Crowe Reply-To: Mike Crowe Cc: Mike Crowe Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Add tests for waiting for the future using both std::chrono::steady_clock and std::chrono::system_clock in preparation for dealing with those clocks properly in futex.cc. * libstdc++-v3/testsuite/30_threads/async/async.cc (test02): Test steady_clock with std::future::wait_until. (test03): Add new test templated on clock type waiting for future associated with async to resolve. (main): Call test03 to test both system_clock and steady_clock. --- libstdc++-v3/testsuite/30_threads/async/async.cc | 33 +++++++++++++++++- 1 file changed, 33 insertions(+) diff --git a/libstdc++-v3/testsuite/30_threads/async/async.cc b/libstdc++-v3/testsuite/30_threads/async/async.cc index 7fa9b03..84d94cf 100644 --- a/libstdc++-v3/testsuite/30_threads/async/async.cc +++ b/libstdc++-v3/testsuite/30_threads/async/async.cc @@ -51,17 +51,50 @@ void test02() VERIFY( status == std::future_status::timeout ); status = f1.wait_until(std::chrono::system_clock::now()); VERIFY( status == std::future_status::timeout ); + status = f1.wait_until(std::chrono::steady_clock::now()); + VERIFY( status == std::future_status::timeout ); l.unlock(); // allow async thread to proceed f1.wait(); // wait for it to finish status = f1.wait_for(std::chrono::milliseconds(0)); VERIFY( status == std::future_status::ready ); status = f1.wait_until(std::chrono::system_clock::now()); VERIFY( status == std::future_status::ready ); + status = f1.wait_until(std::chrono::steady_clock::now()); + VERIFY( status == std::future_status::ready ); +} + +// This test is prone to failures if run on a loaded machine where the +// kernel decides not to schedule us for several seconds. It also +// assumes that no-one will warp CLOCK whilst the test is +// running when CLOCK is std::chrono::system_clock. +template +void test03() +{ + auto const start = CLOCK::now(); + future f1 = async(launch::async, []() { + std::this_thread::sleep_for(std::chrono::seconds(2)); + }); + std::future_status status; + + status = f1.wait_for(std::chrono::milliseconds(500)); + VERIFY( status == std::future_status::timeout ); + + status = f1.wait_until(start + std::chrono::seconds(1)); + VERIFY( status == std::future_status::timeout ); + + status = f1.wait_until(start + std::chrono::seconds(5)); + VERIFY( status == std::future_status::ready ); + + auto const elapsed = CLOCK::now() - start; + VERIFY( elapsed >= std::chrono::seconds(2) ); + VERIFY( elapsed < std::chrono::seconds(5) ); } int main() { test01(); test02(); + test03(); + test03(); return 0; } From patchwork Fri May 29 06:17:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Crowe X-Patchwork-Id: 1300423 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=fcTezfSi; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49YDst4twbz9sSn for ; Fri, 29 May 2020 16:18:58 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 80241388C027; Fri, 29 May 2020 06:18:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 80241388C027 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1590733101; bh=+coNTCZZOvPgGGrqCJudDzuxGwzQCy+n8owM/4G7JyI=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=fcTezfSi8yRLpTICwRGYRU3Y6EoK2Fc3Va1h932cPvtd2U4ptVKZf01jlLB0hsjJZ 9iMizH8o2QZCqbF9wCZfortN9wYHnRELpcVE/byaQaXc0/44VApby4J21fJh5Oaopk NqevRKC1aYWlI359v+TPxXNKJLl4uE29m0c8i6rE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from avasout07.plus.net (avasout07.plus.net [84.93.230.235]) by sourceware.org (Postfix) with ESMTPS id 3E3F1388C00E for ; Fri, 29 May 2020 06:18:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3E3F1388C00E Received: from deneb ([80.229.24.9]) by smtp with ESMTP id eYLJj2a0Y0wwMeYLKj60DN; Fri, 29 May 2020 07:18:17 +0100 X-Clacks-Overhead: "GNU Terry Pratchett" X-CM-Score: 0.00 X-CNFS-Analysis: v=2.3 cv=b/4pHuOx c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=sTwFKg_x9MkA:10 a=0Faq-oMWdgr7wUJnaP0A:9 Received: from mac by deneb with local (Exim 4.92) (envelope-from ) id 1jeYL2-00063h-Ja; Fri, 29 May 2020 07:17:52 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH v5 2/8] libstdc++ futex: Use FUTEX_CLOCK_REALTIME for wait Date: Fri, 29 May 2020 07:17:28 +0100 Message-Id: <796560f571e7787a1bd0aa655bc25b873e2d49b4.1590732962.git-series.mac@mcrowe.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 X-CMAE-Envelope: MS4wfIyAu5WvphuSlNvGvV2MTH0uBJuA3Sp+bNG0Xed3OoogHzOi/BexZLdPRcEu/33IhAuh2tRrkZnmu2WDDxwsM9shRUpiOk5nlftTwy9w0xiErND9Gv9m jXq44cKdrPBRm2mIxyXgfS85T3fsUU7IUtE= X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Mike Crowe via Gcc-patches From: Mike Crowe Reply-To: Mike Crowe Cc: Mike Crowe Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" The futex system call supports waiting for an absolute time if FUTEX_WAIT_BITSET is used rather than FUTEX_WAIT. Doing so provides two benefits: 1. The call to gettimeofday is not required in order to calculate a relative timeout. 2. If someone changes the system clock during the wait then the futex timeout will correctly expire earlier or later. Currently that only happens if the clock is changed prior to the call to gettimeofday. According to futex(2), support for FUTEX_CLOCK_REALTIME was added in the v2.6.28 Linux kernel and FUTEX_WAIT_BITSET was added in v2.6.25. To ensure that the code still works correctly with earlier kernel versions, an ENOSYS error from futex[1] results in the futex_clock_realtime_unavailable flag being set. This flag is used to avoid the unnecessary unsupported futex call in the future and to fall back to the previous gettimeofday and relative time implementation. glibc applied an equivalent switch in pthread_cond_timedwait to use FUTEX_CLOCK_REALTIME and FUTEX_WAIT_BITSET rather than FUTEX_WAIT for glibc-2.10 back in 2009. See glibc:cbd8aeb836c8061c23a5e00419e0fb25a34abee7 The futex_clock_realtime_unavailable flag is accessed using std::memory_order_relaxed to stop it becoming a bottleneck. If the first two calls to _M_futex_wait_until happen to happen simultaneously then the only consequence is that both will try to use FUTEX_CLOCK_REALTIME, both risk discovering that it doesn't work and, if so, both set the flag. [1] This is how glibc's nptl-init.c determines whether these flags are supported. * libstdc++-v3/src/c++11/futex.cc: Add new constants for required futex flags. Add futex_clock_realtime_unavailable flag to store result of trying to use FUTEX_CLOCK_REALTIME. (__atomic_futex_unsigned_base::_M_futex_wait_until): Try to use FUTEX_WAIT_BITSET with FUTEX_CLOCK_REALTIME and only fall back to using gettimeofday and FUTEX_WAIT if that's not supported. --- libstdc++-v3/src/c++11/futex.cc | 37 ++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+) diff --git a/libstdc++-v3/src/c++11/futex.cc b/libstdc++-v3/src/c++11/futex.cc index c9de11a..25b3e05 100644 --- a/libstdc++-v3/src/c++11/futex.cc +++ b/libstdc++-v3/src/c++11/futex.cc @@ -35,8 +35,16 @@ // Constants for the wait/wake futex syscall operations const unsigned futex_wait_op = 0; +const unsigned futex_wait_bitset_op = 9; +const unsigned futex_clock_realtime_flag = 256; +const unsigned futex_bitset_match_any = ~0; const unsigned futex_wake_op = 1; +namespace +{ + std::atomic futex_clock_realtime_unavailable; +} + namespace std _GLIBCXX_VISIBILITY(default) { _GLIBCXX_BEGIN_NAMESPACE_VERSION @@ -58,6 +66,35 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } else { + if (!futex_clock_realtime_unavailable.load(std::memory_order_relaxed)) + { + struct timespec rt; + rt.tv_sec = __s.count(); + rt.tv_nsec = __ns.count(); + if (syscall (SYS_futex, __addr, + futex_wait_bitset_op | futex_clock_realtime_flag, + __val, &rt, nullptr, futex_bitset_match_any) == -1) + { + __glibcxx_assert(errno == EINTR || errno == EAGAIN + || errno == ETIMEDOUT || errno == ENOSYS); + if (errno == ETIMEDOUT) + return false; + if (errno == ENOSYS) + { + futex_clock_realtime_unavailable.store(true, + std::memory_order_relaxed); + // Fall through to legacy implementation if the system + // call is unavailable. + } + else + return true; + } + else + return true; + } + + // We only get to here if futex_clock_realtime_unavailable was + // true or has just been set to true. struct timeval tv; gettimeofday (&tv, NULL); // Convert the absolute timeout value to a relative timeout From patchwork Fri May 29 06:17:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Crowe X-Patchwork-Id: 1300425 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=MX54aoGj; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49YDsz1MM2z9sSn for ; Fri, 29 May 2020 16:19:03 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1A5D1388C011; Fri, 29 May 2020 06:18:25 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1A5D1388C011 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1590733105; bh=vzjJxL74T3z79l4x/+cn+nY0kPTaU4JhXHhLF9pLSN4=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=MX54aoGjNcUg6G1ZiCr4QMGLTxfI6DTiQUPq7zue4LUWglwyaGuogiXQB769c0q2r gs842YHlAii42BCrA81fczmWh5fBsZsQDDqjSAWQYwn2++7q31B4fscYMVYim8GD+d Ljh8Euav40yw7H/ld1HOcaQ7+NgZDpCH/mHq3KvQ= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from avasout02.plus.net (avasout02.plus.net [212.159.14.17]) by sourceware.org (Postfix) with ESMTPS id A99A5388C011 for ; Fri, 29 May 2020 06:18:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A99A5388C011 Received: from deneb ([80.229.24.9]) by smtp with ESMTP id eYLSj9hGIU8CkeYLTjognc; Fri, 29 May 2020 07:18:19 +0100 X-Clacks-Overhead: "GNU Terry Pratchett" X-CM-Score: 0.00 X-CNFS-Analysis: v=2.3 cv=G/eH7+s5 c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=sTwFKg_x9MkA:10 a=dllVhUKQtCpdTioMnPoA:9 a=5E5lp0EbJfARXg7R:21 a=DtQUOWFLPJAUH6Un:21 a=pHzHmUro8NiASowvMSCR:22 a=n87TN5wuljxrRezIQYnT:22 Received: from mac by deneb with local (Exim 4.92) (envelope-from ) id 1jeYL3-000646-0A; Fri, 29 May 2020 07:17:53 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH v5 3/8] libstdc++ futex: Support waiting on std::chrono::steady_clock directly Date: Fri, 29 May 2020 07:17:29 +0100 Message-Id: <55b3c3fdce2d7a9860ef95edfdbecd6776627abc.1590732962.git-series.mac@mcrowe.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 X-CMAE-Envelope: MS4wfPD/UeUOJBQXXJXdYw8/YQtwwsdUyMjKiJ59NY0X/9vDo+nTFLUX/IfNMHQ+DgeiooUxe8G6fWivaYyiA5G/Pi7MqL7NyB93nXIdlSWa4XX2EAgVN39B Tx0aOpywnRxkODjrTFK7DxjC45RvMfGu8+Y= X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Mike Crowe via Gcc-patches From: Mike Crowe Reply-To: Mike Crowe Cc: Mike Crowe Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" The user-visible effect of this change is for std::future::wait_until to use CLOCK_MONOTONIC when passed a timeout of std::chrono::steady_clock type. This makes it immune to any changes made to the system clock CLOCK_REALTIME. Add an overload of __atomic_futex_unsigned::_M_load_and_text_until_impl that accepts a std::chrono::steady_clock, and correctly passes this through to __atomic_futex_unsigned_base::_M_futex_wait_until_steady which uses CLOCK_MONOTONIC for the timeout within the futex system call. These functions are mostly just copies of the std::chrono::system_clock versions with small tweaks. Prior to this commit, a std::chrono::steady timeout would be converted via std::chrono::system_clock which risks reducing or increasing the timeout if someone changes CLOCK_REALTIME whilst the wait is happening. (The commit immediately prior to this one increases the window of opportunity for that from a short period during the calculation of a relative timeout, to the entire duration of the wait.) FUTEX_WAIT_BITSET was added in kernel v2.6.25. If futex reports ENOSYS to indicate that this operation is not supported then the code falls back to using clock_gettime(2) to calculate a relative time to wait for. I believe that I've added this functionality in a way that it doesn't break ABI compatibility, but that has made it more verbose and less type safe. I believe that it would be better to maintain the timeout as an instance of the correct clock type all the way down to a single _M_futex_wait_until function with an overload for each clock. The current scheme of separating out the seconds and nanoseconds early risks accidentally calling the wait function for the wrong clock. Unfortunately, doing this would break code that compiled against the old header. * libstdc++-v3/config/abi/pre/gnu.ver: Update for addition of __atomic_futex_unsigned_base::_M_futex_wait_until_steady. * libstdc++-v3/include/bits/atomic_futex.h (__atomic_futex_unsigned_base): Add comments to clarify that _M_futex_wait_until _M_load_and_test_until use CLOCK_REALTIME. Declare new _M_futex_wait_until_steady and _M_load_and_text_until_steady methods that use CLOCK_MONOTONIC. Add _M_load_and_test_until_impl and _M_load_when_equal_until overloads that accept a steady_clock time_point and use these new methods. * libstdc++-v3/src/c++11/futex.cc: Include headers required for clock_gettime. Add futex_clock_monotonic_flag constant to tell futex to use CLOCK_MONOTONIC to match the existing futex_clock_realtime_flag. Add futex_clock_monotonic_unavailable to store the result of trying to use CLOCK_MONOTONIC. (__atomic_futex_unsigned_base::_M_futex_wait_until_steady): Add new variant of _M_futex_wait_until that uses CLOCK_MONOTONIC to support waiting using steady_clock. --- libstdc++-v3/config/abi/pre/gnu.ver | 10 +-- libstdc++-v3/include/bits/atomic_futex.h | 67 +++++++++++++++++++- libstdc++-v3/src/c++11/futex.cc | 82 +++++++++++++++++++++++++- 3 files changed, 154 insertions(+), 5 deletions(-) diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index edf4485..3d734d7 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -1916,10 +1916,9 @@ GLIBCXX_3.4.21 { _ZNSt7codecvtID[is]c*; _ZT[ISV]St7codecvtID[is]c*E; - extern "C++" - { - std::__atomic_futex_unsigned_base*; - }; + # std::__atomic_futex_unsigned_base members + _ZNSt28__atomic_futex_unsigned_base19_M_futex_notify_all*; + _ZNSt28__atomic_futex_unsigned_base19_M_futex_wait_until*; # codecvt_utf8 etc. _ZNKSt19__codecvt_utf8_base*; @@ -2297,6 +2296,9 @@ GLIBCXX_3.4.28 { _ZNSt3pmr25monotonic_buffer_resourceD[0125]Ev; _ZT[ISV]NSt3pmr25monotonic_buffer_resourceE; + # std::__atomic_futex_unsigned_base::_M_futex_wait_until_steady + _ZNSt28__atomic_futex_unsigned_base26_M_futex_wait_until_steady*; + } GLIBCXX_3.4.27; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h index 886fc63..507c5c9 100644 --- a/libstdc++-v3/include/bits/atomic_futex.h +++ b/libstdc++-v3/include/bits/atomic_futex.h @@ -52,11 +52,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if defined(_GLIBCXX_HAVE_LINUX_FUTEX) && ATOMIC_INT_LOCK_FREE > 1 struct __atomic_futex_unsigned_base { - // Returns false iff a timeout occurred. + // __s and __ns are measured against CLOCK_REALTIME. Returns false + // iff a timeout occurred. bool _M_futex_wait_until(unsigned *__addr, unsigned __val, bool __has_timeout, chrono::seconds __s, chrono::nanoseconds __ns); + // __s and __ns are measured against CLOCK_MONOTONIC. Returns + // false iff a timeout occurred. + bool + _M_futex_wait_until_steady(unsigned *__addr, unsigned __val, + bool __has_timeout, chrono::seconds __s, chrono::nanoseconds __ns); + // This can be executed after the object has been destroyed. static void _M_futex_notify_all(unsigned* __addr); }; @@ -86,6 +93,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // value if equal is false. // The assumed value is the caller's assumption about the current value // when making the call. + // __s and __ns are measured against CLOCK_REALTIME. unsigned _M_load_and_test_until(unsigned __assumed, unsigned __operand, bool __equal, memory_order __mo, bool __has_timeout, @@ -110,6 +118,36 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } + // If a timeout occurs, returns a current value after the timeout; + // otherwise, returns the operand's value if equal is true or a different + // value if equal is false. + // The assumed value is the caller's assumption about the current value + // when making the call. + // __s and __ns are measured against CLOCK_MONOTONIC. + unsigned + _M_load_and_test_until_steady(unsigned __assumed, unsigned __operand, + bool __equal, memory_order __mo, bool __has_timeout, + chrono::seconds __s, chrono::nanoseconds __ns) + { + for (;;) + { + // Don't bother checking the value again because we expect the caller + // to have done it recently. + // memory_order_relaxed is sufficient because we can rely on just the + // modification order (store_notify uses an atomic RMW operation too), + // and the futex syscalls synchronize between themselves. + _M_data.fetch_or(_Waiter_bit, memory_order_relaxed); + bool __ret = _M_futex_wait_until_steady((unsigned*)(void*)&_M_data, + __assumed | _Waiter_bit, + __has_timeout, __s, __ns); + // Fetch the current value after waiting (clears _Waiter_bit). + __assumed = _M_load(__mo); + if (!__ret || ((__operand == __assumed) == __equal)) + return __assumed; + // TODO adapt wait time + } + } + // Returns the operand's value if equal is true or a different value if // equal is false. // The assumed value is the caller's assumption about the current value @@ -140,6 +178,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION true, __s.time_since_epoch(), __ns); } + template + unsigned + _M_load_and_test_until_impl(unsigned __assumed, unsigned __operand, + bool __equal, memory_order __mo, + const chrono::time_point& __atime) + { + auto __s = chrono::time_point_cast(__atime); + auto __ns = chrono::duration_cast(__atime - __s); + // XXX correct? + return _M_load_and_test_until_steady(__assumed, __operand, __equal, __mo, + true, __s.time_since_epoch(), __ns); + } + public: _GLIBCXX_ALWAYS_INLINE unsigned @@ -200,6 +251,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return (__i & ~_Waiter_bit) == __val; } + // Returns false iff a timeout occurred. + template + _GLIBCXX_ALWAYS_INLINE bool + _M_load_when_equal_until(unsigned __val, memory_order __mo, + const chrono::time_point& __atime) + { + unsigned __i = _M_load(__mo); + if ((__i & ~_Waiter_bit) == __val) + return true; + // TODO Spin-wait first. Ignore effect on timeout. + __i = _M_load_and_test_until_impl(__i, __val, true, __mo, __atime); + return (__i & ~_Waiter_bit) == __val; + } + _GLIBCXX_ALWAYS_INLINE void _M_store_notify_all(unsigned __val, memory_order __mo) { diff --git a/libstdc++-v3/src/c++11/futex.cc b/libstdc++-v3/src/c++11/futex.cc index 25b3e05..0331bd6 100644 --- a/libstdc++-v3/src/c++11/futex.cc +++ b/libstdc++-v3/src/c++11/futex.cc @@ -33,9 +33,15 @@ #include #include +#ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL +#include +#include +#endif + // Constants for the wait/wake futex syscall operations const unsigned futex_wait_op = 0; const unsigned futex_wait_bitset_op = 9; +const unsigned futex_clock_monotonic_flag = 0; const unsigned futex_clock_realtime_flag = 256; const unsigned futex_bitset_match_any = ~0; const unsigned futex_wake_op = 1; @@ -43,6 +49,7 @@ const unsigned futex_wake_op = 1; namespace { std::atomic futex_clock_realtime_unavailable; + std::atomic futex_clock_monotonic_unavailable; } namespace std _GLIBCXX_VISIBILITY(default) @@ -121,6 +128,81 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } + bool + __atomic_futex_unsigned_base::_M_futex_wait_until_steady(unsigned *__addr, + unsigned __val, + bool __has_timeout, chrono::seconds __s, chrono::nanoseconds __ns) + { + if (!__has_timeout) + { + // Ignore whether we actually succeeded to block because at worst, + // we will fall back to spin-waiting. The only thing we could do + // here on errors is abort. + int ret __attribute__((unused)); + ret = syscall (SYS_futex, __addr, futex_wait_op, __val, nullptr); + __glibcxx_assert(ret == 0 || errno == EINTR || errno == EAGAIN); + return true; + } + else + { + if (!futex_clock_monotonic_unavailable.load(std::memory_order_relaxed)) + { + struct timespec rt; + rt.tv_sec = __s.count(); + rt.tv_nsec = __ns.count(); + + if (syscall (SYS_futex, __addr, + futex_wait_bitset_op | futex_clock_monotonic_flag, + __val, &rt, nullptr, futex_bitset_match_any) == -1) + { + __glibcxx_assert(errno == EINTR || errno == EAGAIN + || errno == ETIMEDOUT || errno == ENOSYS); + if (errno == ETIMEDOUT) + return false; + else if (errno == ENOSYS) + { + futex_clock_monotonic_unavailable.store(true, + std::memory_order_relaxed); + // Fall through to legacy implementation if the system + // call is unavailable. + } + else + return true; + } + } + + // We only get to here if futex_clock_monotonic_unavailable was + // true or has just been set to true. + struct timespec ts; +#ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL + syscall(SYS_clock_gettime, CLOCK_MONOTONIC, &ts); +#else + clock_gettime(CLOCK_MONOTONIC, &ts); +#endif + // Convert the absolute timeout value to a relative timeout + struct timespec rt; + rt.tv_sec = __s.count() - ts.tv_sec; + rt.tv_nsec = __ns.count() - ts.tv_nsec; + if (rt.tv_nsec < 0) + { + rt.tv_nsec += 1000000000; + --rt.tv_sec; + } + // Did we already time out? + if (rt.tv_sec < 0) + return false; + + if (syscall (SYS_futex, __addr, futex_wait_op, __val, &rt) == -1) + { + __glibcxx_assert(errno == EINTR || errno == EAGAIN + || errno == ETIMEDOUT); + if (errno == ETIMEDOUT) + return false; + } + return true; + } + } + void __atomic_futex_unsigned_base::_M_futex_notify_all(unsigned* __addr) { From patchwork Fri May 29 06:17:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Crowe X-Patchwork-Id: 1300404 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=mcXHkO+D; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49YDs003s1z9sSp for ; Fri, 29 May 2020 16:18:12 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2414D388A82A; Fri, 29 May 2020 06:18:03 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2414D388A82A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1590733083; bh=aFaUFjSQMKFGwaFI5aPxvCxYEUkulqJ3+esFbbpyvWI=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=mcXHkO+DXnTrwBuoPLhuJUg5wID6NM/I5ulI+zOaaq7yd0t2HvUhcME9O4MUFUsn3 knHDOvftec686fCPVJHWB2uWbL+SjWGOlVf5PvMm52/tXN4ZrGokPT8xWtXVZ5UtTL QakmY2xhQW8H2nHAZkCopcRwGEzK+eqikX8Y1Gd4= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from avasout02.plus.net (avasout02.plus.net [212.159.14.17]) by sourceware.org (Postfix) with ESMTPS id C14EE3885C08 for ; Fri, 29 May 2020 06:17:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C14EE3885C08 Received: from deneb ([80.229.24.9]) by smtp with ESMTP id eYL4j9hFBU8CkeYL5jognF; Fri, 29 May 2020 07:17:57 +0100 X-Clacks-Overhead: "GNU Terry Pratchett" X-CM-Score: 0.00 X-CNFS-Analysis: v=2.3 cv=G/eH7+s5 c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=sTwFKg_x9MkA:10 a=Qe1SKS0R_PMm6J9yl14A:9 a=+jEqtf1s3R9VXZ0wqowq2kgwd+I=:19 a=uUF-uEs-_EH4YQze:21 a=yPM3RS9temHarRS9:21 a=pHzHmUro8NiASowvMSCR:22 a=n87TN5wuljxrRezIQYnT:22 Received: from mac by deneb with local (Exim 4.92) (envelope-from ) id 1jeYL3-00064X-Cj; Fri, 29 May 2020 07:17:53 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH v5 4/8] libstdc++ atomic_futex: Use std::chrono::steady_clock as reference clock Date: Fri, 29 May 2020 07:17:30 +0100 Message-Id: X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 X-CMAE-Envelope: MS4wfLE17+N+WjiEAfUN3fMrRfB8go9AhbL8p7Zp/yGTXiDT1hhfrKhK0BN6+QUkSJ2MZlQyQ+B9/48/Y/m9BxrCIqEhMlAGfi0/K4z9WlgS+ruLd8JfRC0y QmAJRnZ2AqWpmVOYzxkfMjPNW0F8K2xflNQ= X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Mike Crowe via Gcc-patches From: Mike Crowe Reply-To: Mike Crowe Cc: Mike Crowe Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" The user-visible effect of this change is that std::future::wait_for now uses std::chrono::steady_clock to determine the timeout. This makes it immune to changes made to the system clock. It also means that anyone using their own clock types with std::future::wait_until will have the timeout converted to std::chrono::steady_clock rather than std::chrono::system_clock. Now that use of both std::chrono::steady_clock and std::chrono::system_clock are correctly supported for the wait timeout, I believe that std::chrono::steady_clock is a better choice for the reference clock that all other clocks are converted to since it is guaranteed to advance steadily. The previous behaviour of converting to std::chrono::system_clock risks timeouts changing dramatically when the system clock is changed. * libstdc++-v3/include/bits/atomic_futex.h: (__atomic_futex_unsigned): Change __clock_t typedef to use steady_clock so that unknown clocks are synced to it rather than system_clock. Change existing __clock_t overloads of _M_load_and_text_until_impl and _M_load_when_equal_until to use system_clock explicitly. Remove comment about DR 887 since these changes address that problem as best as we currently able. --- libstdc++-v3/include/bits/atomic_futex.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h index 507c5c9..4375129 100644 --- a/libstdc++-v3/include/bits/atomic_futex.h +++ b/libstdc++-v3/include/bits/atomic_futex.h @@ -71,7 +71,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template class __atomic_futex_unsigned : __atomic_futex_unsigned_base { - typedef chrono::system_clock __clock_t; + typedef chrono::steady_clock __clock_t; // This must be lock-free and at offset 0. atomic _M_data; @@ -169,7 +169,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION unsigned _M_load_and_test_until_impl(unsigned __assumed, unsigned __operand, bool __equal, memory_order __mo, - const chrono::time_point<__clock_t, _Dur>& __atime) + const chrono::time_point& __atime) { auto __s = chrono::time_point_cast(__atime); auto __ns = chrono::duration_cast(__atime - __s); @@ -229,7 +229,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_load_when_equal_until(unsigned __val, memory_order __mo, const chrono::time_point<_Clock, _Duration>& __atime) { - // DR 887 - Sync unknown clock to known clock. const typename _Clock::time_point __c_entry = _Clock::now(); const __clock_t::time_point __s_entry = __clock_t::now(); const auto __delta = __atime - __c_entry; @@ -241,7 +240,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template _GLIBCXX_ALWAYS_INLINE bool _M_load_when_equal_until(unsigned __val, memory_order __mo, - const chrono::time_point<__clock_t, _Duration>& __atime) + const chrono::time_point& __atime) { unsigned __i = _M_load(__mo); if ((__i & ~_Waiter_bit) == __val) From patchwork Fri May 29 06:17:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Crowe X-Patchwork-Id: 1300414 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=Abme3YBs; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49YDsR58HNz9sSw for ; Fri, 29 May 2020 16:18:35 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D83D1388C017; Fri, 29 May 2020 06:18:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D83D1388C017 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1590733089; bh=BdO9mCT5Fs0MPHf2U109ESN1gnLlQ6hPh8ImQ+iqb9I=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=Abme3YBsOY3z05dzGAdJ+qzjip7j1P40KIBHNGBQlUk5woLHCe3zNG1SVpe8ysc2p Ib2PeSCFUEP/ld0a/ixeEVIUXzpiS4KNOWBQ44VgBDw0HBMYLcnI/rqjjAD19NSxMk Uvy1TJ62GlAkKqqOhy/ucHVd+hngu+T3V/RjNUPM= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from avasout07.plus.net (avasout07.plus.net [84.93.230.235]) by sourceware.org (Postfix) with ESMTPS id F2F07388C013 for ; Fri, 29 May 2020 06:18:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org F2F07388C013 Received: from deneb ([80.229.24.9]) by smtp with ESMTP id eYLDj2a0L0wwMeYLEj60DI; Fri, 29 May 2020 07:18:06 +0100 X-Clacks-Overhead: "GNU Terry Pratchett" X-CM-Score: 0.00 X-CNFS-Analysis: v=2.3 cv=b/4pHuOx c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=sTwFKg_x9MkA:10 a=58IJBuuBCZZ9wiHfMZ8A:9 Received: from mac by deneb with local (Exim 4.92) (envelope-from ) id 1jeYL3-00064z-Pf; Fri, 29 May 2020 07:17:53 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH v5 5/8] libstdc++ futex: Loop when waiting against arbitrary clock Date: Fri, 29 May 2020 07:17:31 +0100 Message-Id: <3e4c0ec6864a254cbc58c32abe80983162ea7f16.1590732962.git-series.mac@mcrowe.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 X-CMAE-Envelope: MS4wfFpidXAuqVePgHf212RAPyDiVd9rva5tv1hhA8azduvAKVyWyxw0RTQ1wxWBpcgrE2G9Y1fatn99hLhL9014H5qacW8n8TWbN0xmsUG4hSZe3gF20MTr KA2jywAwSnglBgoceFImCUmxgoCkkfwMySo= X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Mike Crowe via Gcc-patches From: Mike Crowe Reply-To: Mike Crowe Cc: Mike Crowe Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" If std::future::wait_until is passed a time point measured against a clock that is neither std::chrono::steady_clock nor std::chrono::system_clock then the generic implementation of __atomic_futex_unsigned::_M_load_when_equal_until is called which calculates the timeout based on __clock_t and calls the _M_load_when_equal_until method for that clock to perform the actual wait. There's no guarantee that __clock_t is running at the same speed as the caller's clock, so if the underlying wait times out timeout we need to check the timeout against the caller's clock again before potentially looping. Also add two extra tests to the testsuite's async.cc: * run test03 with steady_clock_copy, which behaves identically to std::chrono::steady_clock, but isn't std::chrono::steady_clock. This causes the overload of __atomic_futex_unsigned::_M_load_when_equal_until that takes an arbitrary clock to be called. * invent test04 which uses a deliberately slow running clock in order to exercise the looping behaviour o __atomic_futex_unsigned::_M_load_when_equal_until described above. * libstdc++-v3/include/bits/atomic_futex.h: (__atomic_futex_unsigned) Add loop to _M_load_when_equal_until on generic _Clock to check the timeout against _Clock again after _M_load_when_equal_until returns indicating a timeout. * libstdc++-v3/testsuite/30_threads/async/async.cc: Invent slow_clock that runs at an eleventh of steady_clock's speed. Use it to test the user-supplied-clock variant of __atomic_futex_unsigned::_M_load_when_equal_until works generally with test03 and loops correctly when the timeout time hasn't been reached in test04. --- libstdc++-v3/include/bits/atomic_futex.h | 15 ++-- libstdc++-v3/testsuite/30_threads/async/async.cc | 70 +++++++++++++++++- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h index 4375129..5f95ade 100644 --- a/libstdc++-v3/include/bits/atomic_futex.h +++ b/libstdc++-v3/include/bits/atomic_futex.h @@ -229,11 +229,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_load_when_equal_until(unsigned __val, memory_order __mo, const chrono::time_point<_Clock, _Duration>& __atime) { - const typename _Clock::time_point __c_entry = _Clock::now(); - const __clock_t::time_point __s_entry = __clock_t::now(); - const auto __delta = __atime - __c_entry; - const auto __s_atime = __s_entry + __delta; - return _M_load_when_equal_until(__val, __mo, __s_atime); + typename _Clock::time_point __c_entry = _Clock::now(); + do { + const __clock_t::time_point __s_entry = __clock_t::now(); + const auto __delta = __atime - __c_entry; + const auto __s_atime = __s_entry + __delta; + if (_M_load_when_equal_until(__val, __mo, __s_atime)) + return true; + __c_entry = _Clock::now(); + } while (__c_entry < __atime); + return false; } // Returns false iff a timeout occurred. diff --git a/libstdc++-v3/testsuite/30_threads/async/async.cc b/libstdc++-v3/testsuite/30_threads/async/async.cc index 84d94cf..ee117f4 100644 --- a/libstdc++-v3/testsuite/30_threads/async/async.cc +++ b/libstdc++-v3/testsuite/30_threads/async/async.cc @@ -63,6 +63,24 @@ void test02() VERIFY( status == std::future_status::ready ); } +// This clock behaves exactly the same as steady_clock, but it is not +// steady_clock which means that the generic clock overload of +// future::wait_until is used. +struct steady_clock_copy +{ + using rep = std::chrono::steady_clock::rep; + using period = std::chrono::steady_clock::period; + using duration = std::chrono::steady_clock::duration; + using time_point = std::chrono::time_point; + static constexpr bool is_steady = true; + + static time_point now() + { + const auto steady = std::chrono::steady_clock::now(); + return time_point{steady.time_since_epoch()}; + } +}; + // This test is prone to failures if run on a loaded machine where the // kernel decides not to schedule us for several seconds. It also // assumes that no-one will warp CLOCK whilst the test is @@ -90,11 +108,63 @@ void test03() VERIFY( elapsed < std::chrono::seconds(5) ); } +// This clock is supposed to run at a tenth of normal speed, but we +// don't have to worry about rounding errors causing us to wake up +// slightly too early below if we actually run it at an eleventh of +// normal speed. It is used to exercise the +// __atomic_futex_unsigned::_M_load_when_equal_until overload that +// takes an arbitrary clock. +struct slow_clock +{ + using rep = std::chrono::steady_clock::rep; + using period = std::chrono::steady_clock::period; + using duration = std::chrono::steady_clock::duration; + using time_point = std::chrono::time_point; + static constexpr bool is_steady = true; + + static time_point now() + { + const auto steady = std::chrono::steady_clock::now(); + return time_point{steady.time_since_epoch() / 11}; + } +}; + +void test04() +{ + using namespace std::chrono; + + auto const slow_start = slow_clock::now(); + future f1 = async(launch::async, []() { + std::this_thread::sleep_for(std::chrono::seconds(2)); + }); + + // Wait for ~1s + { + auto const steady_begin = steady_clock::now(); + auto const status = f1.wait_until(slow_start + milliseconds(100)); + VERIFY(status == std::future_status::timeout); + auto const elapsed = steady_clock::now() - steady_begin; + VERIFY(elapsed >= seconds(1)); + VERIFY(elapsed < seconds(2)); + } + + // Wait for up to ~2s more + { + auto const steady_begin = steady_clock::now(); + auto const status = f1.wait_until(slow_start + milliseconds(300)); + VERIFY(status == std::future_status::ready); + auto const elapsed = steady_clock::now() - steady_begin; + VERIFY(elapsed < seconds(2)); + } +} + int main() { test01(); test02(); test03(); test03(); + test03(); + test04(); return 0; } From patchwork Fri May 29 06:17:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Crowe X-Patchwork-Id: 1300419 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=rEoEwRN5; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49YDsh4btMz9sSn for ; Fri, 29 May 2020 16:18:48 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 24969388C00D; Fri, 29 May 2020 06:18:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 24969388C00D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1590733096; bh=FyWoIRkeShujQqDRLc2fnTTZ8sRsDbh4tlVJi06ZDfM=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=rEoEwRN5WKf+oP+cZ/0lMLpP+rosaSgDtWcYJMGfeOEimrSiFHxOWGVjTWeOrj9cO Nfi2tZ+K2pFQ+DRf7rWz3WNeVLDcXieDK2IDYCK1+meGczJaFuJDnJVJmFPnoFhsIz fAkVBN7APeGXNNq3yG7BHvUeuzD07Y0wfhxokG4A= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from avasout07.plus.net (avasout07.plus.net [84.93.230.235]) by sourceware.org (Postfix) with ESMTPS id BAC3A388C024 for ; Fri, 29 May 2020 06:18:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BAC3A388C024 Received: from deneb ([80.229.24.9]) by smtp with ESMTP id eYLLj2a0d0wwMeYLMj60DP; Fri, 29 May 2020 07:18:12 +0100 X-Clacks-Overhead: "GNU Terry Pratchett" X-CM-Score: 0.00 X-CNFS-Analysis: v=2.3 cv=b/4pHuOx c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=sTwFKg_x9MkA:10 a=mDV3o1hIAAAA:8 a=WONtezP7HnKW7QDAEyIA:9 a=_FVE-zBwftR9WsbkzFJk:22 a=pHzHmUro8NiASowvMSCR:22 a=n87TN5wuljxrRezIQYnT:22 Received: from mac by deneb with local (Exim 4.92) (envelope-from ) id 1jeYL4-00065N-5y; Fri, 29 May 2020 07:17:54 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH v5 6/8] libstdc++ atomic_futex: Avoid rounding errors in std::future::wait_* [PR91486] Date: Fri, 29 May 2020 07:17:32 +0100 Message-Id: X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 X-CMAE-Envelope: MS4wfMyp395nJ1YB6mCiArcV6qJ0Dxd+ayx2UP/RO2OhTqWcZXBn8GMVV+0MgkSYBIY+KzO7L4pGOylL++PdZIzEsF8X8Au7RdP3DRUIRzyD4Hrbjb1ukeqd AckgxTfS1jRvQzwDg03+I6Acphy1G47DaAY= X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Mike Crowe via Gcc-patches From: Mike Crowe Reply-To: Mike Crowe Cc: Mike Crowe Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Convert the specified duration to the target clock's duration type before adding it to the current time in __atomic_futex_unsigned::_M_load_when_equal_for and _M_load_when_equal_until. This removes the risk of the timeout being rounded down to the current time resulting in there being no wait at all when the duration type lacks sufficient precision to hold the steady_clock current time. Rather than using the style of fix from PR68519, let's expose the C++17 std::chrono::ceil function as std::chrono::__detail::ceil so that it can be used in code compiled with earlier standards versions and simplify the fix. This was suggested by John Salmon in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91486#c5 . This problem has become considerably less likely to trigger since I switched the __atomic__futex_unsigned::__clock_t reference clock from system_clock to steady_clock and added the loop, but the consequences of triggering it have changed too. By my calculations it takes just over 194 days from the epoch for the current time not to be representable in a float. This means that system_clock is always subject to the problem (with the standard 1970 epoch) whereas steady_clock with float duration only runs out of resolution machine has been running for that long (assuming the Linux implementation of CLOCK_MONOTONIC.) The recently-added loop in __atomic_futex_unsigned::_M_load_when_equal_until turns this scenario into a busy wait. Unfortunately the combination of both of these things means that it's not possible to write a test case for this occurring in _M_load_when_equal_until as it stands. * libstdc++-v3/include/std/chrono: (__detail::ceil) Move implementation of std::chrono::ceil into private namespace so that it's available to pre-C++17 code. * libstdc++-v3/include/bits/atomic_futex.h: (__atomic_futex_unsigned::_M_load_when_equal_for, __atomic_futex_unsigned::_M_load_when_equal_until): Use __detail::ceil to convert delta to the reference clock duration type to avoid resolution problems * libstdc++-v3/testsuite/30_threads/async/async.cc: (test_pr91486): New test for __atomic_futex_unsigned::_M_load_when_equal_for. --- libstdc++-v3/include/bits/atomic_futex.h | 6 +++-- libstdc++-v3/include/std/chrono | 19 +++++++++++++---- libstdc++-v3/testsuite/30_threads/async/async.cc | 15 +++++++++++++- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h index 5f95ade..aa137a7 100644 --- a/libstdc++-v3/include/bits/atomic_futex.h +++ b/libstdc++-v3/include/bits/atomic_futex.h @@ -219,8 +219,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_load_when_equal_for(unsigned __val, memory_order __mo, const chrono::duration<_Rep, _Period>& __rtime) { + using __dur = typename __clock_t::duration; return _M_load_when_equal_until(__val, __mo, - __clock_t::now() + __rtime); + __clock_t::now() + chrono::__detail::ceil<__dur>(__rtime)); } // Returns false iff a timeout occurred. @@ -233,7 +234,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION do { const __clock_t::time_point __s_entry = __clock_t::now(); const auto __delta = __atime - __c_entry; - const auto __s_atime = __s_entry + __delta; + const auto __s_atime = __s_entry + + chrono::__detail::ceil<_Duration>(__delta); if (_M_load_when_equal_until(__val, __mo, __s_atime)) return true; __c_entry = _Clock::now(); diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono index 6d78f32..4257c7c 100644 --- a/libstdc++-v3/include/std/chrono +++ b/libstdc++-v3/include/std/chrono @@ -299,6 +299,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #endif #endif // C++20 + // We want to use ceil even when compiling for earlier standards versions + namespace __detail + { + template + constexpr __enable_if_is_duration<_ToDur> + ceil(const duration<_Rep, _Period>& __d) + { + auto __to = chrono::duration_cast<_ToDur>(__d); + if (__to < __d) + return __to + _ToDur{1}; + return __to; + } + } + #if __cplusplus >= 201703L # define __cpp_lib_chrono 201611 @@ -316,10 +330,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr __enable_if_is_duration<_ToDur> ceil(const duration<_Rep, _Period>& __d) { - auto __to = chrono::duration_cast<_ToDur>(__d); - if (__to < __d) - return __to + _ToDur{1}; - return __to; + return __detail::ceil<_ToDur>(__d); } template diff --git a/libstdc++-v3/testsuite/30_threads/async/async.cc b/libstdc++-v3/testsuite/30_threads/async/async.cc index ee117f4..f697292 100644 --- a/libstdc++-v3/testsuite/30_threads/async/async.cc +++ b/libstdc++-v3/testsuite/30_threads/async/async.cc @@ -158,6 +158,20 @@ void test04() } } +void test_pr91486() +{ + future f1 = async(launch::async, []() { + std::this_thread::sleep_for(std::chrono::seconds(1)); + }); + + std::chrono::duration const wait_time = std::chrono::seconds(1); + auto const start_steady = chrono::steady_clock::now(); + auto status = f1.wait_for(wait_time); + auto const elapsed_steady = chrono::steady_clock::now() - start_steady; + + VERIFY( elapsed_steady >= std::chrono::seconds(1) ); +} + int main() { test01(); @@ -166,5 +180,6 @@ int main() test03(); test03(); test04(); + test_pr91486(); return 0; } From patchwork Fri May 29 06:17:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Crowe X-Patchwork-Id: 1300429 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=A8Kh07ol; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49YDt84XPnz9sSn for ; Fri, 29 May 2020 16:19:12 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CAF9A388C03F; Fri, 29 May 2020 06:18:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CAF9A388C03F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1590733107; bh=e2j2V65b1rccHk75U7ukwdMfVb0IE3O5QVgnIe89XhY=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=A8Kh07olzvzlg1w90QgNLXtZnpp8o4AA5tz6FLWsNILaX0FgZMpeGuHhx7UekfaFt /XAIflBwuyk2kIVqVej86C2nirzEZozOY6NJ8R2iRjUtpwRKJmXu0ksx1mFUq8oEVk R/ux1KG2wvBLM7cJ4s58GVlyX2d48aEJaBZxJ2M8= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from avasout07.plus.net (avasout07.plus.net [84.93.230.235]) by sourceware.org (Postfix) with ESMTPS id 78C8F388C02D for ; Fri, 29 May 2020 06:18:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 78C8F388C02D Received: from deneb ([80.229.24.9]) by smtp with ESMTP id eYLTj2a0w0wwMeYLUj60DY; Fri, 29 May 2020 07:18:24 +0100 X-Clacks-Overhead: "GNU Terry Pratchett" X-CM-Score: 0.00 X-CNFS-Analysis: v=2.3 cv=b/4pHuOx c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=sTwFKg_x9MkA:10 a=6I8-TjbVYSU-v1zY1DsA:9 Received: from mac by deneb with local (Exim 4.92) (envelope-from ) id 1jeYL4-00065l-Iv; Fri, 29 May 2020 07:17:54 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH v5 7/8] libstdc++ condition_variable: Avoid rounding errors on custom clocks Date: Fri, 29 May 2020 07:17:33 +0100 Message-Id: <55a17a4c2254cbbcccb036e898aa27ace663a908.1590732962.git-series.mac@mcrowe.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 X-CMAE-Envelope: MS4wfEbWl1NxTwgD1IVbYgxoArUFWfF/4aeNyuPiZCl5+jNRH4ruJfZaJxTT9Z/PgB77SzocrYV/CB1BSc+LFYVDKrBI3P90hyVnctQmturiq0PN3uZvzcSy ffzBJbvgKa3RyIZEOkbZeKs/hcZQFYPpHpA= X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Mike Crowe via Gcc-patches From: Mike Crowe Reply-To: Mike Crowe Cc: Mike Crowe Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" The fix for PR68519 in 83fd5e73b3c16296e0d7ba54f6c547e01c7eae7b only applied to condition_variable::wait_for. This problem can also apply to condition_variable::wait_until but only if the custom clock is using a more recent epoch so that a small enough delta can be calculated. let's use the newly-added chrono::__detail::ceil to fix this and also make use of that function to simplify the previous wait_for fixes. Also, simplify the existing test case for PR68519 a little and make its variables local so we can add a new test case for the above problem. Unfortunately, the test would have only started failing if sufficient time has passed since the chrono::steady_clock epoch had passed anyway, but it's better than nothing. * libstdc++-v3/include/std/condition_variable: (condition_variable::wait_until): Convert delta to steady_clock duration before adding to current steady_clock time to avoid rounding errors described in PR68519. (condition_variable::wait_for): Simplify calculation of absolute time by using chrono::__detail::ceil in both overloads. * libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc: (test_wait_for): Renamed from test01. Replace unassigned val variable with constant false. Reduce scope of mx and cv variables to just test_wait_for function. (test_wait_until): Add new test case. --- libstdc++-v3/include/std/condition_variable | 18 +++++++++--------- libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable index 2db9dff..0796ca9 100644 --- a/libstdc++-v3/include/std/condition_variable +++ b/libstdc++-v3/include/std/condition_variable @@ -133,10 +133,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cplusplus > 201703L static_assert(chrono::is_clock_v<_Clock>); #endif + using __s_dur = typename __clock_t::duration; const typename _Clock::time_point __c_entry = _Clock::now(); const __clock_t::time_point __s_entry = __clock_t::now(); const auto __delta = __atime - __c_entry; - const auto __s_atime = __s_entry + __delta; + const auto __s_atime = __s_entry + + chrono::__detail::ceil<__s_dur>(__delta); if (__wait_until_impl(__lock, __s_atime) == cv_status::no_timeout) return cv_status::no_timeout; @@ -166,10 +168,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION const chrono::duration<_Rep, _Period>& __rtime) { using __dur = typename steady_clock::duration; - auto __reltime = chrono::duration_cast<__dur>(__rtime); - if (__reltime < __rtime) - ++__reltime; - return wait_until(__lock, steady_clock::now() + __reltime); + return wait_until(__lock, + steady_clock::now() + + chrono::__detail::ceil<__dur>(__rtime)); } template @@ -179,10 +180,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Predicate __p) { using __dur = typename steady_clock::duration; - auto __reltime = chrono::duration_cast<__dur>(__rtime); - if (__reltime < __rtime) - ++__reltime; - return wait_until(__lock, steady_clock::now() + __reltime, + return wait_until(__lock, + steady_clock::now() + + chrono::__detail::ceil<__dur>(__rtime), std::move(__p)); } diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc b/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc index 9a70713..739f74c 100644 --- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc +++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/68519.cc @@ -26,25 +26,72 @@ // PR libstdc++/68519 -bool val = false; -std::mutex mx; -std::condition_variable cv; - void -test01() +test_wait_for() { + std::mutex mx; + std::condition_variable cv; + for (int i = 0; i < 3; ++i) { std::unique_lock l(mx); auto start = std::chrono::system_clock::now(); - cv.wait_for(l, std::chrono::duration(1), [] { return val; }); + cv.wait_for(l, std::chrono::duration(1), [] { return false; }); auto t = std::chrono::system_clock::now(); VERIFY( (t - start) >= std::chrono::seconds(1) ); } } +// In order to ensure that the delta calculated in the arbitrary clock overload +// of condition_variable::wait_until fits accurately in a float, but the result +// of adding it to steady_clock with a float duration does not, this clock +// needs to use a more recent epoch. +struct recent_epoch_float_clock +{ + using rep = std::chrono::duration::rep; + using period = std::chrono::duration::period; + using time_point = std::chrono::time_point>; + static constexpr bool is_steady = true; + + static const std::chrono::steady_clock::time_point epoch; + + static time_point now() + { + const auto steady = std::chrono::steady_clock::now(); + return time_point{steady - epoch}; + } +}; + +const std::chrono::steady_clock::time_point recent_epoch_float_clock::epoch = + std::chrono::steady_clock::now(); + +void +test_wait_until() +{ + using clock = recent_epoch_float_clock; + + std::mutex mx; + std::condition_variable cv; + + for (int i = 0; i < 3; ++i) + { + std::unique_lock l(mx); + const auto start = clock::now(); + const auto wait_time = start + std::chrono::duration{1.0}; + + // In theory we could get a spurious wakeup, but in practice we won't. + const auto result = cv.wait_until(l, wait_time); + + VERIFY( result == std::cv_status::timeout ); + const auto elapsed = clock::now() - start; + VERIFY( elapsed >= std::chrono::seconds(1) ); + } +} + int main() { - test01(); + test_wait_for(); + test_wait_until(); } From patchwork Fri May 29 06:17:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Crowe X-Patchwork-Id: 1300403 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=ie/XWJLS; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49YDrv2dzWz9sSn for ; Fri, 29 May 2020 16:18:05 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 89069388A81E; Fri, 29 May 2020 06:18:02 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 89069388A81E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1590733082; bh=VR/t/xYhTGoNsgktD5mOiB1BMlh/P649TkAzaAh5bXw=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ie/XWJLSqkhm0uddmTmMOM7tNvZZcHnBmpj9aFaCQZayAkxBwOe29Nv92BNdkhohz U2RqSQxFcMrYIkmUaPuq0EqsffaCu1D4TS5nmYYGolAGFjlw8fJ0SJ4zSXMWqLsHfc Drf1Nt+ESkkvkNH7ySAtwR0XdPUYUt3Sbn4HlBNY= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from avasout02.plus.net (avasout02.plus.net [212.159.14.17]) by sourceware.org (Postfix) with ESMTPS id B9E58385DC35 for ; Fri, 29 May 2020 06:17:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B9E58385DC35 Received: from deneb ([80.229.24.9]) by smtp with ESMTP id eYL6j9hFFU8CkeYL7jognK; Fri, 29 May 2020 07:17:57 +0100 X-Clacks-Overhead: "GNU Terry Pratchett" X-CM-Score: 0.00 X-CNFS-Analysis: v=2.3 cv=G/eH7+s5 c=1 sm=1 tr=0 a=E/9URZZQ5L3bK/voZ0g0HQ==:117 a=E/9URZZQ5L3bK/voZ0g0HQ==:17 a=sTwFKg_x9MkA:10 a=bJqDUyK1CcgIiNPkY0MA:9 Received: from mac by deneb with local (Exim 4.92) (envelope-from ) id 1jeYL4-00066A-W5; Fri, 29 May 2020 07:17:55 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH v5 8/8] libstdc++: Extra async tests, not for merging Date: Fri, 29 May 2020 07:17:34 +0100 Message-Id: X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 X-CMAE-Envelope: MS4wfLE17+N+WjiEAfUN3fMrRfB8go9AhbL8p7Zp/yGTXiDT1hhfrKhK0BN6+QUkSJ2MZlQyQ+B9/48/Y/m9BxrCIqEhMlAGfi0/K4z9WlgS+ruLd8JfRC0y QmAJRnZ2AqWpmVOYzxkfMjPNW0F8K2xflNQ= X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Mike Crowe via Gcc-patches From: Mike Crowe Reply-To: Mike Crowe Cc: Mike Crowe Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" These tests show that changing the system clock has an effect on std::future::wait_until when using std::chrono::system_clock but not when using std::chrono::steady_clock. Unfortunately these tests have a number of downsides: 1. Nothing that is attempting to keep the clock set correctly (ntpd, systemd-timesyncd) can be running at the same time. 2. The test process requires the CAP_SYS_TIME capability (although, as it's written it checks for being root.) 3. Other processes running concurrently may misbehave when the clock darts back and forth. 4. They are slow to run. As such, I don't think they are suitable for merging. I include them here because I wanted to document how I had tested the changes in the previous commits. --- libstdc++-v3/testsuite/30_threads/async/async.cc | 70 +++++++++++++++++- 1 file changed, 70 insertions(+) diff --git a/libstdc++-v3/testsuite/30_threads/async/async.cc b/libstdc++-v3/testsuite/30_threads/async/async.cc index f697292..8b44810 100644 --- a/libstdc++-v3/testsuite/30_threads/async/async.cc +++ b/libstdc++-v3/testsuite/30_threads/async/async.cc @@ -24,6 +24,7 @@ #include #include +#include using namespace std; @@ -172,6 +173,71 @@ void test_pr91486() VERIFY( elapsed_steady >= std::chrono::seconds(1) ); } +void perturb_system_clock(const std::chrono::seconds &seconds) +{ + struct timeval tv; + if (gettimeofday(&tv, NULL)) + abort(); + + tv.tv_sec += seconds.count(); + if (settimeofday(&tv, NULL)) + abort(); +} + +// Ensure that advancing CLOCK_REALTIME doesn't make any difference +// when we're waiting on std::chrono::steady_clock. +void test05() +{ + auto const start = chrono::steady_clock::now(); + future f1 = async(launch::async, []() { + std::this_thread::sleep_for(std::chrono::seconds(10)); + }); + + perturb_system_clock(chrono::seconds(20)); + + std::future_status status; + status = f1.wait_for(std::chrono::seconds(4)); + VERIFY( status == std::future_status::timeout ); + + status = f1.wait_until(start + std::chrono::seconds(6)); + VERIFY( status == std::future_status::timeout ); + + status = f1.wait_until(start + std::chrono::seconds(12)); + VERIFY( status == std::future_status::ready ); + + auto const elapsed = chrono::steady_clock::now() - start; + VERIFY( elapsed >= std::chrono::seconds(10) ); + VERIFY( elapsed < std::chrono::seconds(15) ); + + perturb_system_clock(chrono::seconds(-20)); +} + +// Ensure that advancing CLOCK_REALTIME does make a difference when +// we're waiting on std::chrono::system_clock. +void test06() +{ + auto const start = chrono::system_clock::now(); + auto const start_steady = chrono::steady_clock::now(); + + future f1 = async(launch::async, []() { + std::this_thread::sleep_for(std::chrono::seconds(5)); + perturb_system_clock(chrono::seconds(60)); + std::this_thread::sleep_for(std::chrono::seconds(5)); + }); + future_status status; + status = f1.wait_until(start + std::chrono::seconds(60)); + VERIFY( status == std::future_status::timeout ); + + auto const elapsed_steady = chrono::steady_clock::now() - start_steady; + VERIFY( elapsed_steady >= std::chrono::seconds(5) ); + VERIFY( elapsed_steady < std::chrono::seconds(10) ); + + status = f1.wait_until(start + std::chrono::seconds(75)); + VERIFY( status == std::future_status::ready ); + + perturb_system_clock(chrono::seconds(-60)); +} + int main() { test01(); @@ -181,5 +247,9 @@ int main() test03(); test04(); test_pr91486(); + if (geteuid() == 0) { + test05(); + test06(); + } return 0; }