This is the mail archive of the
mailing list for the libstdc++ project.
Re: hash policy patch
- From: Paolo Carlini <paolo dot carlini at oracle dot com>
- To: François Dumont <francois dot cppdevs at free dot fr>
- Cc: Paolo Carlini <pcarlini at gmail dot com>, "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>
- Date: Thu, 1 Sep 2011 11:36:00 -0700 (PDT)
- Subject: Re: hash policy patch
- References: <4E2F1A56.email@example.com> <4E2F204B.firstname.lastname@example.org> <4E31C6CE.email@example.com> <7B3982F6-FEAA-4023-AC36-84B10A513651@oracle.com> <4E3849E9.firstname.lastname@example.org>
> After some additional time spent on hashtable I prefer to do this
> new proposal. It fixes
> - Yet some issues with hash policy that was still able to give a
> bucket count for which load_factor == max_load_factor, Standard says
> load_factor < max_load_factor. Note that in fact the refactoring to
> generalize use of _M_next_bkt had indeed a side effect which is that
> when looking for instance for 11.5 lower_bound was returning 13 but
> now that it is casted to integer it will return 11. Not only that
> reason now _M_next_bkt takes an optional __strict bool parameter
> signaling if the returned value shall be not only not shorter but even
> - In hashtable implementation I removed usages of std::max that was
> potentially leaving the hashtable in an inconsistent state with a hash
> policy next resize value not matching the current bucket count. It was
> not really a bug because the next resize value was updated on the next
> insertion but at the cost of a useless floating point operation.
> - I deal with allocation failure directly in _M_rehash method to avoid
> introducing new try/catch blocks. I also reset hash policy next resize
> value when the container is emptied on a hash functor exception.
> - In __rehash_policy I only commit the new hash policy instance if the
> rehash operation succeeded. The associated test change_load_factor.cc
> requires exception support, is there already a way to detect it or I
> need to add a new dg-require-exceptions dejaGnu macro ?
Today, I had time to look a bit more into these issues. I think we
should handle one change at a time. About the first one above, I don't
like the new __strict parameter, looks like we are going through this
complication only because we are refactoring to use _M_next_bkt, because
otherwise, if I understand correctly, we are not really incorrect, since
we are talking about something like *strict* equality of *floating*
point quantities, by itself something badly defined (indeed, carefully,
the standard talks about "keeping the load factor below this number",
using plain English, not a formula).
I think we can delay point 2.
For points 3 and 4 above, I would like to see separate patches and
separate testcases. Is it possible?
Also, please be more descriptive in the ChangeLog entry, saying which
specific functions are touched. Then, splitting the big patch will also
help clarifying the rationale behind each smaller one.