From e816018569b5baef8071badd6b9a6798a2c15916 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 31 Oct 2019 09:58:23 +0100 Subject: [PATCH] Remove usage of finalizer in Recycler (#9726) Motivation: We currently use a finalizer to ensure we correctly return the reserved back to the Stack but this is not really needed as we can ensure we return it when needed before dropping the WeakOrderQueue Modifications: Use explicit method call to ensure we return the reserved space back before dropping the object Result: Less finalizer usage and so less work for the GC --- .../src/main/java/io/netty/util/Recycler.java | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/common/src/main/java/io/netty/util/Recycler.java b/common/src/main/java/io/netty/util/Recycler.java index 25e46c27f3..6515e1d2ca 100644 --- a/common/src/main/java/io/netty/util/Recycler.java +++ b/common/src/main/java/io/netty/util/Recycler.java @@ -245,7 +245,6 @@ public abstract class Recycler { Link next; } - // This act as a place holder for the head Link but also will reclaim space once finalized. // Its important this does not hold any reference to either Stack or WeakOrderQueue. static final class Head { private final AtomicInteger availableSharedCapacity; @@ -256,21 +255,22 @@ public abstract class Recycler { this.availableSharedCapacity = availableSharedCapacity; } - /// TODO: In the future when we move to Java9+ we should use java.lang.ref.Cleaner. - @Override - protected void finalize() throws Throwable { - try { - super.finalize(); - } finally { - Link head = link; - link = null; - while (head != null) { - reclaimSpace(LINK_CAPACITY); - Link next = head.next; - // Unlink to help GC and guard against GC nepotism. - head.next = null; - head = next; - } + /** + * Reclaim all used space and also unlink the nodes to prevent GC nepotism. + */ + void reclaimAllSpaceAndUnlink() { + Link head = link; + link = null; + int reclaimSpace = 0; + while (head != null) { + reclaimSpace += LINK_CAPACITY; + Link next = head.next; + // Unlink to help GC and guard against GC nepotism. + head.next = null; + head = next; + } + if (reclaimSpace != 0) { + reclaimSpace(reclaimSpace); } } @@ -344,6 +344,11 @@ public abstract class Recycler { ? newQueue(stack, thread) : null; } + void reclaimAllSpaceAndUnlink() { + head.reclaimAllSpaceAndUnlink(); + this.next = null; + } + void add(DefaultHandle handle) { handle.lastRecycledId = id; @@ -575,6 +580,8 @@ public abstract class Recycler { } if (prev != null) { + // Ensure we reclaim all space before dropping the WeakOrderQueue to be GC'ed. + cursor.reclaimAllSpaceAndUnlink(); prev.setNext(next); } } else {