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.