From patchwork Thu Oct 26 05:18:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Fran=C3=A7ois_Dumont?= X-Patchwork-Id: 1855607 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=ZtG4kUEX; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.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 ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4SGDc76CJWz23jh for ; Thu, 26 Oct 2023 16:18:57 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5AE4E3856DDA for ; Thu, 26 Oct 2023 05:18:55 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by sourceware.org (Postfix) with ESMTPS id 4D31F3858D37; Thu, 26 Oct 2023 05:18:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4D31F3858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4D31F3858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::32c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698297523; cv=none; b=C5mUaiVX6PiFrGIXMRrXqgXcrP4LAS4P8fV9WcpQF2EwXoNl2DtFXF8tvFlN3FUHuZfx2ryFvF2Mm4N8wbHhliDbJXDZIbQmi951cUxjiJ0eUiPRPm+EpzpTdFwrD30eyN6/yoVzx/O+yHdhblTC+CaLgMJl7TUMRZfu8FFzpJA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698297523; c=relaxed/simple; bh=wJRQrF8Q56kL+oS6tt+hlGKWciFDeamZlbINeWC7i6Y=; h=DKIM-Signature:Message-ID:Date:MIME-Version:To:From:Subject; b=WQhMz15eDsBy7T+B63BOCER1rKomFDoo/zRWOUUzgmGXM5aAVm7Ba9r+RLT43eNVKz0ca+YV1OavEY8UVDeCDJ+8mXupP5t76XtgMx5wPAoeFagke+BCVG3brPRWvQtynF6B9QVVeJlq2ya58xR3cn0JK2Q7oHgL5I+zjUGilPA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-x32c.google.com with SMTP id 5b1f17b1804b1-4083cd3917eso3787415e9.3; Wed, 25 Oct 2023 22:18:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1698297520; x=1698902320; darn=gcc.gnu.org; h=subject:from:cc:to:content-language:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=IehNhqnCbbZErt24eNF7AxMBjUIndMQARqW2zMaoMV4=; b=ZtG4kUEX82FL3mQfLPe5QkEWzzGTOPGsfXQWkv8E2sAAt/eqaEQpkXFQC/K8tgOHKN D0256rObd8frquSO9CS4LA/KEZbtmhkZnIphj3pjKcik5Umo9epjl6+aUlop0WP3Km/I vFYYubWYHM3le7PP2L9mixb16dTw4NdltSClpJvTqzzoDEjqkqZO8ivhPOTLbrBgCj8Z LV8ssl0VhkHqicfYOdAzQjFlAMRZF/3Mz+N0hqWHUh9H2QIvyLElOhLYqkWDnuw21lxE vKrd0fBTxP5OYjmoF5hmcZy1dH2CyJgC/jGY0ibtEzExyZHQZ4tZCM7iizlLZw4OdlVS J3dQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698297520; x=1698902320; h=subject:from:cc:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=IehNhqnCbbZErt24eNF7AxMBjUIndMQARqW2zMaoMV4=; b=pwViYSGk0Ja6SctKISFcNTIRetSRRGIVQa/kKqyAXa3qBFfhBFz5LT/Qa+cIPqeHop aZZhLjTpSk6zriY2KxiInFGOWV076o8vc2TOM5HzMdwSwDtQhUZYbmdw4iTOm91YllsU pQ38nUuQVlPS1vlEiUXUZzCEGaW/w20fRlrzJoMLhU5iZuUIw/vSm2IcAs1WMqogUuIu gwtSRnU3qNH1IYa6zTmqxF+0I4ae4ImBLy6lZHxG/+yqiYyOSO3v8+WFXAFTkaTyRhAb 7DRZOjw00f23kU3z1ghElLT39Jq52owb11Q/sjhKscVocEM7QHBB4qNqGeScpMV38PA9 qRcA== X-Gm-Message-State: AOJu0YxhJ0+6CWB2HBEpvegRl47VyzX/R1E1TbUAoSsT/JIxwqm20lId NS0WKKmSDlY4uLkT8B30bCWtbPykHtM= X-Google-Smtp-Source: AGHT+IHNSLZ2qokUiB8RKXHyvsUxBcKCCk0QHaXNizRUu2vmAkNTKLsdIizpSK7ht5xmTp8bBEtdcQ== X-Received: by 2002:a05:600c:a01:b0:406:53c0:3c71 with SMTP id z1-20020a05600c0a0100b0040653c03c71mr13936388wmp.37.1698297519320; Wed, 25 Oct 2023 22:18:39 -0700 (PDT) Received: from [10.25.0.75] ([89.207.171.100]) by smtp.gmail.com with ESMTPSA id h12-20020adff18c000000b0032d402f816csm13229932wro.98.2023.10.25.22.18.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 25 Oct 2023 22:18:38 -0700 (PDT) Message-ID: <7f61df18-dd99-4ff5-9fcd-8ca7820403d4@gmail.com> Date: Thu, 26 Oct 2023 07:18:36 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: libstdc++ Cc: gcc-patches From: =?utf-8?q?Fran=C3=A7ois_Dumont?= Subject: [PATCH][_Hashtable] Use RAII to restore Rehash state X-Spam-Status: No, score=-8.9 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org libstdc++: [_Hashtable] Use RAII type to manage rehash functor state     Replace usage of __try/__catch with a RAII type to restore rehash functor     state when needed.     libstdc++-v3/ChangeLog:             * include/bits/hashtable_policy.h (_RehashStateGuard): New.             (_Insert_base<>::_M_insert_range(_IIt, _IIt, const _NodeGet&, false_type)):             Adapt.             * include/bits/hashtable.h (__rehash_guard_t): New.             (__rehash_state): Remove.             (_M_rehash): Remove.             (_M_rehash_aux): Rename into _M_rehash.             (_M_assign_elements, _M_insert_unique_node, _M_insert_multi_node): Adapt.             (rehash): Adapt. Tested under Linux x64. Ok to commit ? François diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h index 0857448f7ed..64071ac1fb2 100644 --- a/libstdc++-v3/include/bits/hashtable.h +++ b/libstdc++-v3/include/bits/hashtable.h @@ -234,6 +234,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _RehashPolicy, _Traits>; using __enable_default_ctor = _Hashtable_enable_default_ctor<_Equal, _Hash, _Alloc>; + using __rehash_guard_t + = __detail::_RehashStateGuard<_RehashPolicy>; public: typedef _Key key_type; @@ -264,7 +266,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION private: using __rehash_type = _RehashPolicy; - using __rehash_state = typename __rehash_type::_State; using __unique_keys = typename __traits_type::__unique_keys; @@ -1200,14 +1201,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION private: // Helper rehash method used when keys are unique. - void _M_rehash_aux(size_type __bkt_count, true_type __uks); + void _M_rehash(size_type __bkt_count, true_type __uks); // Helper rehash method used when keys can be non-unique. - void _M_rehash_aux(size_type __bkt_count, false_type __uks); - - // Unconditionally change size of bucket array to n, restore - // hash policy state to __state on exception. - void _M_rehash(size_type __bkt_count, const __rehash_state& __state); + void _M_rehash(size_type __bkt_count, false_type __uks); }; // Definitions of class template _Hashtable's out-of-line member functions. @@ -1337,7 +1334,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { __buckets_ptr __former_buckets = nullptr; std::size_t __former_bucket_count = _M_bucket_count; - const __rehash_state& __former_state = _M_rehash_policy._M_state(); + __rehash_guard_t __rehash_guard(_M_rehash_policy); if (_M_bucket_count != __ht._M_bucket_count) { @@ -1359,6 +1356,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_assign(std::forward<_Ht>(__ht), __roan); if (__former_buckets) _M_deallocate_buckets(__former_buckets, __former_bucket_count); + __rehash_guard._M_reset = false; } __catch(...) { @@ -1366,7 +1364,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { // Restore previous buckets. _M_deallocate_buckets(); - _M_rehash_policy._M_reset(__former_state); _M_buckets = __former_buckets; _M_bucket_count = __former_bucket_count; } @@ -2142,17 +2139,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __node_ptr __node, size_type __n_elt) -> iterator { - const __rehash_state& __saved_state = _M_rehash_policy._M_state(); + __rehash_guard_t __rehash_guard(_M_rehash_policy); std::pair __do_rehash = _M_rehash_policy._M_need_rehash(_M_bucket_count, _M_element_count, __n_elt); if (__do_rehash.first) { - _M_rehash(__do_rehash.second, __saved_state); + _M_rehash(__do_rehash.second, true_type{}); __bkt = _M_bucket_index(__code); } + __rehash_guard._M_reset = false; this->_M_store_code(*__node, __code); // Always insert at the beginning of the bucket. @@ -2172,13 +2170,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __hash_code __code, __node_ptr __node) -> iterator { - const __rehash_state& __saved_state = _M_rehash_policy._M_state(); + __rehash_guard_t __rehash_guard(_M_rehash_policy); std::pair __do_rehash = _M_rehash_policy._M_need_rehash(_M_bucket_count, _M_element_count, 1); if (__do_rehash.first) - _M_rehash(__do_rehash.second, __saved_state); + _M_rehash(__do_rehash.second, false_type{}); + __rehash_guard._M_reset = false; this->_M_store_code(*__node, __code); const key_type& __k = _ExtractKey{}(__node->_M_v()); size_type __bkt = _M_bucket_index(__code); @@ -2509,39 +2508,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>:: rehash(size_type __bkt_count) { - const __rehash_state& __saved_state = _M_rehash_policy._M_state(); + __rehash_guard_t __rehash_guard(_M_rehash_policy); __bkt_count = std::max(_M_rehash_policy._M_bkt_for_elements(_M_element_count + 1), __bkt_count); __bkt_count = _M_rehash_policy._M_next_bkt(__bkt_count); if (__bkt_count != _M_bucket_count) - _M_rehash(__bkt_count, __saved_state); - else - // No rehash, restore previous state to keep it consistent with - // container state. - _M_rehash_policy._M_reset(__saved_state); - } - - template - void - _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, - _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>:: - _M_rehash(size_type __bkt_count, const __rehash_state& __state) - { - __try - { - _M_rehash_aux(__bkt_count, __unique_keys{}); - } - __catch(...) { - // A failure here means that buckets allocation failed. We only - // have to restore hash policy previous state. - _M_rehash_policy._M_reset(__state); - __throw_exception_again; + _M_rehash(__bkt_count, __unique_keys{}); + __rehash_guard._M_reset = false; } } @@ -2553,7 +2529,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>:: - _M_rehash_aux(size_type __bkt_count, true_type /* __uks */) + _M_rehash(size_type __bkt_count, true_type /* __uks */) { __buckets_ptr __new_buckets = _M_allocate_buckets(__bkt_count); __node_ptr __p = _M_begin(); @@ -2596,7 +2572,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal, _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>:: - _M_rehash_aux(size_type __bkt_count, false_type /* __uks */) + _M_rehash(size_type __bkt_count, false_type /* __uks */) { __buckets_ptr __new_buckets = _M_allocate_buckets(__bkt_count); __node_ptr __p = _M_begin(); diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h index 5d162463dc3..8b9626b1575 100644 --- a/libstdc++-v3/include/bits/hashtable_policy.h +++ b/libstdc++-v3/include/bits/hashtable_policy.h @@ -715,6 +715,26 @@ namespace __detail std::size_t _M_next_resize; }; + template + struct _RehashStateGuard + { + _RehashPolicy& _M_rehash_policy; + typename _RehashPolicy::_State _M_prev_state; + bool _M_reset = true; + + _RehashStateGuard(_RehashPolicy& __policy) + : _M_rehash_policy(__policy) + , _M_prev_state(__policy._M_state()) + { } + _RehashStateGuard(const _RehashStateGuard&) = delete; + + ~_RehashStateGuard() + { + if (_M_reset) + _M_rehash_policy._M_reset(_M_prev_state); + } + }; + // Base classes for std::_Hashtable. We define these base classes // because in some cases we want to do different things depending on // the value of a policy class. In some cases the policy class @@ -1007,7 +1027,7 @@ namespace __detail const _NodeGetter& __node_gen, false_type __uks) { using __rehash_type = typename __hashtable::__rehash_type; - using __rehash_state = typename __hashtable::__rehash_state; + using __rehash_guard_t = typename __hashtable::__rehash_guard_t; using pair_type = std::pair; size_type __n_elt = __detail::__distance_fw(__first, __last); @@ -1016,14 +1036,15 @@ namespace __detail __hashtable& __h = _M_conjure_hashtable(); __rehash_type& __rehash = __h._M_rehash_policy; - const __rehash_state& __saved_state = __rehash._M_state(); + __rehash_guard_t __rehash_guard(__rehash); pair_type __do_rehash = __rehash._M_need_rehash(__h._M_bucket_count, __h._M_element_count, __n_elt); if (__do_rehash.first) - __h._M_rehash(__do_rehash.second, __saved_state); + __h._M_rehash(__do_rehash.second, __uks); + __rehash_guard._M_reset = false; for (; __first != __last; ++__first) __h._M_insert(*__first, __node_gen, __uks); }