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