RFR 8056084: Refactor Hashtable to allow implementations without rehashing support

Mikael Gerdin mikael.gerdin at oracle.com
Fri Aug 29 14:16:28 UTC 2014


Thanks everyone for the reviews!

/Mikael

On 2014-08-29 15:55, Coleen Phillimore wrote:
>
>
> Yes, your new comment is fine.
> Coleen
>
> On 8/29/14, 7:17 AM, Mikael Gerdin wrote:
>> Coleen,
>>
>> On 2014-08-29 09:06, Mikael Gerdin wrote:
>>> Coleen,
>>>
>>> On 2014-08-28 23:56, Coleen Phillimore wrote:
>>>>
>>>> Mikael, Thank you for doing this.
>>>>
>>>> Can you remove the comments in hashtable.hpp:
>>>>
>>>>    34 // This is a generic hashtable, designed to be used for the
>>>> symbol
>>>>    35 // and string tables.
>>>
>>> Will do.
>>>
>>>>
>>>> also this comment is outdated so should be removed.
>>>>
>>>>    38 //
>>>>    39 // %note:
>>>>    40 //  - TableEntrys are allocated in blocks to reduce the space
>>>> overhead.
>>>>    41
>>>
>>> That comment is still correct for callers of
>>> BasicHashtableEntry<F>* BasicHashtable<F>::new_entry
>>
>> I changed it to:
>> +// HashtableEntrys are allocated in blocks to reduce the space overhead.
>> +template <MEMFLAGS F> BasicHashtableEntry<F>*
>> BasicHashtable<F>::new_entry(unsigned int hashValue) {
>> +  BasicHashtableEntry<F>* entry = new_entry_free_list();
>> +
>>
>> Is that ok?
>>
>> /Mikael
>>
>>>
>>>
>>>>
>>>> This refactoring looks good.
>>>
>>> Thanks
>>> /Mikael
>>>
>>>>
>>>> Thanks!
>>>> Coleen
>>>>
>>>>
>>>> On 8/26/14, 11:26 AM, 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