RFR: 8269004 Implement ResizableResourceHashtable [v3]

Ioi Lam iklam at openjdk.java.net
Tue Jun 29 21:12:05 UTC 2021


On Tue, 29 Jun 2021 20:11:23 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   @kimbarrett feedback to move the storage code to a base class
>
> src/hotspot/share/utilities/resizeableResourceHash.hpp line 127:
> 
>> 125:     }
>> 126: 
>> 127:     FREE_C_HEAP_ARRAY(Node*, old_table);
> 
> If you resource allocated old_table, this doesn't seem right.
> It seems like the ResizeableHashtable should be a CHeapHashtable and it's always resizeable.
> And the code either has a fixed size ResourceHashtable or a variable/resizeable CHeapHashtable.

Fixed. The `FREE_C_HEAP_ARRAY` should be called only if the table is c-heap allocated.

Per our off-line conversation,  let's leave the 4 combination of {res, c-heap} x {fixed, resizeable}, and live with the bad naming for now (even `ResourceHashtable` is misleading since it can be resource allocated). We should try to start getting rid of the old `Hashtable` classes. Eventually, we can clean up the naming with something like:

- ResizeableResourceHashtable -> Hashtable
- ResourceHashtable -> FixedHashtable

> src/hotspot/share/utilities/resourceHash.hpp line 246:
> 
>> 244:     K, V, HASH, EQUALS, ALLOC_TYPE, MEM_TYPE> {
>> 245: public:
>> 246:   ResourceHashtable() : ResourceHashtableBase<FixedResourceHashtableStorage<SIZE, K, V>, K, V, HASH, EQUALS, ALLOC_TYPE, MEM_TYPE>() {}
> 
> nit: can you break up this line after the V> ?

Fixed.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4536


More information about the hotspot-dev mailing list