6899078: potential deadlock and performance issue in freeing strike resources with D3D pipeline
Reviewed-by: tdv, igor
--- a/jdk/src/share/classes/sun/font/Font2D.java Tue Nov 03 23:23:15 2009 +0000
+++ b/jdk/src/share/classes/sun/font/Font2D.java Mon Nov 09 14:23:49 2009 -0800
@@ -320,21 +320,6 @@
lastFontStrike = new SoftReference(strike);
StrikeCache.refStrike(strike);
return strike;
- } else {
- /* We have found a cleared reference that has not yet
- * been removed by the disposer.
- * If we make this reference unreachable by removing it
- * from the map,or overwriting it with a new reference to
- * a new strike, then it is possible it may never be
- * enqueued for disposal. That is the implication of
- * the docs for java.lang.ref. So on finding a cleared
- * reference, we need to dispose the native resources,
- * if they haven't already been freed.
- * The reference object needs to have a reference to
- * the disposer instance for this to occur.
- */
- ((StrikeCache.DisposableStrike)strikeRef)
- .getDisposer().dispose();
}
}
/* When we create a new FontStrike instance, we *must*
--- a/jdk/src/share/classes/sun/font/FontDesignMetrics.java Tue Nov 03 23:23:15 2009 +0000
+++ b/jdk/src/share/classes/sun/font/FontDesignMetrics.java Mon Nov 09 14:23:49 2009 -0800
@@ -171,7 +171,7 @@
* out we can clear the keys from the table.
*/
private static class KeyReference extends SoftReference
- implements DisposerRecord {
+ implements DisposerRecord, Disposer.PollDisposable {
static ReferenceQueue queue = Disposer.getQueue();
--- a/jdk/src/share/classes/sun/font/FontStrikeDisposer.java Tue Nov 03 23:23:15 2009 +0000
+++ b/jdk/src/share/classes/sun/font/FontStrikeDisposer.java Mon Nov 09 14:23:49 2009 -0800
@@ -25,6 +25,7 @@
package sun.font;
+import sun.java2d.Disposer;
import sun.java2d.DisposerRecord;
/*
@@ -49,7 +50,8 @@
* entries would be removed much more promptly than we need.
*/
-class FontStrikeDisposer implements DisposerRecord {
+class FontStrikeDisposer
+ implements DisposerRecord, Disposer.PollDisposable {
Font2D font2D;
FontStrikeDesc desc;
--- a/jdk/src/share/classes/sun/font/StrikeCache.java Tue Nov 03 23:23:15 2009 +0000
+++ b/jdk/src/share/classes/sun/font/StrikeCache.java Mon Nov 09 14:23:49 2009 -0800
@@ -254,9 +254,20 @@
// because they may be accessed on that thread at the time of the
// disposal (for example, when the accel. cache is invalidated)
- // REMIND: this look a bit heavyweight, but should be ok
- // because strike disposal is a relatively infrequent operation,
- // more worrisome is the necessity of getting a GC here.
+ // Whilst this is a bit heavyweight, in most applications
+ // strike disposal is a relatively infrequent operation, so it
+ // doesn't matter. But in some tests that use vast numbers
+ // of strikes, the switching back and forth is measurable.
+ // So the "pollRemove" call is added to batch up the work.
+ // If we are polling we know we've already been called back
+ // and can directly dispose the record.
+ // Also worrisome is the necessity of getting a GC here.
+
+ if (Disposer.pollingQueue) {
+ doDispose(disposer);
+ return;
+ }
+
RenderQueue rq = null;
GraphicsEnvironment ge =
GraphicsEnvironment.getLocalGraphicsEnvironment();
@@ -277,6 +288,7 @@
rq.flushAndInvokeNow(new Runnable() {
public void run() {
doDispose(disposer);
+ Disposer.pollRemove();
}
});
} finally {
--- a/jdk/src/share/classes/sun/java2d/Disposer.java Tue Nov 03 23:23:15 2009 +0000
+++ b/jdk/src/share/classes/sun/java2d/Disposer.java Mon Nov 09 14:23:49 2009 -0800
@@ -29,6 +29,7 @@
import java.lang.ref.ReferenceQueue;
import java.lang.ref.PhantomReference;
import java.lang.ref.WeakReference;
+import java.util.ArrayList;
import java.util.Hashtable;
/**
@@ -146,6 +147,7 @@
rec.dispose();
obj = null;
rec = null;
+ clearDeferredRecords();
} catch (Exception e) {
System.out.println("Exception while removing reference: " + e);
e.printStackTrace();
@@ -153,6 +155,85 @@
}
}
+ /*
+ * This is a marker interface that, if implemented, means it
+ * doesn't acquire any special locks, and is safe to
+ * be disposed in the poll loop on whatever thread
+ * which happens to be the Toolkit thread, is in use.
+ */
+ public static interface PollDisposable {
+ };
+
+ private static ArrayList<DisposerRecord> deferredRecords = null;
+
+ private static void clearDeferredRecords() {
+ if (deferredRecords == null || deferredRecords.isEmpty()) {
+ return;
+ }
+ for (int i=0;i<deferredRecords.size(); i++) {
+ try {
+ DisposerRecord rec = deferredRecords.get(i);
+ rec.dispose();
+ } catch (Exception e) {
+ System.out.println("Exception while disposing deferred rec.");
+ e.printStackTrace();
+ }
+ }
+ deferredRecords.clear();
+ }
+
+ /*
+ * Set to indicate the queue is presently being polled.
+ */
+ public static volatile boolean pollingQueue = false;
+
+ /*
+ * The pollRemove() method is called back from a dispose method
+ * that is running on the toolkit thread and wants to
+ * dispose any pending refs that are safe to be disposed
+ * on that thread.
+ */
+ public static void pollRemove() {
+
+ /* This should never be called recursively, so this check
+ * is just a safeguard against the unexpected.
+ */
+ if (pollingQueue) {
+ return;
+ }
+ Object obj;
+ pollingQueue = true;
+ int freed = 0;
+ int deferred = 0;
+ try {
+ while ((obj = queue.poll()) != null
+ && freed < 10000 && deferred < 100) {
+ freed++;
+ ((Reference)obj).clear();
+ DisposerRecord rec = (DisposerRecord)records.remove(obj);
+ if (rec instanceof PollDisposable) {
+ rec.dispose();
+ obj = null;
+ rec = null;
+ } else {
+ if (rec == null) { // shouldn't happen, but just in case.
+ continue;
+ }
+ deferred++;
+ if (deferredRecords == null) {
+ deferredRecords = new ArrayList<DisposerRecord>(5);
+ }
+ deferredRecords.add(rec);
+ }
+ }
+ } catch (Exception e) {
+ System.out.println("Exception while removing reference: " + e);
+ e.printStackTrace();
+ } finally {
+ pollingQueue = false;
+ }
+ }
+
private static native void initIDs();
/*