[v3] Fix management of non empty hash functor

[François Dumont]

Attached patch applied.

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

     * include/bits/hashtable_policy.h (_Local_iterator_base): Use
     _Hashtable_ebo_helper to embed 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 iterator 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.

On 01/28/2013 04:42 PM, Jonathan Wakely wrote:
> On 10 January 2013 21:02, François Dumont wrote:
>> 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.
> OK.
>
>>      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.
> OK.  The reduced constraints are good.  Does this actually affect
> performance?  In my tests it doesn't, so I assume we still need to
> change the caching decision to notice any performance improvements?

No performance gain plan with that patch indeed. It just restore support 
for non-empty hash functor that used to work with previous 
implementation. There is also no performance test impacted by the 
modification of the default cache behavior so it is not surprised that 
you noticed nothing.

>
> (Do the performance benchmarks actually tell us anything useful?
> When I run them I get such varying results it doesn't seem to be reliable.)
Last time I run the tests it was showing when not caching was better 
than caching. I have even added a bench on the unordered containers 
directly to show what are the performance of default behavior. For the 
moment, for the Foo type used in 54075.cc, the default behavior is not 
the best one. But I will submit a patch for that soon with a hash traits 
telling if it is fast or not, like we already talk about.

François

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