RFR: 8267752: KVHashtable doesn't deallocate entries

[Ioi Lam]

On Wed, 16 Jun 2021 03:08:35 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/utilities/hashtable.hpp line 268:
>> 
>>> 266:       for (KVHashtableEntry** p = bucket_addr(index); *p != NULL; ) {
>>> 267:         probe = *p;
>>> 268:         *p = probe->next();
>> 
>> Should we also call the destructor (something like `probe->_value.~V()` , not sure about the syntax). I.e., similar to `GrowableArray<E>`:
>> 
>> https://github.com/openjdk/jdk/blob/e0f6f70d3f9e748d2bc53f371beca487e9343d4a/src/hotspot/share/utilities/growableArray.hpp#L499-L501
>> 
>> This way, we can get rid of this code, and move `delete ref()` into `SourceObjInfo::~SourceObjInfo()`
>> 
>> https://github.com/openjdk/jdk/blob/e0f6f70d3f9e748d2bc53f371beca487e9343d4a/src/hotspot/share/cds/archiveBuilder.hpp#L180-L186
>
> I wanted to do that (and even had a version that did), but unfortunately for this code, adding a destructor to SrcObjRef destroys the ref pointer in this scope, so we'd need to have a copy constructor etc to keep the pointer alive.
> 
> https://github.com/openjdk/jdk/blob/e0f6f70d3f9e748d2bc53f371beca487e9343d4a/src/hotspot/share/cds/archiveBuilder.cpp#L464
> 
> So I thought we should wait until we replace this table to make it better.  I was going to add a comment here.

Sounds good. The current code is a bit clumsy but there's no memory leak anymore. We should eventually move all functionalities of KVHashTable into ResourceHashtable so we don't have 2 different partially working hashtable types!

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

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