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