RFR: 8343704: Bad GC parallelism with processing Cleaner queues [v13]

[Aleksey Shipilev]

On Wed, 20 Nov 2024 00:50:01 GMT, Brent Christian <bchristi at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Use RandomFactory in test
>
> src/java.base/share/classes/jdk/internal/ref/PhantomCleanable.java line 55:
> 
>> 53:      * Synchronized by the same lock as the list itself.
>> 54:      */
>> 55:     CleanerImpl.CleanableList.Node node;
> 
> I think we can get away with not storing a `Node` in every `PhantomCleanable`, and instead look for it at `index` in each `Node` in the list. But I don't know if this would be a performance win. Heap savings (in `PhantomCleanableRef`)? Worse locality from loading multiple `Nodes`?
> Just an idea...

I don't think this is a great idea: it means linear search under the lock, which I don't think we want. As it stands now, we are doing removals in constant time. Current version also affects the footprint positively already: we change 3 references into 2 references + 1 int, so there is no pressing need for smartness in optimizing the footprint.

> test/jdk/jdk/internal/ref/Cleaner/CleanableListTest.java line 126:
> 
>> 124:                 bs.set(idx, true);
>> 125:             }
>> 126:         }
> 
> Can the test remove everything left in the list after doing the `RANDOM_ITERATIONS`?

Done in new commit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22043#discussion_r1849918892
PR Review Comment: https://git.openjdk.org/jdk/pull/22043#discussion_r1849920309