Merge pull request #20 from netty/always-cleaner

Buffers always have a cleaner attached

Author: chrisvest

src/main/java/io/netty/buffer/api/Allocator.java

@@ -150,14 +150,10 @@ default void close() {
     }
 
     static Allocator heap() {
-        return new ManagedAllocator(MemoryManager.getHeapMemoryManager(), null);
+        return new ManagedAllocator(MemoryManager.getHeapMemoryManager(), Statics.CLEANER);
     }
 
     static Allocator direct() {
-        return new ManagedAllocator(MemoryManager.getNativeMemoryManager(), null);
-    }
-
-    static Allocator directWithCleaner() {
         return new ManagedAllocator(MemoryManager.getNativeMemoryManager(), Statics.CLEANER);
     }
 
@@ -168,13 +164,4 @@ static Allocator pooledHeap() {
     static Allocator pooledDirect() {
         return new SizeClassedMemoryPool(MemoryManager.getNativeMemoryManager());
     }
-
-    static Allocator pooledDirectWithCleaner() {
-        return new SizeClassedMemoryPool(MemoryManager.getNativeMemoryManager()) {
-            @Override
-            protected Drop<Buf> getDrop() {
-                return new NativeMemoryCleanerDrop(this, getMemoryManager(), super.getDrop());
-            }
-        };
-    }
 }

src/main/java/io/netty/buffer/api/NativeMemoryCleanerDrop.java renamed to src/main/java/io/netty/buffer/api/CleanerPooledDrop.java

@@ -24,17 +24,17 @@
 import static io.netty.buffer.api.Statics.findVarHandle;
 import static java.lang.invoke.MethodHandles.lookup;
 
-class NativeMemoryCleanerDrop implements Drop<Buf> {
+class CleanerPooledDrop implements Drop<Buf> {
     private static final VarHandle CLEANABLE =
-            findVarHandle(lookup(), NativeMemoryCleanerDrop.class, "cleanable", GatedCleanable.class);
+            findVarHandle(lookup(), CleanerPooledDrop.class, "cleanable", GatedCleanable.class);
     private final SizeClassedMemoryPool pool;
     private final MemoryManager manager;
     private final Drop<Buf> delegate;
     @SuppressWarnings("unused")
     private volatile GatedCleanable cleanable;
 
-    NativeMemoryCleanerDrop(SizeClassedMemoryPool pool, MemoryManager manager,
-                            Drop<Buf> delegate) {
+    CleanerPooledDrop(SizeClassedMemoryPool pool, MemoryManager manager,
+                      Drop<Buf> delegate) {
         this.pool = pool;
         this.manager = manager;
         this.delegate = delegate;

src/main/java/io/netty/buffer/api/SizeClassedMemoryPool.java

@@ -59,7 +59,7 @@ protected Buf createBuf(int size, Drop<Buf> drop) {
     }
 
     protected Drop<Buf> getDrop() {
-        return this;
+        return new CleanerPooledDrop(this, getMemoryManager(), this);
     }
 
     @Override

src/test/java/io/netty/buffer/api/BufTest.java

@@ -68,11 +68,9 @@ static Fixture[] allocators() {
     static List<Fixture> initialAllocators() {
         return List.of(
                 new Fixture("heap", Allocator::heap, HEAP),
-                new Fixture("direct", Allocator::direct, DIRECT),
-                new Fixture("directWithCleaner", Allocator::directWithCleaner, DIRECT, CLEANER),
+                new Fixture("direct", Allocator::direct, DIRECT, CLEANER),
                 new Fixture("pooledHeap", Allocator::pooledHeap, POOLED, HEAP),
-                new Fixture("pooledDirect", Allocator::pooledDirect, POOLED, DIRECT),
-                new Fixture("pooledDirectWithCleaner", Allocator::pooledDirectWithCleaner, POOLED, DIRECT, CLEANER));
+                new Fixture("pooledDirect", Allocator::pooledDirect, POOLED, DIRECT, CLEANER));
     }
 
     static Stream<Fixture> nonSliceAllocators() {
@@ -87,18 +85,10 @@ static Stream<Fixture> directAllocators() {
         return fixtureCombinations().filter(Fixture::isDirect);
     }
 
-    static Stream<Fixture> directWithCleanerAllocators() {
-        return fixtureCombinations().filter(f -> f.isDirect() && f.isCleaner());
-    }
-
-    static Stream<Fixture> directPooledWithCleanerAllocators() {
+    static Stream<Fixture> directPooledAllocators() {
         return fixtureCombinations().filter(f -> f.isDirect() && f.isCleaner() && f.isPooled());
     }
 
-    static Stream<Fixture> poolingAllocators() {
-        return fixtureCombinations().filter(f -> f.isPooled());
-    }
-
     private static Stream<Fixture> fixtureCombinations() {
         Fixture[] fxs = fixtures;
         if (fxs != null) {
@@ -1496,7 +1486,7 @@ public void directBufferMustHaveNonZeroAddress(Fixture fixture) {
     class CleanerTests {
         @Disabled("Precise native memory accounting does not work since recent panama-foreign changes.")
         @ParameterizedTest
-        @MethodSource("io.netty.buffer.api.BufTest#directWithCleanerAllocators")
+        @MethodSource("io.netty.buffer.api.BufTest#directAllocators")
         public void bufferMustBeClosedByCleaner(Fixture fixture) throws InterruptedException {
             var allocator = fixture.createAllocator();
             allocator.close();
@@ -1518,7 +1508,7 @@ private void allocateAndForget(Allocator allocator, int size) {
 
         @Disabled("Precise native memory accounting does not work since recent panama-foreign changes.")
         @ParameterizedTest
-        @MethodSource("io.netty.buffer.api.BufTest#directPooledWithCleanerAllocators")
+        @MethodSource("io.netty.buffer.api.BufTest#directPooledAllocators")
         public void buffersMustBeReusedByPoolingAllocatorEvenWhenDroppedByCleanerInsteadOfExplicitly(Fixture fixture)
                 throws InterruptedException {
             try (var allocator = fixture.createAllocator()) {

src/test/java/io/netty/buffer/api/benchmarks/MemorySegmentClosedByCleanerBenchmark.java

@@ -42,9 +42,7 @@
 @State(Scope.Benchmark)
 public class MemorySegmentClosedByCleanerBenchmark {
     private static final Allocator direct = Allocator.direct();
-    private static final Allocator withCleaner = Allocator.directWithCleaner();
     private static final Allocator directPooled = Allocator.pooledDirect();
-    private static final Allocator pooledWithCleaner = Allocator.pooledDirectWithCleaner();
 
     @Param({"heavy", "light"})
     public String workload;
@@ -77,19 +75,12 @@ public Buf explicitPooledClose() throws Exception {
 
     @Benchmark
     public Buf cleanerClose() throws Exception {
-        return process(withCleaner.allocate(256));
+        return process(direct.allocate(256));
     }
 
     @Benchmark
     public Buf cleanerClosePooled() throws Exception {
-        return process(pooledWithCleaner.allocate(256));
-    }
-
-    @Benchmark
-    public Buf pooledWithCleanerExplicitClose() throws Exception {
-        try (Buf buf = pooledWithCleaner.allocate(256)) {
-            return process(buf);
-        }
+        return process(directPooled.allocate(256));
     }
 
     private Buf process(Buf buffer) throws Exception {

src/test/java/io/netty/buffer/api/examples/AsyncExample.java

@@ -22,7 +22,7 @@
 
 public final class AsyncExample {
     public static void main(String[] args) throws Exception {
-        try (Allocator allocator = Allocator.pooledDirectWithCleaner();
+        try (Allocator allocator = Allocator.pooledDirect();
              Buf startBuf = allocator.allocate(16)) {
             startBuf.writeLong(threadId());