RFR: 8269004 Implement ResizableResourceHashtable [v4]

[Ioi Lam]

On Wed, 30 Jun 2021 12:46:53 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   @coleenp comments
>
> src/hotspot/share/utilities/resourceHash.hpp line 80:
> 
>> 78: 
>> 79:   Node const** lookup_node(unsigned hash, K const& key) const {
>> 80:     return const_cast<Node const**>(
> 
> [pre-existing] I think this `const_cast` to add const is unnecessary.

I tried removing it but gcc complains. I will leave it as is for now.

> src/hotspot/share/utilities/resourceHash.hpp line 88:
> 
>> 86: 
>> 87:  public:
>> 88:   ResourceHashtableBase() : _number_of_entries(0) {}
> 
> I'd prefer this explicitly initialize STORAGE, e.g. add `STORAGE()` to value-initialize rather than default-initialize it.  (I think it doesn't currently make a difference, but I think being explicit is clearer.)

Fixed.

> src/hotspot/share/utilities/resourceHash.hpp line 90:
> 
>> 88:   ResourceHashtableBase() : _number_of_entries(0) {}
>> 89: 
>> 90:   ResourceHashtableBase(unsigned size) : STORAGE(size), _number_of_entries(0) {}
> 
> These constructors seem like they should be non-public.

Fixed.

> src/hotspot/share/utilities/resourceHash.hpp line 92:
> 
>> 90:   ResourceHashtableBase(unsigned size) : STORAGE(size), _number_of_entries(0) {}
>> 91: 
>> 92:   ~ResourceHashtableBase() {
> 
> Base class constructor should be non-public to avoid slicing.  Also need to decide what to do about copying, either disallow or deep copy.  Default shallow copy seems likely to be wrong.

I made all the constructors and destructors protected. I also made the classes `NONCOPYABLE`.

> src/hotspot/share/utilities/resourceHash.hpp line 222:
> 
>> 220: protected:
>> 221:   FixedResourceHashtableStorage() {
>> 222:     memset(_table, 0, TABLE_SIZE * sizeof(Node*));
> 
> Instead of memset, consider `FixedResourceHashtableStorage() : _table{} {}`.

Fixed.

> src/hotspot/share/utilities/resourceHash.hpp line 224:
> 
>> 222:     memset(_table, 0, TABLE_SIZE * sizeof(Node*));
>> 223:   }
>> 224: 
> 
> Destructor should be non-public to prevent slicing.  Also need to consider what to do about copying.

This class doesn't have a destructor. Should I add one and make it protected?

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

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