On Wed, 20 Nov 2024 00:50:01 GMT, Brent Christian <bchri...@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
