This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: hash policy patch


Hi,
>     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 
> larger.
> - 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.

Paolo.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]