RFR: 8267752: KVHashtable doesn't deallocate entries

[Thomas Stuefe]

On Tue, 15 Jun 2021 22:34:40 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> Add a free_entry iteration to the destructor of ~KVHashtables.
> Tested with tier1-3.

Hi Coleen,

seems to be right.

I did a little test (filling a KV<int, int> hashtable with 500mio entries and releasing it) and was confused when RSS did not go down es expected; but this seems to be a glibc issue (repeating the test did not allocate twice as much memory, so the second round of mallocs reused the first frees).

See my question inline about unlink(). Otherwise this is fine. This was only ever used in CDS dumping, right?

Cheers, Thomas

src/hotspot/share/utilities/hashtable.hpp line 269:

> 267:         probe = *p;
> 268:         *p = probe->next();
> 269:         free_entry(probe);

I tried to understand `BasicHashTable::free_entry()` and `BasicHashTable::unlink_entry()`. I may be wrong, but the latter looks wrong. I expected some linked list splicing, but all it does is set the next ptr of the unlinked entry to NULL. So it would orphan follow up entries, and leave a dangling pointer to itself at its chain predecessor. I may be missing something here.

You code still seems to be correct since you manually walk the whole entry chain.

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

Marked as reviewed by stuefe (Reviewer).

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