[v3] Fix management of non empty hash functor

[François Dumont]

Hi

     Here is an other version of this patch. Indeed there were no need 
to expose many stuff public. Inheriting from _Hash_code_base is fine, it 
is not final and it deals with EBO itself. I only kept usage of 
_Hashtable_ebo_helper when embedding H2 functor. As it is an extension 
we could have impose it not to be final but it doesn't cost a lot to 
deal with it. Finally I only needed a single friend declaration to get 
access to the H2 part of _Hash_code_base.

     I didn't touch the default cache policy for the moment except 
reducing constraints on the hash functor. I prefer to submit an other 
patch to change when we cache or not depending on the hash functor 
expected performance.

     I also took the time to replace some typedef expressions with using 
ones. I really know what is the rule about using one or the other but I 
remembered that Benjamin spent quite some time changing typedef in using 
so I prefer to stick to this approach in this file, even if there are 
still some typedef left.

     Tested under linux x86_64 normal and debug modes.

2013-01-10  François Dumont  <fdumont@gcc.gnu.org>

     * include/bits/hashtable_policy.h (_Local_iterator_base): Use
     _Hashtable_ebo_helper to embed necessary functors into the
     local_iterator when necessary. Pass information about functors
     involved in hash code by copy.
     * include/bits/hashtable.h (__cache_default): Do not cache for
     builtin integral types unless the hash functor is not noexcept
     qualified or is not default constructible. Adapt static assertions
     and local iteraror instantiations.
     * include/debug/unordered_set
     (std::__debug::unordered_set<>::erase): Detect local iterators to
     invalidate using contained node rather than generating a dummy
     local_iterator instance.
     (std::__debug::unordered_multiset<>::erase): Likewise.
     * include/debug/unordered_map
     (std::__debug::unordered_map<>::erase): Likewise.
     (std::__debug::unordered_multimap<>::erase): Likewise.
     * testsuite/performance/23_containers/insert_erase/41975.cc: Test
     std::tr1 and std versions of unordered_set regardless of any
     macro. Add test on default cache behavior.
     * testsuite/performance/23_containers/insert/54075.cc: Likewise.
     * testsuite/23_containers/unordered_set/instantiation_neg.cc:
     Adapt line number.
     * testsuite/23_containers/unordered_set/
     not_default_constructible_hash_neg.cc: New.
     * testsuite/23_containers/unordered_set/buckets/swap.cc: New.

If you agree with the patch tell me where and when to apply it.

François


On 01/04/2013 12:17 PM, Paolo Carlini wrote:
> Hi,
>
> On 12/13/2012 10:32 PM, François Dumont wrote:
>> Hi
>>
>>     As part of a performance patch proposed in an other mailing 
>> thread was a patch to improve management of hash functor with state. 
>> This part is I think less sensible than the performance patch so I 
>> propose it independently. I only would like to commit the 
>> modification on the performance tests here if you don't mind.
>>
>>     Thanks to this patch caching the hash code or not doesn't depend 
>> on the hash functor to be empty of final anymore. I only keep the 
>> default constructible condition so that local_iterator can be default 
>> constructible, considering it is a Standard request.
> I'm finally having a closer look at this work of yours (sorry aboutt 
> the delay!) and I think we want something similar for 4.8.0. However, 
> to be honest, I'm not convinced we are implementing the general idea 
> in the best way, in particular I don't like the much more complex 
> access control structure, _Hash_code_base loses encapsulation, etc. 
> Did you consider maybe adding friend declarations in a few places?
>
> Jon, do you have suggestiong? The idea of managing to get rid of the 
> empty & !final requirement for dispatching seems right to me.
>
> By the way, I'm also not convinced that is_integral is the right 
> category, I think is_scalar for example is better: pointers are common 
> and very similar in terms of std::hash, likewise floating point 
> quantities (with the possible exception of long double, but I don't 
> think we should spend time on it).
>
> Paolo.
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: hashtable.patch
Type: text/x-patch
Size: 43209 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20130110/486f37dd/attachment.bin>