RFR 8056084: Refactor Hashtable to allow implementations without rehashing support

Bengt Rutisson bengt.rutisson at oracle.com
Thu Aug 28 11:20:01 UTC 2014


Hi Mikael,


On 2014-08-28 13:20, Mikael Gerdin wrote:
> Hi Bengt,
>
> On 08/28/2014 12:51 PM, Bengt Rutisson wrote:
>>
>> Hi Mikael,
>>
>> Looks good.
>>
>> One very minor nit if you feel like it.
>>
>> hashtable.cpp
>>   50   if (_free_list) {
>>
>> Would you mind changing to "if (_free_list != NULL) {" ? I know it is
>> not your code, but you are changing code in that method and did take me
>> some extra brain cycles to realize that _free_list was not a bool
>> indicating whether we were using a free list or not.
>
> That seems like a good suggestion. The HotSpot style also explicitly 
> mentions that using pointers as booleans should be avoided.
>
> New webrev at: 
> http://cr.openjdk.java.net/~mgerdin/8048268/hashtable-refactor/webrev.1/

Great! Thanks! Reviewed.

Bengt

>
> /Mikael
>
>>
>> Thanks,
>> Bengt
>>
>>
>> On 2014-08-26 17:26, Mikael Gerdin wrote:
>>> Hi all,
>>>
>>> For JDK-8048268 I need to do some refactoring of the Hashtable class
>>> template.
>>>
>>>  From the bug description:
>>> In order to simplify usage of the Hashtable<T, F> class the code to
>>> perform
>>> rehashing of the SymbolTable and StringTable should be factored out 
>>> to an
>>> intermediate class common to those two tables.
>>> Also factor out HashtableEntry freelist allocation to allow
>>> implementors an
>>> easier route for providing their own memory management.
>>>
>>> Introducing the new intermediary superclass requires some calls to
>>> superclass
>>> functions to be qualified using the "this->" syntax due to the C++
>>> template
>>> name lookup rules.
>>>
>>> a comment about hashtable.cpp RehashableHashtable::dump_table:
>>>
>>> -  int bucket_bytes = (int)num_buckets * sizeof(bucket(0));
>>> +  int bucket_bytes = (int)num_buckets * sizeof(HashtableBucket<F>);
>>>
>>> I decided to change the sizeof to count the size of the hashtable
>>> bucket class
>>> instead of the HashtableEntry* returned by bucket(0), even though they
>>> may be
>>> of the same size currently.
>>>
>>> Note that the webrev url refers to another bug.
>>> Webrev:
>>> http://cr.openjdk.java.net/~mgerdin/8048268/hashtable-refactor/webrev/
>>> Buglink: https://bugs.openjdk.java.net/browse/JDK-8056084
>>>
>>> Testing: JPRT.
>>>
>>
>



More information about the hotspot-dev mailing list