From patchwork Fri Jan 6 21:21:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 40268 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp1031700wrt; Fri, 6 Jan 2023 13:23:44 -0800 (PST) X-Google-Smtp-Source: AMrXdXvznuy+is661OIuJZleqAU71dUvow/JRhUF74QzPk5AEJk/1H+MM9HkFeEuFc5Cf3k3uVlE X-Received: by 2002:a17:906:d216:b0:7ae:ccf3:7be5 with SMTP id w22-20020a170906d21600b007aeccf37be5mr63568019ejz.32.1673040224145; Fri, 06 Jan 2023 13:23:44 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1673040224; cv=none; d=google.com; s=arc-20160816; b=AEa1ev/7UFv0k+soUKTNaydtHkraIHwkGj3wiyl8ZkB+oxOmU/C0ZA6U8ocNlKQRHA 4eHNP1/65ZmiRhJB3tCuYsequJktNlljBTjo/H8T9MEf5GOhG6P1slBbLer1Z13lP1uP 1orHVWqh/QdgsVu32+jpfz7a3Oa39yp6uXOYM2RlLkc+yUIoX+lw93FdnfRJB2sd/ydK Rm1lC0AwigosOer7y9abczsf2sEMSGXrZfS6hdW1or4cJSBBATVEi3I1ZEdGSZvp/qtc uG5BBhqOJAfLuUhIaDNagUYTCFTKyBiijXc1CDN8DRrebej1DNtQhe+p3mz9sRfTzA9I ddlQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=svXrH66mpuAIzJKscO6lAPrgIm2l9c0+P+nFZtAE19g=; b=O1SASy/u0L+/BdX+zvoEu5Jw1UpHNIT1SHujxUyE2/HbTsRZ+ybnCEsTbS5VGfuyno fLvaG0v/EgT95fPqH7DqhXGEJT8TQssjguLddu4/wcd/yvjeIogZMilFMMS2biPy+5Ej e19wdzhNQwF+C0bLSsYLRvQUR2VkmZ2cttic7rusEw2bjy45ki9WNZaa5n3GBzSEoS0H LuiAIm1eMaryy5aipL9/8k1bbePySAwiVG4dQt9fOy//H07pBB7CmAyGwk3xqC+JSeRx 6kutGwA393/kiCxvU0oMS9KCU+0oBZIuLnONS2xOnTL1EGJecG49Lc6zmLhiUUe9EgTD 1cFw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=vXCeGkMG; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id sa15-20020a1709076d0f00b0084c464789absi2320710ejc.781.2023.01.06.13.23.43 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Jan 2023 13:23:44 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=vXCeGkMG; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B3D1C3858439 for ; Fri, 6 Jan 2023 21:22:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B3D1C3858439 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1673040175; bh=svXrH66mpuAIzJKscO6lAPrgIm2l9c0+P+nFZtAE19g=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=vXCeGkMGKcVzUE6d+PB5Kn181pcB8lbSRqGVlQ5UfUKRnQnHvK1IoxXMfT9/ef2/d KIkMeHIdAAYOtTk0EFNeFGq/NqepHL9AIDV7PdDXwi+NbxLjRu8qF0fFlnWVwsFVvn cR3xZRxvtWSgfBGoNCNiYkcYCB/DMra1YZlWuDVk= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 20465385B502 for ; Fri, 6 Jan 2023 21:21:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 20465385B502 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-315-DuD59JuBNk-b0NPAKCdKWQ-1; Fri, 06 Jan 2023 16:21:30 -0500 X-MC-Unique: DuD59JuBNk-b0NPAKCdKWQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4E761299E759; Fri, 6 Jan 2023 21:21:30 +0000 (UTC) Received: from localhost (unknown [10.33.36.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id 147DD51FF; Fri, 6 Jan 2023 21:21:29 +0000 (UTC) To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Refactor time_zone::_Impl::rules_counter [PR108235] Date: Fri, 6 Jan 2023 21:21:29 +0000 Message-Id: <20230106212129.397061-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Jonathan Wakely via Gcc-patches From: Jonathan Wakely Reply-To: Jonathan Wakely Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1754309825695779013?= X-GMAIL-MSGID: =?utf-8?q?1754309825695779013?= Tested x86_64-linux and powerpc-aix. Pushed to trunk. -- >8 -- Abstract the atomic counter used to synchronize access to time_zone infos behind a Lockable class API, and use atomic_signed_lock_free instead of atomic, as that should be the most efficient type. (For futex-supporting targets it makes no difference, but might benefit other targets in future.) The new API allows the calling code to be simpler, without needing to repeat the same error prone preprocessor conditions in multiple places. It also allows using template metaprogramming to decide whether to use the atomic or a mutex, which gives us more flexibility than only using preprocessor conditions. That allows us to choose the mutex implementation for targets such as hppa-hp-hpux11.11 where 32-bit atomics are not lock-free and so would introduce an unwanted dependency on libatomic. libstdc++-v3/ChangeLog: PR libstdc++/108235 * src/c++20/tzdb.cc (time_zone::_Impl::RulesCounter): New class template and partial specialization for synchronizing access to time_zone::_Impl::infos. (time_zone::_M_get_sys_info, reload_tzdb): Adjust uses of rules_counter. --- libstdc++-v3/src/c++20/tzdb.cc | 137 ++++++++++++++++++++------------- 1 file changed, 84 insertions(+), 53 deletions(-) diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc index 9103b159400..fa4f4c7a30c 100644 --- a/libstdc++-v3/src/c++20/tzdb.cc +++ b/libstdc++-v3/src/c++20/tzdb.cc @@ -29,7 +29,7 @@ #include // ifstream #include // istringstream #include // ranges::upper_bound, ranges::lower_bound, ranges::sort -#include // atomic, atomic +#include // atomic, atomic #include // atomic> #include // mutex #include // filesystem::read_symlink @@ -598,13 +598,86 @@ namespace std::chrono // Needed to access the list of rules for the time zones. weak_ptr node; -#ifndef __GTHREADS - // Don't need synchronization for accessing the infos vector. -#elif __cpp_lib_atomic_wait - atomic rules_counter; -#else - mutex infos_mutex; + // In the simple case, we don't actual keep count. No concurrent access + // to the infos vector is possible, even if all infos are expanded. + template + struct RulesCounter + { + // Called for each rule-based ZoneInfo added to the infos vector. + // Called when the time_zone::_Impl is created, so no concurrent calls. + void increment() { } + // Called when a rule-based ZoneInfo is expanded. + // The caller must have called lock() for exclusive access to infos. + void decrement() { } + + // Use a mutex to synchronize all access to the infos vector. + mutex infos_mutex; + + void lock() { infos_mutex.lock(); } + void unlock() { infos_mutex.unlock(); } + }; + +#if defined __GTHREADS && __cpp_lib_atomic_wait + // Atomic count of unexpanded ZoneInfo objects in the infos vector. + // Concurrent access is allowed when all objects have been expanded. + // Only use an atomic counter if it would not require libatomic, + // because we don't want libstdc++.so to depend on libatomic. + template requires _Tp::is_always_lock_free + struct RulesCounter<_Tp> + { + atomic_signed_lock_free counter{0}; + + void + increment() + { counter.fetch_add(1, memory_order::relaxed); } + + void + decrement() + { + // The current thread holds the lock, so the counter is negative + // and so we need to increment it to decrement it! + // If the count reaches zero then there are no more unexpanded infos, + // so notify all waiting threads that they can access the infos. + // We must do this here, because unlock() is a no-op if counter==0. + if (++counter == 0) + counter.notify_all(); + } + + void + lock() + { + // If counter is zero then concurrent access is allowed, so lock() + // and unlock() are no-ops and multiple threads can "lock" at once. + // If counter is non-zero then the contents of the infos vector might + // need to be changed, so only one thread is allowed to access it. + for (auto c = counter.load(memory_order::relaxed); c != 0;) + { + // Setting counter to negative means this thread has the lock. + if (c > 0 && counter.compare_exchange_strong(c, -c)) + return; + + if (c < 0) + { + // Counter is negative, another thread already has the lock. + counter.wait(c); + c = counter.load(); + } + } + } + + void + unlock() + { + if (auto c = counter.load(memory_order::relaxed); c < 0) + { + counter.store(-c, memory_order::release); + counter.notify_one(); + } + } + }; #endif + + RulesCounter rules_counter; }; namespace @@ -666,46 +739,8 @@ namespace std::chrono const auto node = _M_impl->node.lock(); auto& infos = _M_impl->infos; -#ifndef __GTHREADS -#elif __cpp_lib_atomic_wait // Prevent concurrent access to _M_impl->infos if it might need to change. - struct Lock - { - Lock(atomic& counter) : counter(counter) - { - // If counter is non-zero then the contents of _M_impl->info might - // need to be changed, so only one thread is allowed to access it. - for (auto c = counter.load(memory_order::relaxed); c != 0;) - { - // Setting counter to negative means this thread has the lock. - if (c > 0 && counter.compare_exchange_strong(c, -c)) - return; - - if (c < 0) - { - // Counter is negative, another thread already has the lock. - counter.wait(c); - c = counter.load(); - } - } - } - - ~Lock() - { - if (auto c = counter.load(memory_order::relaxed); c < 0) - { - counter.store(-c, memory_order::release); - counter.notify_one(); - } - } - - atomic& counter; - }; - Lock lock{_M_impl->rules_counter}; -#else - // Keep it simple, just use a mutex for all access. - lock_guard lock(_M_impl->infos_mutex); -#endif + lock_guard lock(_M_impl->rules_counter); // Find the transition info for the time point. auto i = ranges::upper_bound(infos, tp, ranges::less{}, &ZoneInfo::until); @@ -894,10 +929,8 @@ namespace std::chrono std::rotate(infos.begin() + result_index + 1, i, infos.end()); // Then replace the original rules_info object with new_infos.front(): infos[result_index] = ZoneInfo(new_infos.front()); -#if defined __GTHREADS && __cpp_lib_atomic_wait - if (++_M_impl->rules_counter == 0) // No more unexpanded infos. - _M_impl->rules_counter.notify_all(); -#endif + // Decrement count of rule-based infos (might also release lock). + _M_impl->rules_counter.decrement(); } return info; @@ -1339,11 +1372,9 @@ namespace std::chrono ZoneInfo& info = impl.infos.emplace_back(); is >> info; -#if defined __GTHREADS && __cpp_lib_atomic_wait // Keep count of ZoneInfo objects that refer to named Rules. if (!info.rules().empty()) - impl.rules_counter.fetch_add(1, memory_order::relaxed); -#endif + impl.rules_counter.increment(); } } }