RFR: 8290812: Add a test for ResourceHashtable [v7]

[Ioi Lam]

On Mon, 1 Aug 2022 22:17:30 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> I added a test for ResourceHashtable to show the interactions with it and Symbol* refcounting.
>> Tested with tier1 on Oracle platforms.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Expand to 5 tests and one that's simple.

test/hotspot/gtest/utilities/test_resourceHash.cpp line 363:

> 361:         }
> 362:     };
> 363: };

It's a bit challenging to read 5 consecutive test cases that are subtly different. Maybe SimpleDeleter should be moved to a separate class, like ResourceHashtableSimpleDeleteTest. Then we can separate the test cases into a group of 2 and another group of 3.

test/hotspot/gtest/utilities/test_resourceHash.cpp line 429:

> 427:   _test_table.unlink(&deleter);
> 428:   ASSERT_EQ(d->refcount(), d_orig_count) << "refcount should be as we started";
> 429: }

`value_delete` and `value_remove` use the exact same table instance. It's unclear from the example why `value_delete` needs to manipulate the key's refcount explicitly, but the latter doesn't.

test/hotspot/gtest/utilities/test_resourceHash.cpp line 439:

> 437:     // refcount outside the hashtable functions.
> 438:     s->increment_refcount();
> 439:     _ptr_test_table.put(s, tv);

I am not sure why it's necessary to do s->increment_refcount() here and key->decrement_refcount() in PtrDeleter. We know that the symbol will be kept alive by the TestValue. These two inc/dec calls are done where we know that the TestValue is alive (after instantiation, and before deallocation). By removing these calls, we can reduce the chance of mishandling the refcount.

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

PR: https://git.openjdk.org/jdk/pull/9603