diff --git a/src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java b/src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java index b4565daa466d3..bb7293e5d2b89 100644 --- a/src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java +++ b/src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java @@ -48,9 +48,9 @@ public final class CleanerImpl implements Runnable { private static Function cleanerImplAccess = null; /** - * Heads of a CleanableList for each reference type. + * Currently active PhantomCleanable-s. */ - final PhantomCleanable phantomCleanableList; + final CleanableList activeList; // The ReferenceQueue of pending cleaning actions final ReferenceQueue queue; @@ -82,7 +82,7 @@ static CleanerImpl getCleanerImpl(Cleaner cleaner) { */ public CleanerImpl() { queue = new ReferenceQueue<>(); - phantomCleanableList = new PhantomCleanableRef(); + activeList = new CleanableList(); } /** @@ -129,7 +129,7 @@ public void run() { InnocuousThread mlThread = (t instanceof InnocuousThread) ? (InnocuousThread) t : null; - while (!phantomCleanableList.isListEmpty()) { + while (!activeList.isEmpty()) { if (mlThread != null) { // Clear the thread locals mlThread.eraseThreadLocals(); @@ -165,14 +165,6 @@ public PhantomCleanableRef(Object obj, Cleaner cleaner, Runnable action) { this.action = action; } - /** - * Constructor used only for root of phantom cleanable list. - */ - PhantomCleanableRef() { - super(); - this.action = null; - } - @Override protected void performCleanup() { action.run(); @@ -231,4 +223,137 @@ protected void performCleanup() { // no action } } + + /** + * A specialized implementation that tracks phantom cleanables. + */ + static final class CleanableList { + /** + * Capacity for a single node in the list. + * This balances memory overheads vs locality vs GC walking costs. + */ + static final int NODE_CAPACITY = 4096; + + /** + * Head node. This is the only node where PhantomCleanables are + * added to or removed from. This is the only node with variable size, + * all other nodes linked from the head are always at full capacity. + */ + private Node head; + + /** + * Cached node instance to provide better behavior near NODE_CAPACITY + * threshold: if list size flips around NODE_CAPACITY, it would reuse + * the cached node instead of wasting and re-allocating a new node all + * the time. + */ + private Node cache; + + public CleanableList() { + reset(); + } + + /** + * Testing support: reset list to initial state. + */ + synchronized void reset() { + this.head = new Node(); + } + + /** + * Returns true if cleanable list is empty. + * + * @return true if the list is empty + */ + public synchronized boolean isEmpty() { + // Head node size is zero only when the entire list is empty. + return head.size == 0; + } + + /** + * Insert this PhantomCleanable in the list. + */ + public synchronized void insert(PhantomCleanable phc) { + if (head.size == NODE_CAPACITY) { + // Head node is full, insert new one. + // If possible, pick a pre-allocated node from cache. + Node newHead; + if (cache != null) { + newHead = cache; + cache = null; + } else { + newHead = new Node(); + } + newHead.next = head; + head = newHead; + } + assert head.size < NODE_CAPACITY; + + // Put the incoming object in head node and record indexes. + final int lastIndex = head.size; + phc.node = head; + phc.index = lastIndex; + head.arr[lastIndex] = phc; + head.size++; + } + + /** + * Remove this PhantomCleanable from the list. + * + * @return true if Cleanable was removed or false if not because + * it had already been removed before + */ + public synchronized boolean remove(PhantomCleanable phc) { + if (phc.node == null) { + // Not in the list. + return false; + } + assert phc.node.arr[phc.index] == phc; + + // Replace with another element from the head node, as long + // as it is not the same element. This keeps all non-head + // nodes at full capacity. + final int lastIndex = head.size - 1; + assert lastIndex >= 0; + if (head != phc.node || (phc.index != lastIndex)) { + PhantomCleanable mover = head.arr[lastIndex]; + mover.node = phc.node; + mover.index = phc.index; + phc.node.arr[phc.index] = mover; + } + + // Now we can unlink the removed element. + phc.node = null; + + // Remove the last element from the head node. + head.arr[lastIndex] = null; + head.size--; + + // If head node becomes empty after this, and there are + // nodes that follow it, replace the head node with another + // full one. If needed, stash the now free node in cache. + if (head.size == 0 && head.next != null) { + Node newHead = head.next; + if (cache == null) { + cache = head; + cache.next = null; + } + head = newHead; + } + + return true; + } + + /** + * Segment node. + */ + static class Node { + // Array of tracked cleanables, and the amount of elements in it. + final PhantomCleanable[] arr = new PhantomCleanable[NODE_CAPACITY]; + int size; + + // Linked list structure. + Node next; + } + } } diff --git a/src/java.base/share/classes/jdk/internal/ref/PhantomCleanable.java b/src/java.base/share/classes/jdk/internal/ref/PhantomCleanable.java index 24db6d8ef9a07..3564a16d0a062 100644 --- a/src/java.base/share/classes/jdk/internal/ref/PhantomCleanable.java +++ b/src/java.base/share/classes/jdk/internal/ref/PhantomCleanable.java @@ -44,14 +44,21 @@ public abstract class PhantomCleanable extends PhantomReference implements Cleaner.Cleanable { /** - * Links to previous and next in a doubly-linked list. + * The list of PhantomCleanable; synchronizes insert and remove. */ - PhantomCleanable prev = this, next = this; + private final CleanerImpl.CleanableList list; /** - * The list of PhantomCleanable; synchronizes insert and remove. + * Node for this PhantomCleanable in the list. + * Synchronized by the same lock as the list itself. */ - private final PhantomCleanable list; + CleanerImpl.CleanableList.Node node; + + /** + * Index of this PhantomCleanable in the list node. + * Synchronized by the same lock as the list itself. + */ + int index; /** * Constructs new {@code PhantomCleanable} with @@ -62,73 +69,29 @@ public abstract class PhantomCleanable extends PhantomReference * @param referent the referent to track * @param cleaner the {@code Cleaner} to register with */ + @SuppressWarnings("this-escape") public PhantomCleanable(T referent, Cleaner cleaner) { super(Objects.requireNonNull(referent), CleanerImpl.getCleanerImpl(cleaner).queue); - this.list = CleanerImpl.getCleanerImpl(cleaner).phantomCleanableList; - insert(); + index = -1; + list = CleanerImpl.getCleanerImpl(cleaner).activeList; + list.insert(this); + + // Check that list insertion populated the backlinks. + assert node != null; + assert index >= 0; // Ensure referent and cleaner remain accessible Reference.reachabilityFence(referent); Reference.reachabilityFence(cleaner); } - /** - * Construct a new root of the list; not inserted. - */ - PhantomCleanable() { - super(null, null); - this.list = this; - } - - /** - * Insert this PhantomCleanable after the list head. - */ - private void insert() { - synchronized (list) { - prev = list; - next = list.next; - next.prev = this; - list.next = this; - } - } - - /** - * Remove this PhantomCleanable from the list. - * - * @return true if Cleanable was removed or false if not because - * it had already been removed before - */ - private boolean remove() { - synchronized (list) { - if (next != this) { - next.prev = prev; - prev.next = next; - prev = this; - next = this; - return true; - } - return false; - } - } - - /** - * Returns true if the list's next reference refers to itself. - * - * @return true if the list is empty - */ - boolean isListEmpty() { - synchronized (list) { - return list == list.next; - } - } - /** * Unregister this PhantomCleanable and invoke {@link #performCleanup()}, * ensuring at-most-once semantics. */ @Override public final void clean() { - if (remove()) { + if (list.remove(this)) { super.clear(); performCleanup(); } @@ -140,7 +103,7 @@ public final void clean() { */ @Override public void clear() { - if (remove()) { + if (list.remove(this)) { super.clear(); } } diff --git a/test/jdk/jdk/internal/ref/Cleaner/CleanableListTest.java b/test/jdk/jdk/internal/ref/Cleaner/CleanableListTest.java new file mode 100644 index 0000000000000..eedd21d3f5373 --- /dev/null +++ b/test/jdk/jdk/internal/ref/Cleaner/CleanableListTest.java @@ -0,0 +1,136 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8343704 + * @key randomness + * @library /test/lib + * @compile/module=java.base jdk/internal/ref/CleanableListTestHelper.java jdk/internal/ref/TestCleanable.java + * @modules java.base/jdk.internal.ref + * @run testng/othervm CleanableListTest + */ + +import java.util.ArrayList; +import java.util.BitSet; +import java.util.List; +import java.util.Random; + +import jdk.internal.ref.CleanableListTestHelper; +import jdk.internal.ref.TestCleanable; +import jdk.test.lib.RandomFactory; + +import org.testng.Assert; +import org.testng.annotations.Test; +import org.testng.annotations.Ignore; + +public class CleanableListTest { + + static final int SINGLE_NODE_CAPACITY = CleanableListTestHelper.NODE_CAPACITY; + static final int MULTI_NODE_CAPACITY = CleanableListTestHelper.NODE_CAPACITY * 4; + + static final Random RND = RandomFactory.getRandom(); + static final int RANDOM_ITERATIONS = 10_000_000; + + @Test + public void testSingle() { + CleanableListTestHelper list = new CleanableListTestHelper(); + Assert.assertTrue(list.isEmpty()); + TestCleanable tc = list.newCleanable(); + Assert.assertFalse(list.isEmpty()); + Assert.assertTrue(list.remove(tc)); + Assert.assertTrue(list.isEmpty()); + Assert.assertFalse(list.remove(tc)); + } + + @Test + public void testSequential_Single() { + doSequential(SINGLE_NODE_CAPACITY); + } + + @Test + public void testSequential_Multi() { + doSequential(MULTI_NODE_CAPACITY); + } + + private void doSequential(int size) { + CleanableListTestHelper list = new CleanableListTestHelper(); + Assert.assertTrue(list.isEmpty()); + + List tcs = new ArrayList<>(); + for (int c = 0; c < size; c++) { + tcs.add(list.newCleanable()); + } + Assert.assertFalse(list.isEmpty()); + + for (TestCleanable tc : tcs) { + Assert.assertTrue(list.remove(tc)); + } + Assert.assertTrue(list.isEmpty()); + } + + @Test + public void testRandom_Single() { + doRandom(SINGLE_NODE_CAPACITY); + } + + @Test + public void testRandom_Multi() { + doRandom(MULTI_NODE_CAPACITY); + } + + private void doRandom(int size) { + CleanableListTestHelper list = new CleanableListTestHelper(); + Assert.assertTrue(list.isEmpty()); + + BitSet bs = new BitSet(size); + + List tcs = new ArrayList<>(); + for (int c = 0; c < size; c++) { + tcs.add(list.newCleanable()); + bs.set(c, true); + } + Assert.assertFalse(list.isEmpty()); + + for (int t = 0; t < RANDOM_ITERATIONS; t++) { + int idx = RND.nextInt(size); + TestCleanable tc = tcs.get(idx); + if (bs.get(idx)) { + Assert.assertTrue(list.remove(tc)); + bs.set(idx, false); + } else { + Assert.assertFalse(list.remove(tc)); + list.insert(tc); + bs.set(idx, true); + } + } + + for (int c = 0; c < size; c++) { + if (bs.get(c)) { + TestCleanable tc = tcs.get(c); + Assert.assertTrue(list.remove(tc)); + } + } + } + +} diff --git a/test/jdk/jdk/internal/ref/Cleaner/java.base/jdk/internal/ref/CleanableListTestHelper.java b/test/jdk/jdk/internal/ref/Cleaner/java.base/jdk/internal/ref/CleanableListTestHelper.java new file mode 100644 index 0000000000000..d3ee187a0970f --- /dev/null +++ b/test/jdk/jdk/internal/ref/Cleaner/java.base/jdk/internal/ref/CleanableListTestHelper.java @@ -0,0 +1,66 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.internal.ref; + +import java.lang.ref.Cleaner; +import jdk.internal.ref.PhantomCleanable; +import jdk.internal.ref.CleanerImpl; +import jdk.internal.ref.CleanerImpl.CleanableList; + +/** + * This class provides package-private access to CleanableList internals. + */ +public class CleanableListTestHelper { + + public static final int NODE_CAPACITY = CleanableList.NODE_CAPACITY; + + final Cleaner cleaner; + final CleanableList list; + + public CleanableListTestHelper() { + cleaner = Cleaner.create(); + list = CleanerImpl.getCleanerImpl(cleaner).activeList; + + // List contains CleanerCleanable for Cleaner itself. + // For testing empty list paths, we want to drop it. + list.reset(); + } + + public TestCleanable newCleanable() { + return new TestCleanable(cleaner); + } + + public void insert(PhantomCleanable cl) { + list.insert(cl); + } + + public boolean remove(PhantomCleanable cl) { + return list.remove(cl); + } + + public boolean isEmpty() { + return list.isEmpty(); + } + +} diff --git a/test/jdk/jdk/internal/ref/Cleaner/java.base/jdk/internal/ref/TestCleanable.java b/test/jdk/jdk/internal/ref/Cleaner/java.base/jdk/internal/ref/TestCleanable.java new file mode 100644 index 0000000000000..9e8da6f48303b --- /dev/null +++ b/test/jdk/jdk/internal/ref/Cleaner/java.base/jdk/internal/ref/TestCleanable.java @@ -0,0 +1,39 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.internal.ref; + +import java.lang.ref.Cleaner; + +public class TestCleanable extends PhantomCleanable { + static final Object TARGET = new Object(); + + public TestCleanable(Cleaner cleaner) { + super(TARGET, cleaner); + } + + @Override + protected void performCleanup() { + // no action + } +} diff --git a/test/micro/org/openjdk/bench/java/lang/ref/CleanerChurn.java b/test/micro/org/openjdk/bench/java/lang/ref/CleanerChurn.java new file mode 100644 index 0000000000000..eab4c0f59d069 --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/ref/CleanerChurn.java @@ -0,0 +1,58 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.openjdk.bench.java.lang.ref; + +import java.lang.ref.Cleaner; +import java.lang.ref.Cleaner.Cleanable; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.*; +import org.openjdk.jmh.infra.Blackhole; + +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Fork(value = 3, jvmArgs = {"-Xmx256m", "-Xms256m", "-XX:+AlwaysPreTouch"}) +public class CleanerChurn { + + @Param({"128", "256", "512", "1024", "2048"}) + int recipFreq; + + @Benchmark + public Object test() { + boolean register = ThreadLocalRandom.current().nextInt(recipFreq) == 0; + return new Target(register); + } + + static class Target { + private static final Cleaner CLEANER = Cleaner.create(); + public Target(boolean register) { + if (register) { + CLEANER.register(this, () -> {}); + } + } + } + +} diff --git a/test/micro/org/openjdk/bench/java/lang/ref/CleanerGC.java b/test/micro/org/openjdk/bench/java/lang/ref/CleanerGC.java new file mode 100644 index 0000000000000..8d43f25c6c676 --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/ref/CleanerGC.java @@ -0,0 +1,68 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.openjdk.bench.java.lang.ref; + +import java.lang.ref.Cleaner; +import java.lang.ref.Cleaner.Cleanable; +import java.util.ArrayList; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.*; +import org.openjdk.jmh.infra.Blackhole; + +@OutputTimeUnit(TimeUnit.MILLISECONDS) +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Fork(value = 3, jvmArgs = {"-Xmx1g", "-Xms1g", "-XX:+AlwaysPreTouch"}) +public class CleanerGC { + + @Param({"16384", "65536", "262144", "1048576", "4194304"}) + int count; + + // Make sure all targets are reachable and available for GC in scalable manner. + // This exposes the potential GC problem in Cleaner lists. + ArrayList targets; + + @Setup + public void setup() { + targets = new ArrayList<>(); + for (int c = 0; c < count; c++) { + targets.add(new Target()); + } + } + + @Benchmark + public void test() { + System.gc(); + } + + static class Target { + private static final Cleaner CLEANER = Cleaner.create(); + public Target() { + CLEANER.register(this, () -> {}); + } + } + +}