From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85918 invoked by alias); 19 Sep 2018 11:07:29 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 85891 invoked by uid 89); 19 Sep 2018 11:07:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Sep 2018 11:07:26 +0000 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 980848CD0; Wed, 19 Sep 2018 11:07:25 +0000 (UTC) Received: from localhost (unknown [10.33.36.94]) by smtp.corp.redhat.com (Postfix) with ESMTP id 41F68308BDA0; Wed, 19 Sep 2018 11:07:25 +0000 (UTC) Date: Wed, 19 Sep 2018 11:21:00 -0000 From: Jonathan Wakely To: =?iso-8859-1?Q?Fran=E7ois?= Dumont Cc: "libstdc++@gcc.gnu.org" , gcc-patches Subject: Re: [PATCH] PR libstdc++/87135 Rehash only when necessary (LWG2156) Message-ID: <20180919110724.GC23172@redhat.com> References: <1ad7ad86-0f0c-e8b0-8f29-2b5303718988@gmail.com> <20180918084159.GV23172@redhat.com> <712c7e85-fd7b-5849-85b6-14784266d567@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <712c7e85-fd7b-5849-85b6-14784266d567@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.9.2 (2017-12-15) X-SW-Source: 2018-09/txt/msg01049.txt.bz2 On 18/09/18 22:39 +0200, François Dumont wrote: >On 09/18/2018 10:41 AM, Jonathan Wakely wrote: >>On 13/09/18 07:49 +0200, François Dumont wrote: >>>All changes limited to hashtable_c++0x.cc file. >>> >>>_Prime_rehash_policy::_M_next_bkt now really does what its comment >>>was declaring that is to say: >>>  // Return a prime no smaller than n. >>> >>>_Prime_rehash_policy::_M_need_rehash rehash only when _M_next_size >>>is exceeded, not only when it is reach. >>> >>>    PR libstdc++/87135 >>>    * src/c++11/hashtable_c++0x.cc: >>>    (_Prime_rehash_policy::_M_next_bkt): Return a prime no smaller than >>>    requested size, but not necessarily greater. >>>    (_Prime_rehash_policy::_M_need_rehash): Rehash only if target >>>size is >>>    strictly greater than next resize threshold. >>>    * testsuite/23_containers/unordered_map/modifiers/reserve.cc: >>>Adapt test >>>    to validate that there is no rehash as long as number of >>>insertion is >>>    lower or equal to the reserved number of elements. >>> >>>unordered_map tests successful, ok to commit once all other tests >>>completed ? >>> >>>François >> >>>diff --git a/libstdc++-v3/src/c++11/hashtable_c++0x.cc >>>b/libstdc++-v3/src/c++11/hashtable_c++0x.cc >>>index a776a8506fe..ec6031b3f5b 100644 >>>--- a/libstdc++-v3/src/c++11/hashtable_c++0x.cc >>>+++ b/libstdc++-v3/src/c++11/hashtable_c++0x.cc >>>@@ -46,10 +46,10 @@ namespace __detail >>>  { >>>    // Optimize lookups involving the first elements of __prime_list. >>>    // (useful to speed-up, eg, constructors) >>>-    static const unsigned char __fast_bkt[13] >>>-      = { 2, 2, 3, 5, 5, 7, 7, 11, 11, 11, 11, 13, 13 }; >>>+    static const unsigned char __fast_bkt[] >>>+      = { 2, 2, 2, 3, 5, 5, 7, 7, 11, 11, 11, 11, 13, 13 }; >>> >>>-    if (__n <= 12) >>>+    if (__n < sizeof(__fast_bkt) / sizeof(unsigned char)) >> >>sizeof(unsigned char) is defined to be 1, always. > >I also though it was overkill but there are so many platforms that I >prefer to be caution. Good to know that it can't be otherwise. > >> >>I think this should be just sizeof(__fast_bkt), or if you're trying to >>guard against the type of __fast_bkt changing, then use >>sizeof(__fast_bkt) / sizeof(__fast_bkt[0])) > >I committed with simply sizeof(__fast_bkt) This caused some testsuite regressions: FAIL: 23_containers/unordered_set/hash_policy/71181.cc execution test /home/jwakely/src/gcc/gcc/libstdc++-v3/testsuite/23_containers/unordered_set/hash_policy/71181.cc:42: void test(int) [with _USet = std::unordered_set]: Assertion 'us.bucket_count() == bkts' failed. FAIL: 23_containers/unordered_set/hash_policy/load_factor.cc execution test /home/jwakely/src/gcc/gcc/libstdc++-v3/testsuite/23_containers/unordered_set/hash_policy/load_factor.cc:50: void test() [with _USet = std::unordered_set]: Assertion 'us.load_factor() <= us.max_load_factor()' failed. FAIL: 23_containers/unordered_set/hash_policy/prime_rehash.cc execution test /home/jwakely/src/gcc/gcc/libstdc++-v3/testsuite/23_containers/unordered_set/hash_policy/prime_rehash.cc:37: void test01(): Assertion 'nxt == policy._M_next_bkt(mx)' failed.