From b901c180f4a1026675a2859bbf24727574e19e6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Bl=C3=A4sing?= Date: Sat, 13 Jul 2024 20:23:42 +0200 Subject: [PATCH 1/2] Improve performance for Cleaner implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Cleaner used multiple monitors to protect its datastructures. And as datastructre a (manually) linked list was used. The datastructure was updated to a ConcurrentHashMap and the multiple monitor usages are replaced with a ReentrandReadWriteLocks. Performance numbers: Commandline: java -jar target/benchmarks.jar -t 1000 -i 1 -wi 0 ========== 5.14.0 ========== Result "eu.doppelhelix.jna.jmh.MyBenchmark.testMethod": 1211666,184 ±(99.9%) 134595,856 ops/s [Average] (min, avg, max) = (1178371,132, 1211666,184, 1271195,212), stdev = 34954,116 CI (99.9%): [1077070,328, 1346262,040] (assumes normal distribution) Estimated CPU Load: 650% ========== 5.14.0 ========== Result "eu.doppelhelix.jna.jmh.MyBenchmark.testMethod": 3260953,271 ±(99.9%) 655799,010 ops/s [Average] (min, avg, max) = (3092006,068, 3260953,271, 3527896,224), stdev = 170308,920 CI (99.9%): [2605154,261, 3916752,281] (assumes normal distribution) Estimated CPU Load: 1500% ============================ Code: package eu.doppelhelix.jna.jmh; import com.sun.jna.internal.Cleaner; import java.util.concurrent.atomic.AtomicLong; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.infra.Blackhole; public class MyBenchmark { @Benchmark public void testMethod(Blackhole blackhole) { DummyObject dummyObj = new DummyObject(); DummyObjectCleaner dummyCleaner = new DummyObjectCleaner(blackhole, dummyObj.getDummyValue()); Cleaner.getCleaner().register(dummyObj, dummyCleaner); } public static class DummyObject { private static final AtomicLong ai = new AtomicLong(); private final String dummyValue; public DummyObject() { this.dummyValue = "d " + ai.incrementAndGet(); } public String getDummyValue() { return dummyValue; } } public static class DummyObjectCleaner implements Runnable{ private final Blackhole bh; private final String data; public DummyObjectCleaner(Blackhole bh, String data) { this.bh = bh; this.data = data; } public void run() { this.bh.consume(this.data); } } } --- src/com/sun/jna/internal/Cleaner.java | 149 ++++++++++++++------------ 1 file changed, 80 insertions(+), 69 deletions(-) diff --git a/src/com/sun/jna/internal/Cleaner.java b/src/com/sun/jna/internal/Cleaner.java index a2095937f..333b9e698 100644 --- a/src/com/sun/jna/internal/Cleaner.java +++ b/src/com/sun/jna/internal/Cleaner.java @@ -27,6 +27,12 @@ import java.lang.ref.PhantomReference; import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.logging.Level; import java.util.logging.Logger; @@ -44,65 +50,79 @@ public static Cleaner getCleaner() { return INSTANCE; } + // Guard for trackedObjects and cleanerThread. The readlock is utilized when + // the trackedObjects are manipulated, the writelock protectes starting and + // stopping the CleanerThread + private final ReadWriteLock cleanerThreadLock = new ReentrantReadWriteLock(); private final ReferenceQueue referenceQueue; - private Thread cleanerThread; - private CleanerRef firstCleanable; + // Count of objects tracked by the cleaner. When >0 it means objects are + // being tracked by the cleaner and the cleanerThread must be running + private final AtomicLong trackedObjects = new AtomicLong(); + // Map only serves as holder, so that the CleanerRefs stay hard referenced + // and quickly accessible for removal + @SuppressWarnings("MismatchedQueryAndUpdateOfCollection") + private final Map map = new ConcurrentHashMap<>(); + // Thread to handle the actual cleaning + private volatile Thread cleanerThread; private Cleaner() { referenceQueue = new ReferenceQueue<>(); } - public synchronized Cleanable register(Object obj, Runnable cleanupTask) { + @SuppressWarnings("EmptySynchronizedStatement") + public Cleanable register(Object obj, Runnable cleanupTask) { // The important side effect is the PhantomReference, that is yielded // after the referent is GCed - return add(new CleanerRef(this, obj, referenceQueue, cleanupTask)); + try { + return add(new CleanerRef(this, obj, referenceQueue, cleanupTask)); + } finally { + synchronized (obj) { + // Used as a reachability fence for obj + // Ensure, that add completes before obj can be collected and + // the cleaner is run + } + } } - private synchronized CleanerRef add(CleanerRef ref) { - synchronized (referenceQueue) { - if (firstCleanable == null) { - firstCleanable = ref; - } else { - ref.setNext(firstCleanable); - firstCleanable.setPrevious(ref); - firstCleanable = ref; - } - if (cleanerThread == null) { - Logger.getLogger(Cleaner.class.getName()).log(Level.FINE, "Starting CleanerThread"); - cleanerThread = new CleanerThread(); - cleanerThread.start(); + private CleanerRef add(CleanerRef ref) { + map.put(ref, ref); + cleanerThreadLock.readLock().lock(); + try { + long count = trackedObjects.incrementAndGet(); + if (cleanerThread == null && count > 0) { + cleanerThreadLock.readLock().unlock(); + cleanerThreadLock.writeLock().lock(); + try { + if (cleanerThread == null && trackedObjects.get() > 0) { + Logger.getLogger(Cleaner.class.getName()).log(Level.FINE, "Starting CleanerThread"); + cleanerThread = new CleanerThread(); + cleanerThread.start(); + } + } finally { + cleanerThreadLock.readLock().lock(); + cleanerThreadLock.writeLock().unlock(); + } } - return ref; + } finally { + cleanerThreadLock.readLock().unlock(); } + return ref; } - private synchronized boolean remove(CleanerRef ref) { - synchronized (referenceQueue) { - boolean inChain = false; - if (ref == firstCleanable) { - firstCleanable = ref.getNext(); - inChain = true; - } - if (ref.getPrevious() != null) { - ref.getPrevious().setNext(ref.getNext()); - } - if (ref.getNext() != null) { - ref.getNext().setPrevious(ref.getPrevious()); - } - if (ref.getPrevious() != null || ref.getNext() != null) { - inChain = true; - } - ref.setNext(null); - ref.setPrevious(null); - return inChain; + private void remove(CleanerRef ref) { + map.remove(ref); + cleanerThreadLock.readLock().lock(); + try { + trackedObjects.decrementAndGet(); + } finally { + cleanerThreadLock.readLock().unlock(); } } private static class CleanerRef extends PhantomReference implements Cleanable { private final Cleaner cleaner; private final Runnable cleanupTask; - private CleanerRef previous; - private CleanerRef next; + private final AtomicBoolean cleaned = new AtomicBoolean(false); public CleanerRef(Cleaner cleaner, Object referent, ReferenceQueue q, Runnable cleanupTask) { super(referent, q); @@ -112,26 +132,11 @@ public CleanerRef(Cleaner cleaner, Object referent, ReferenceQueue Date: Wed, 17 Jul 2024 21:24:35 +0200 Subject: [PATCH 2/2] Move break to correct location --- src/com/sun/jna/internal/Cleaner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/com/sun/jna/internal/Cleaner.java b/src/com/sun/jna/internal/Cleaner.java index 333b9e698..1fe5ac657 100644 --- a/src/com/sun/jna/internal/Cleaner.java +++ b/src/com/sun/jna/internal/Cleaner.java @@ -169,8 +169,8 @@ public void run() { if (trackedObjects.get() == 0) { Logger.getLogger(Cleaner.class.getName()).log(Level.FINE, "Shutting down CleanerThread"); cleanerThread = null; + break; } - break; } finally { cleanerThreadLock.readLock().lock(); cleanerThreadLock.writeLock().unlock();