Skip to content

Commit b901c18

Browse files
Improve performance for Cleaner implementation
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); } } }
1 parent a587921 commit b901c18

File tree

1 file changed

+80
-69
lines changed

1 file changed

+80
-69
lines changed

src/com/sun/jna/internal/Cleaner.java

Lines changed: 80 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@
2727
import java.lang.ref.PhantomReference;
2828
import java.lang.ref.Reference;
2929
import java.lang.ref.ReferenceQueue;
30+
import java.util.Map;
31+
import java.util.concurrent.ConcurrentHashMap;
32+
import java.util.concurrent.atomic.AtomicBoolean;
33+
import java.util.concurrent.atomic.AtomicLong;
34+
import java.util.concurrent.locks.ReadWriteLock;
35+
import java.util.concurrent.locks.ReentrantReadWriteLock;
3036
import java.util.logging.Level;
3137
import java.util.logging.Logger;
3238

@@ -44,65 +50,79 @@ public static Cleaner getCleaner() {
4450
return INSTANCE;
4551
}
4652

53+
// Guard for trackedObjects and cleanerThread. The readlock is utilized when
54+
// the trackedObjects are manipulated, the writelock protectes starting and
55+
// stopping the CleanerThread
56+
private final ReadWriteLock cleanerThreadLock = new ReentrantReadWriteLock();
4757
private final ReferenceQueue<Object> referenceQueue;
48-
private Thread cleanerThread;
49-
private CleanerRef firstCleanable;
58+
// Count of objects tracked by the cleaner. When >0 it means objects are
59+
// being tracked by the cleaner and the cleanerThread must be running
60+
private final AtomicLong trackedObjects = new AtomicLong();
61+
// Map only serves as holder, so that the CleanerRefs stay hard referenced
62+
// and quickly accessible for removal
63+
@SuppressWarnings("MismatchedQueryAndUpdateOfCollection")
64+
private final Map<CleanerRef,CleanerRef> map = new ConcurrentHashMap<>();
65+
// Thread to handle the actual cleaning
66+
private volatile Thread cleanerThread;
5067

5168
private Cleaner() {
5269
referenceQueue = new ReferenceQueue<>();
5370
}
5471

55-
public synchronized Cleanable register(Object obj, Runnable cleanupTask) {
72+
@SuppressWarnings("EmptySynchronizedStatement")
73+
public Cleanable register(Object obj, Runnable cleanupTask) {
5674
// The important side effect is the PhantomReference, that is yielded
5775
// after the referent is GCed
58-
return add(new CleanerRef(this, obj, referenceQueue, cleanupTask));
76+
try {
77+
return add(new CleanerRef(this, obj, referenceQueue, cleanupTask));
78+
} finally {
79+
synchronized (obj) {
80+
// Used as a reachability fence for obj
81+
// Ensure, that add completes before obj can be collected and
82+
// the cleaner is run
83+
}
84+
}
5985
}
6086

61-
private synchronized CleanerRef add(CleanerRef ref) {
62-
synchronized (referenceQueue) {
63-
if (firstCleanable == null) {
64-
firstCleanable = ref;
65-
} else {
66-
ref.setNext(firstCleanable);
67-
firstCleanable.setPrevious(ref);
68-
firstCleanable = ref;
69-
}
70-
if (cleanerThread == null) {
71-
Logger.getLogger(Cleaner.class.getName()).log(Level.FINE, "Starting CleanerThread");
72-
cleanerThread = new CleanerThread();
73-
cleanerThread.start();
87+
private CleanerRef add(CleanerRef ref) {
88+
map.put(ref, ref);
89+
cleanerThreadLock.readLock().lock();
90+
try {
91+
long count = trackedObjects.incrementAndGet();
92+
if (cleanerThread == null && count > 0) {
93+
cleanerThreadLock.readLock().unlock();
94+
cleanerThreadLock.writeLock().lock();
95+
try {
96+
if (cleanerThread == null && trackedObjects.get() > 0) {
97+
Logger.getLogger(Cleaner.class.getName()).log(Level.FINE, "Starting CleanerThread");
98+
cleanerThread = new CleanerThread();
99+
cleanerThread.start();
100+
}
101+
} finally {
102+
cleanerThreadLock.readLock().lock();
103+
cleanerThreadLock.writeLock().unlock();
104+
}
74105
}
75-
return ref;
106+
} finally {
107+
cleanerThreadLock.readLock().unlock();
76108
}
109+
return ref;
77110
}
78111

79-
private synchronized boolean remove(CleanerRef ref) {
80-
synchronized (referenceQueue) {
81-
boolean inChain = false;
82-
if (ref == firstCleanable) {
83-
firstCleanable = ref.getNext();
84-
inChain = true;
85-
}
86-
if (ref.getPrevious() != null) {
87-
ref.getPrevious().setNext(ref.getNext());
88-
}
89-
if (ref.getNext() != null) {
90-
ref.getNext().setPrevious(ref.getPrevious());
91-
}
92-
if (ref.getPrevious() != null || ref.getNext() != null) {
93-
inChain = true;
94-
}
95-
ref.setNext(null);
96-
ref.setPrevious(null);
97-
return inChain;
112+
private void remove(CleanerRef ref) {
113+
map.remove(ref);
114+
cleanerThreadLock.readLock().lock();
115+
try {
116+
trackedObjects.decrementAndGet();
117+
} finally {
118+
cleanerThreadLock.readLock().unlock();
98119
}
99120
}
100121

101122
private static class CleanerRef extends PhantomReference<Object> implements Cleanable {
102123
private final Cleaner cleaner;
103124
private final Runnable cleanupTask;
104-
private CleanerRef previous;
105-
private CleanerRef next;
125+
private final AtomicBoolean cleaned = new AtomicBoolean(false);
106126

107127
public CleanerRef(Cleaner cleaner, Object referent, ReferenceQueue<? super Object> q, Runnable cleanupTask) {
108128
super(referent, q);
@@ -112,26 +132,11 @@ public CleanerRef(Cleaner cleaner, Object referent, ReferenceQueue<? super Objec
112132

113133
@Override
114134
public void clean() {
115-
if(cleaner.remove(this)) {
135+
if (!cleaned.getAndSet(true)) {
136+
cleaner.remove(this);
116137
cleanupTask.run();
117138
}
118139
}
119-
120-
CleanerRef getPrevious() {
121-
return previous;
122-
}
123-
124-
void setPrevious(CleanerRef previous) {
125-
this.previous = previous;
126-
}
127-
128-
CleanerRef getNext() {
129-
return next;
130-
}
131-
132-
void setNext(CleanerRef next) {
133-
this.next = next;
134-
}
135140
}
136141

137142
public static interface Cleanable {
@@ -155,22 +160,28 @@ public void run() {
155160
if (ref instanceof CleanerRef) {
156161
((CleanerRef) ref).clean();
157162
} else if (ref == null) {
158-
synchronized (referenceQueue) {
159-
Logger logger = Logger.getLogger(Cleaner.class.getName());
160-
if (firstCleanable == null) {
161-
cleanerThread = null;
162-
logger.log(Level.FINE, "Shutting down CleanerThread");
163-
break;
164-
} else if (logger.isLoggable(Level.FINER)) {
165-
StringBuilder registeredCleaners = new StringBuilder();
166-
for(CleanerRef cleanerRef = firstCleanable; cleanerRef != null; cleanerRef = cleanerRef.next) {
167-
if(registeredCleaners.length() != 0) {
168-
registeredCleaners.append(", ");
163+
cleanerThreadLock.readLock().lock();
164+
try {
165+
if (trackedObjects.get() == 0) {
166+
cleanerThreadLock.readLock().unlock();
167+
cleanerThreadLock.writeLock().lock();
168+
try {
169+
if (trackedObjects.get() == 0) {
170+
Logger.getLogger(Cleaner.class.getName()).log(Level.FINE, "Shutting down CleanerThread");
171+
cleanerThread = null;
169172
}
170-
registeredCleaners.append(cleanerRef.cleanupTask.toString());
173+
break;
174+
} finally {
175+
cleanerThreadLock.readLock().lock();
176+
cleanerThreadLock.writeLock().unlock();
171177
}
172-
logger.log(Level.FINER, "Registered Cleaners: {0}", registeredCleaners.toString());
173178
}
179+
} finally {
180+
cleanerThreadLock.readLock().unlock();
181+
}
182+
Logger logger = Logger.getLogger(Cleaner.class.getName());
183+
if (logger.isLoggable(Level.FINER)) {
184+
logger.log(Level.FINER, "Registered Cleaners: {0}", trackedObjects.get());
174185
}
175186
}
176187
} catch (InterruptedException ex) {

0 commit comments

Comments
 (0)