From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 1F1A53890401 for ; Mon, 24 May 2021 11:19:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1F1A53890401 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-183-QhFpf67tNDibF0LttBU6Vg-1; Mon, 24 May 2021 07:19:42 -0400 X-MC-Unique: QhFpf67tNDibF0LttBU6Vg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id F300D1013720; Mon, 24 May 2021 11:19:40 +0000 (UTC) Received: from localhost (unknown [10.33.37.15]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8E66A5C541; Mon, 24 May 2021 11:19:40 +0000 (UTC) Date: Mon, 24 May 2021 12:19:39 +0100 From: Jonathan Wakely To: =?iso-8859-1?Q?Fran=E7ois?= Dumont Cc: Jonathan Wakely , libstdc++ , gcc-patches Subject: Re: [PATCH] Hashtable PR96088 Message-ID: References: <5b198dd5-cb89-91a0-9070-13927ac263a4@gmail.com> <524e2eee-a4ee-a05e-087f-6000c8274eff@gmail.com> <21854fd0-ad6b-1eaa-adaa-2074421fc107@gmail.com> <7bd748f6-77bd-fdcc-f925-1700ac9da3de@gmail.com> <50b5f8f5-56a3-57a5-2e03-b23118a1a2c5@gmail.com> MIME-Version: 1.0 In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_NUMSUBJECT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, 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: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 May 2021 11:19:45 -0000 On 22/05/21 18:35 +0200, François Dumont wrote: >This was indeed the right approach. > >    The only minor drawback is that __is_noexcept_invocable<> combines >noexcept qualification of the conversion and of the hash functor. So >if the hash functor is not noexcept we could end up creating >temporaries for nothing. > >    So I've eventually used this condition: > >__and_<__is_nothrow_invocable<_Hash&, const key_type&>, >         __not_<__is_nothrow_invocable<_Hash&, _Kt>>>::value, > >    so that we do not create a temporary key_type if invoking _Hash >with it can still throw. > >    libstdc++: Limit allocation on iterator insertion in Hashtable [PR >96088] > >    When inserting into unordered_multiset or unordered_multimap first >instantiate >    the node to store and compute the hash code from it to avoid a >potential >    intermediate key_type instantiation. > >    When inserting into unordered_set or unordered_map check if >invoking the hash >    functor with container key_type is noexcept and invoking the same >hash functor >    with key part of the iterator value_type can throw. In this case >create a >    temporary key_type instance at Hashtable level and use it to >compute the hash >    code. This temporary instance is moved to the final storage >location if needed. > >    libstdc++-v3/ChangeLog: > >            PR libstdc++/96088 >            * include/bits/hashtable_policy.h (_Select2nd): New. >            (_NodeBuilder<>): New. >            (_ReuseOrAllocNode<>::operator()): Use variadic template args. >            (_AllocNode<>::operator()): Likewise. >            * include/bits/hashtable.h >            (_Hashtable<>::__node_builder_t): New. >(_Hashtable<>::_M_insert_unique<>(_Kt&&, _Arg&&, const _NodeGenerator&)): >             New. >            (_Hashtable<>::_S_forward_key): New. >            (_Hashtable<>::_M_insert): Use latter. >            (_Hashtable<>::_M_insert(const_iterator, _Arg&&, const >_NodeGenerator&, false_type)): >            Instantiate node first, compute hash code second. >            * testsuite/23_containers/unordered_map/96088.cc: New test. >            * testsuite/23_containers/unordered_multimap/96088.cc: New >test. >            * testsuite/23_containers/unordered_multiset/96088.cc: New >test. >            * testsuite/23_containers/unordered_set/96088.cc: New test. >            * testsuite/util/replacement_memory_operators.h >            (counter::_M_increment): New. >            (counter::_M_decrement): New. >            (counter::reset()): New. > >Tested under Linux x64. > >Ok to commit ? OK for trunk, thanks.