8057020: LambdaForm caches should support eviction
authorvlivanov
Thu, 04 Dec 2014 07:15:37 -0800
changeset 27803 d04ca9d519ce
parent 27802 c6d453fa55bb
child 27804 4659e70271c4
8057020: LambdaForm caches should support eviction Reviewed-by: psandoz, jrose, shade
jdk/src/java.base/share/classes/java/lang/invoke/LambdaForm.java
jdk/src/java.base/share/classes/java/lang/invoke/LambdaFormBuffer.java
jdk/src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java
jdk/src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java
jdk/test/java/lang/invoke/LFCaching/LFCachingTestCase.java
jdk/test/java/lang/invoke/LFCaching/LambdaFormTestCase.java
--- a/jdk/src/java.base/share/classes/java/lang/invoke/LambdaForm.java	Thu Dec 04 16:50:31 2014 +0800
+++ b/jdk/src/java.base/share/classes/java/lang/invoke/LambdaForm.java	Thu Dec 04 07:15:37 2014 -0800
@@ -125,7 +125,7 @@
     MemberName vmentry;   // low-level behavior, or null if not yet prepared
     private boolean isCompiled;
 
-    Object transformCache;  // managed by LambdaFormEditor
+    volatile Object transformCache;  // managed by LambdaFormEditor
 
     public static final int VOID_RESULT = -1, LAST_RESULT = -2;
 
--- a/jdk/src/java.base/share/classes/java/lang/invoke/LambdaFormBuffer.java	Thu Dec 04 16:50:31 2014 +0800
+++ b/jdk/src/java.base/share/classes/java/lang/invoke/LambdaFormBuffer.java	Thu Dec 04 07:15:37 2014 -0800
@@ -46,19 +46,16 @@
     private static final int F_TRANS = 0x10, F_OWNED = 0x03;
 
     LambdaFormBuffer(LambdaForm lf) {
-        this(lf.arity, lf.names, lf.result);
+        this.arity = lf.arity;
+        setNames(lf.names);
+        int result = lf.result;
+        if (result == LAST_RESULT)  result = length - 1;
+        if (result >= 0 && lf.names[result].type != V_TYPE)
+            resultName = lf.names[result];
         debugName = lf.debugName;
         assert(lf.nameRefsAreLegal());
     }
 
-    private LambdaFormBuffer(int arity, Name[] names, int result) {
-        this.arity = arity;
-        setNames(names);
-        if (result == LAST_RESULT)  result = length - 1;
-        if (result >= 0 && names[result].type != V_TYPE)
-            resultName = names[result];
-    }
-
     private LambdaForm lambdaForm() {
         assert(!inTrans());  // need endEdit call to tidy things up
         return new LambdaForm(debugName, arity, nameArray(), resultIndex());
--- a/jdk/src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java	Thu Dec 04 16:50:31 2014 +0800
+++ b/jdk/src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java	Thu Dec 04 07:15:37 2014 -0800
@@ -25,6 +25,7 @@
 
 package java.lang.invoke;
 
+import java.lang.ref.SoftReference;
 import java.util.Arrays;
 import static java.lang.invoke.LambdaForm.*;
 import static java.lang.invoke.LambdaForm.BasicType.*;
@@ -58,10 +59,9 @@
      *  The sequence is unterminated, ending with an indefinite number of zero bytes.
      *  Sequences that are simple (short enough and with small enough values) pack into a 64-bit long.
      */
-    private static final class Transform {
+    private static final class Transform extends SoftReference<LambdaForm> {
         final long packedBytes;
         final byte[] fullBytes;
-        final LambdaForm result;  // result of transform, or null, if there is none available
 
         private enum Kind {
             NO_KIND,  // necessary because ordinal must be greater than zero
@@ -140,9 +140,9 @@
         Kind kind() { return Kind.values()[byteAt(0)]; }
 
         private Transform(long packedBytes, byte[] fullBytes, LambdaForm result) {
+            super(result);
             this.packedBytes = packedBytes;
             this.fullBytes = fullBytes;
-            this.result = result;
         }
         private Transform(long packedBytes) {
             this(packedBytes, null, null);
@@ -243,6 +243,7 @@
                 buf.append("unpacked");
                 buf.append(Arrays.toString(fullBytes));
             }
+            LambdaForm result = get();
             if (result != null) {
                 buf.append(" result=");
                 buf.append(result);
@@ -253,7 +254,7 @@
 
     /** Find a previously cached transform equivalent to the given one, and return its result. */
     private LambdaForm getInCache(Transform key) {
-        assert(key.result == null);
+        assert(key.get() == null);
         // The transformCache is one of null, Transform, Transform[], or ConcurrentHashMap.
         Object c = lambdaForm.transformCache;
         Transform k = null;
@@ -276,7 +277,7 @@
             }
         }
         assert(k == null || key.equals(k));
-        return k == null ? null : k.result;
+        return (k != null) ? k.get() : null;
     }
 
     /** Arbitrary but reasonable limits on Transform[] size for cache. */
@@ -293,7 +294,17 @@
                 @SuppressWarnings("unchecked")
                 ConcurrentHashMap<Transform,Transform> m = (ConcurrentHashMap<Transform,Transform>) c;
                 Transform k = m.putIfAbsent(key, key);
-                return k != null ? k.result : form;
+                if (k == null) return form;
+                LambdaForm result = k.get();
+                if (result != null) {
+                    return result;
+                } else {
+                    if (m.replace(key, k, key)) {
+                        return form;
+                    } else {
+                        continue;
+                    }
+                }
             }
             assert(pass == 0);
             synchronized (lambdaForm) {
@@ -308,17 +319,27 @@
                 if (c instanceof Transform) {
                     Transform k = (Transform)c;
                     if (k.equals(key)) {
-                        return k.result;
+                        LambdaForm result = k.get();
+                        if (result == null) {
+                            lambdaForm.transformCache = key;
+                            return form;
+                        } else {
+                            return result;
+                        }
+                    } else if (k.get() == null) { // overwrite stale entry
+                        lambdaForm.transformCache = key;
+                        return form;
                     }
                     // expand one-element cache to small array
                     ta = new Transform[MIN_CACHE_ARRAY_SIZE];
                     ta[0] = k;
-                    lambdaForm.transformCache = c = ta;
+                    lambdaForm.transformCache = ta;
                 } else {
                     // it is already expanded
                     ta = (Transform[])c;
                 }
                 int len = ta.length;
+                int stale = -1;
                 int i;
                 for (i = 0; i < len; i++) {
                     Transform k = ta[i];
@@ -326,10 +347,18 @@
                         break;
                     }
                     if (k.equals(key)) {
-                        return k.result;
+                        LambdaForm result = k.get();
+                        if (result == null) {
+                            ta[i] = key;
+                            return form;
+                        } else {
+                            return result;
+                        }
+                    } else if (stale < 0 && k.get() == null) {
+                        stale = i; // remember 1st stale entry index
                     }
                 }
-                if (i < len) {
+                if (i < len || stale >= 0) {
                     // just fall through to cache update
                 } else if (len < MAX_CACHE_ARRAY_SIZE) {
                     len = Math.min(len * 2, MAX_CACHE_ARRAY_SIZE);
@@ -344,7 +373,8 @@
                     // The second iteration will update for this query, concurrently.
                     continue;
                 }
-                ta[i] = key;
+                int idx = (stale >= 0) ? stale : i;
+                ta[idx] = key;
                 return form;
             }
         }
--- a/jdk/src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java	Thu Dec 04 16:50:31 2014 +0800
+++ b/jdk/src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java	Thu Dec 04 07:15:37 2014 -0800
@@ -26,9 +26,8 @@
 package java.lang.invoke;
 
 import sun.invoke.util.Wrapper;
+import java.lang.ref.SoftReference;
 import static java.lang.invoke.MethodHandleStatics.*;
-import static java.lang.invoke.MethodHandleNatives.Constants.*;
- import static java.lang.invoke.MethodHandles.Lookup.IMPL_LOOKUP;
 
 /**
  * Shared information for a group of method types, which differ
@@ -51,7 +50,7 @@
     final MethodType basicType;         // the canonical erasure, with primitives simplified
 
     // Cached adapter information:
-    @Stable final MethodHandle[] methodHandles;
+    @Stable final SoftReference<MethodHandle>[] methodHandles;
     // Indexes into methodHandles:
     static final int
             MH_BASIC_INV      =  0,  // cached instance of MH.invokeBasic
@@ -60,7 +59,7 @@
             MH_LIMIT          =  3;
 
     // Cached lambda form information, for basic types only:
-    final @Stable LambdaForm[] lambdaForms;
+    final @Stable SoftReference<LambdaForm>[] lambdaForms;
     // Indexes into lambdaForms:
     static final int
             LF_INVVIRTUAL              =  0,  // DMH invokeVirtual
@@ -108,26 +107,40 @@
 
     public MethodHandle cachedMethodHandle(int which) {
         assert(assertIsBasicType());
-        return methodHandles[which];
+        SoftReference<MethodHandle> entry = methodHandles[which];
+        return (entry != null) ? entry.get() : null;
     }
 
     synchronized public MethodHandle setCachedMethodHandle(int which, MethodHandle mh) {
         // Simulate a CAS, to avoid racy duplication of results.
-        MethodHandle prev = methodHandles[which];
-        if (prev != null)  return prev;
-        return methodHandles[which] = mh;
+        SoftReference<MethodHandle> entry = methodHandles[which];
+        if (entry != null) {
+            MethodHandle prev = entry.get();
+            if (prev != null) {
+                return prev;
+            }
+        }
+        methodHandles[which] = new SoftReference<>(mh);
+        return mh;
     }
 
     public LambdaForm cachedLambdaForm(int which) {
         assert(assertIsBasicType());
-        return lambdaForms[which];
+        SoftReference<LambdaForm> entry = lambdaForms[which];
+        return (entry != null) ? entry.get() : null;
     }
 
     synchronized public LambdaForm setCachedLambdaForm(int which, LambdaForm form) {
         // Simulate a CAS, to avoid racy duplication of results.
-        LambdaForm prev = lambdaForms[which];
-        if (prev != null) return prev;
-        return lambdaForms[which] = form;
+        SoftReference<LambdaForm> entry = lambdaForms[which];
+        if (entry != null) {
+            LambdaForm prev = entry.get();
+            if (prev != null) {
+                return prev;
+            }
+        }
+        lambdaForms[which] = new SoftReference<>(form);
+        return form;
     }
 
     /**
@@ -135,6 +148,7 @@
      * This MTF will stand for that type and all un-erased variations.
      * Eagerly compute some basic properties of the type, common to all variations.
      */
+    @SuppressWarnings({"rawtypes", "unchecked"})
     protected MethodTypeForm(MethodType erasedType) {
         this.erasedType = erasedType;
 
@@ -234,8 +248,8 @@
 
         // Initialize caches, but only for basic types
         assert(basicType == erasedType);
-        this.lambdaForms = new LambdaForm[LF_LIMIT];
-        this.methodHandles = new MethodHandle[MH_LIMIT];
+        this.lambdaForms   = new SoftReference[LF_LIMIT];
+        this.methodHandles = new SoftReference[MH_LIMIT];
     }
 
     private static long pack(int a, int b, int c, int d) {
@@ -409,5 +423,4 @@
     public String toString() {
         return "Form"+erasedType;
     }
-
 }
--- a/jdk/test/java/lang/invoke/LFCaching/LFCachingTestCase.java	Thu Dec 04 16:50:31 2014 +0800
+++ b/jdk/test/java/lang/invoke/LFCaching/LFCachingTestCase.java	Thu Dec 04 07:15:37 2014 -0800
@@ -63,12 +63,17 @@
             }
 
             if (lambdaForm0 != lambdaForm1) {
-                System.err.println("Lambda form 0 toString is:");
-                System.err.println(lambdaForm0);
-                System.err.println("Lambda form 1 toString is:");
-                System.err.println(lambdaForm1);
-                throw new AssertionError("Error: Lambda forms of the two method handles"
-                        + " are not the same. LF cahing does not work");
+                // Since LambdaForm caches are based on SoftReferences, GC can cause element eviction.
+                if (noGCHappened()) {
+                    System.err.println("Lambda form 0 toString is:");
+                    System.err.println(lambdaForm0);
+                    System.err.println("Lambda form 1 toString is:");
+                    System.err.println(lambdaForm1);
+                    throw new AssertionError("Error: Lambda forms of the two method handles"
+                            + " are not the same. LF cahing does not work");
+                } else {
+                    System.err.println("LambdaForms differ, but there was a GC in between. Ignore the failure.");
+                }
             }
         } catch (IllegalAccessException | IllegalArgumentException |
                 SecurityException | InvocationTargetException ex) {
--- a/jdk/test/java/lang/invoke/LFCaching/LambdaFormTestCase.java	Thu Dec 04 16:50:31 2014 +0800
+++ b/jdk/test/java/lang/invoke/LFCaching/LambdaFormTestCase.java	Thu Dec 04 07:15:37 2014 -0800
@@ -23,9 +23,12 @@
 
 import com.oracle.testlibrary.jsr292.Helper;
 import com.sun.management.HotSpotDiagnosticMXBean;
+
+import java.lang.management.GarbageCollectorMXBean;
 import java.lang.management.ManagementFactory;
 import java.lang.reflect.Method;
 import java.util.Collection;
+import java.util.List;
 import java.util.function.Function;
 import jdk.testlibrary.Utils;
 
@@ -49,6 +52,11 @@
      * used to get a lambda form from a method handle.
      */
     protected final static Method INTERNAL_FORM;
+    private static final List<GarbageCollectorMXBean> gcInfo;
+
+    private static long gcCount() {
+        return gcInfo.stream().mapToLong(GarbageCollectorMXBean::getCollectionCount).sum();
+    }
 
     static {
         try {
@@ -58,10 +66,15 @@
         } catch (Exception ex) {
             throw new Error("Unexpected exception: ", ex);
         }
+
+        gcInfo = ManagementFactory.getGarbageCollectorMXBeans();
+        if (gcInfo.size() == 0)  {
+            throw new Error("No GarbageCollectorMXBeans found.");
+        }
     }
 
     private final TestMethods testMethod;
-
+    private long gcCountAtStart;
     /**
      * Test case constructor. Generates test cases with random method types for
      * given methods form {@code j.l.i.MethodHandles} class.
@@ -71,12 +84,17 @@
      */
     protected LambdaFormTestCase(TestMethods testMethod) {
         this.testMethod = testMethod;
+        this.gcCountAtStart = gcCount();
     }
 
     public TestMethods getTestMethod() {
         return testMethod;
     }
 
+    protected boolean noGCHappened() {
+        return gcCount() == gcCountAtStart;
+    }
+
     /**
      * Routine that executes a test case.
      */