On Thu, 4 May 2023 22:32:36 GMT, Coleen Phillimore <[email protected]> wrote:
> The ResourceHashtable conversion for JDK-8292741 didn't add the resizing
> code. The old hashtable code was tuned for resizing in anticipation of large
> hashtables for JVMTI tags. This patch ports over the old hashtable resizing
> code. It also adds a ResourceHashtable::put_fast() function that prepends to
> the bucket list, which is also reclaims the performance of the old hashtable
> for this test with 10M tags. The ResourceHashtable put function is really a
> put_if_absent. This can be cleaned up in a future change. Also, the remove
> function needed a lambda to destroy the WeakHandle, since resizing requires
> copying entries.
>
> Tested with JVMTI and JDI tests locally, and tier1-4 tests.
Thank you fore taking care about these performance issue!
I've posted a couple of comments but am still looking at it.
It is hard to make sure the changes are fully correct.
src/hotspot/share/utilities/resourceHash.hpp line 234:
> 232: if (node != nullptr) {
> 233: *ptr = node->_next;
> 234: bool cont = function(node->_key, node->_value);
Q: The local `cont` is not used. Just wanted to check if anything is missed
here.
Also, what does this name mean? Should it be named `cond` instead?
-------------
PR Review: https://git.openjdk.org/jdk/pull/13818#pullrequestreview-1414110945
PR Review Comment: https://git.openjdk.org/jdk/pull/13818#discussion_r1185651139
