From 2fdf51d313d0cfe3873df81990a47299bc8aed3a Mon Sep 17 00:00:00 2001 From: Phil Race Date: Mon, 9 Nov 2009 14:23:49 -0800 Subject: [PATCH] 6899078: potential deadlock and performance issue in freeing strike resources with D3D pipeline Reviewed-by: tdv, igor --- jdk/src/share/classes/sun/font/Font2D.java | 15 ---- .../classes/sun/font/FontDesignMetrics.java | 2 +- .../classes/sun/font/FontStrikeDisposer.java | 4 +- .../share/classes/sun/font/StrikeCache.java | 18 ++++- .../share/classes/sun/java2d/Disposer.java | 81 +++++++++++++++++++ 5 files changed, 100 insertions(+), 20 deletions(-) diff --git a/jdk/src/share/classes/sun/font/Font2D.java b/jdk/src/share/classes/sun/font/Font2D.java index c344ebb7007..3a930d6502e 100644 --- a/jdk/src/share/classes/sun/font/Font2D.java +++ b/jdk/src/share/classes/sun/font/Font2D.java @@ -320,21 +320,6 @@ public abstract class Font2D { 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* diff --git a/jdk/src/share/classes/sun/font/FontDesignMetrics.java b/jdk/src/share/classes/sun/font/FontDesignMetrics.java index 929c0d21dd8..e81c397fdea 100644 --- a/jdk/src/share/classes/sun/font/FontDesignMetrics.java +++ b/jdk/src/share/classes/sun/font/FontDesignMetrics.java @@ -171,7 +171,7 @@ public final class FontDesignMetrics extends FontMetrics { * 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(); diff --git a/jdk/src/share/classes/sun/font/FontStrikeDisposer.java b/jdk/src/share/classes/sun/font/FontStrikeDisposer.java index 7245cc3736f..3ee33eccea6 100644 --- a/jdk/src/share/classes/sun/font/FontStrikeDisposer.java +++ b/jdk/src/share/classes/sun/font/FontStrikeDisposer.java @@ -25,6 +25,7 @@ package sun.font; +import sun.java2d.Disposer; import sun.java2d.DisposerRecord; /* @@ -49,7 +50,8 @@ import sun.java2d.DisposerRecord; * entries would be removed much more promptly than we need. */ -class FontStrikeDisposer implements DisposerRecord { +class FontStrikeDisposer + implements DisposerRecord, Disposer.PollDisposable { Font2D font2D; FontStrikeDesc desc; diff --git a/jdk/src/share/classes/sun/font/StrikeCache.java b/jdk/src/share/classes/sun/font/StrikeCache.java index 945b01f867e..9e5b02a8650 100644 --- a/jdk/src/share/classes/sun/font/StrikeCache.java +++ b/jdk/src/share/classes/sun/font/StrikeCache.java @@ -254,9 +254,20 @@ public final class StrikeCache { // 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 @@ public final class StrikeCache { rq.flushAndInvokeNow(new Runnable() { public void run() { doDispose(disposer); + Disposer.pollRemove(); } }); } finally { diff --git a/jdk/src/share/classes/sun/java2d/Disposer.java b/jdk/src/share/classes/sun/java2d/Disposer.java index bf9ea8ad71a..4433a278d5d 100644 --- a/jdk/src/share/classes/sun/java2d/Disposer.java +++ b/jdk/src/share/classes/sun/java2d/Disposer.java @@ -29,6 +29,7 @@ import java.lang.ref.Reference; 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 @@ public class Disposer implements Runnable { rec.dispose(); obj = null; rec = null; + clearDeferredRecords(); } catch (Exception e) { System.out.println("Exception while removing reference: " + e); e.printStackTrace(); @@ -153,6 +155,85 @@ public class Disposer implements Runnable { } } + /* + * 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 deferredRecords = null; + + private static void clearDeferredRecords() { + if (deferredRecords == null || deferredRecords.isEmpty()) { + return; + } + for (int i=0;i(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(); /*