[GR-65296] Use implemented interface as differentiator when generating stable lambda name.

PullRequest: graal/20907

Author: cstancu

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/LambdaStableNameTest.java

@@ -26,11 +26,13 @@
 package jdk.graal.compiler.hotspot.test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.math.BigInteger;
+import java.util.function.Function;
 
 import org.junit.Test;
 import org.objectweb.asm.Type;
@@ -40,22 +42,43 @@
 import jdk.vm.ci.runtime.JVMCI;
 
 public class LambdaStableNameTest {
+
     @Test
     public void checkStableLamdaNameForRunnableAndAutoCloseable() {
         String s = "a string";
-        Runnable r = s::hashCode;
-        ResolvedJavaType rType = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess().lookupJavaType(r.getClass());
+        Runnable r0 = s::hashCode;
+        String r0Name = getLambdaName(r0.getClass());
 
-        String name = LambdaUtils.findStableLambdaName(rType);
-        assertLambdaName(name);
+        Runnable r1 = s::hashCode;
+        String r1Name = getLambdaName(r1.getClass());
+
+        assertEquals("The two stable lambda names should the same as they reference the same method and implement the same interface", r0Name, r1Name);
 
         AutoCloseable ac = s::hashCode;
-        ResolvedJavaType acType = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess().lookupJavaType(ac.getClass());
-        String acName = LambdaUtils.findStableLambdaName(acType);
-        assertEquals("Both stable lambda names are the same as they reference the same method", name, acName);
+        String acName = getLambdaName(ac.getClass());
+
+        assertNotEquals("The two stable lambda names should not be the same as they reference the same method but implement different interfaces", r0Name, acName);
 
         String myName = Type.getInternalName(getClass());
-        assertEquals("The name known in 24.0 version is computed", "L" + myName + "$$Lambda.0x605511206480068bfd9e0bafd4f79e22;", name);
+        assertEquals("The name known in 24.0 version is computed", "L" + myName + "$$Lambda.0x59cf38d78b5471f8ea57f1c28b37039c;", r0Name);
+
+        Function<String, Integer> f0 = (str) -> str.hashCode();
+        String f0Name = getLambdaName(f0.getClass());
+
+        interface ValueTransformer<L, R> extends Function<L, R> {
+        }
+
+        ValueTransformer<String, Integer> f1 = (str) -> str.hashCode();
+        String f1Name = getLambdaName(f1.getClass());
+
+        assertNotEquals("The two stable lambda names should not be the same as they reference the same method but implement different interfaces", f0Name, f1Name);
+    }
+
+    private static String getLambdaName(Class<?> clazz) {
+        ResolvedJavaType type = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess().lookupJavaType(clazz);
+        String name = LambdaUtils.findStableLambdaName(type);
+        assertLambdaName(name);
+        return name;
     }
 
     private static void assertLambdaName(String name) {

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/java/LambdaUtils.java

@@ -37,6 +37,7 @@
 import java.util.regex.Pattern;
 
 import jdk.graal.compiler.bytecode.BytecodeStream;
+import jdk.graal.compiler.debug.GraalError;
 import jdk.graal.compiler.util.Digest;
 import jdk.vm.ci.common.JVMCIError;
 import jdk.vm.ci.meta.ConstantPool;
@@ -141,18 +142,28 @@ public static boolean isLambdaName(String name) {
         return isLambdaClassName(name) && lambdaMatcher(name).find();
     }
 
-    private static String createStableLambdaName(ResolvedJavaType lambdaType, List<JavaMethod> targetMethods) {
+    private static String createStableLambdaName(ResolvedJavaType lambdaType, List<JavaMethod> invokedMethods) {
         final String lambdaName = lambdaType.getName();
         assert lambdaMatcher(lambdaName).find() : "Stable name should be created for lambda types: " + lambdaName;
 
         Matcher m = lambdaMatcher(lambdaName);
+        /* Generate lambda signature by hashing its composing parts. */
         StringBuilder sb = new StringBuilder();
-        targetMethods.forEach((targetMethod) -> sb.append(targetMethod.format("%H.%n(%P)%R")));
-        // Take parameter types of constructor into consideration, see GR-52837
+        /* Append invoked methods. */
+        for (JavaMethod method : invokedMethods) {
+            sb.append(method.format("%H.%n(%P)%R"));
+        }
+        /* Append constructor parameter types. */
         for (JavaMethod ctor : lambdaType.getDeclaredConstructors()) {
             sb.append(ctor.format("%P"));
         }
-        return m.replaceFirst(Matcher.quoteReplacement(LAMBDA_CLASS_NAME_SUBSTRING + ADDRESS_PREFIX + Digest.digestAsHex(sb.toString()) + ";"));
+        /* Append implemented interfaces. */
+        for (ResolvedJavaType iface : lambdaType.getInterfaces()) {
+            sb.append(iface.toJavaName());
+        }
+        String signature = Digest.digestAsHex(sb.toString());
+        GraalError.guarantee(signature.length() == 32, "Expecting a 32 digits long hex value.");
+        return m.replaceFirst(Matcher.quoteReplacement(LAMBDA_CLASS_NAME_SUBSTRING + ADDRESS_PREFIX + signature + ";"));
     }
 
     private static Matcher lambdaMatcher(String value) {

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/util/Digest.java

@@ -123,7 +123,18 @@ private static void encodeBase62(long value, byte[] result, int resultIndex) {
     public static String digestAsHex(String value) {
         byte[] bytes = value.getBytes(StandardCharsets.UTF_8);
         LongLong hash = MurmurHash3_x64_128(bytes, 0, bytes.length, HASH_SEED);
-        return Long.toHexString(hash.l1) + Long.toHexString(hash.l2);
+        String asHex = toFixedSizeHexString(hash.l1) + toFixedSizeHexString(hash.l2);
+        GraalError.guarantee(asHex.length() == 32, "Expecting a 32 digits long hex value.");
+        return asHex;
+    }
+
+    /**
+     * Note: The string generated by {@link Long#toHexString(long)} can vary in length, and it
+     * doesn't include any leading 0s. With {@link String#format(String, Object...)} we inject
+     * leading 0s when necessary to ensure that the produced digest has a fixed size.
+     */
+    private static String toFixedSizeHexString(long l) {
+        return String.format("%016x", l);
     }
 
     public static UUID digestAsUUID(String value) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/lambda/LambdaProxyRenamingSubstitutionProcessor.java

@@ -71,9 +71,16 @@ private LambdaSubstitutionType getSubstitution(ResolvedJavaType original) {
     }
 
     /**
-     * Finds a unique name for a lambda proxies with a same target originating from the same class.
-     *
-     * NOTE: the name truly stable only in a single threaded build.
+     * Finds a unique name for lambda proxies with the same target originating from the same class.
+     * Method {@link LambdaUtils#findStableLambdaName(ResolvedJavaType)} hashes over a lambda type
+     * descriptor (invoked methods, constructor parameters, implemented interfaces) in an effort to
+     * find enough differences that can yield a unique name. However, this can still lead to
+     * conflicts. To create unique names when conflicts are found the naming scheme uses a counter:
+     * first a 0 is added to the end of the generated lambda name, and it is incremented until a
+     * unique name is found. This also means that the name is truly stable only in a single threaded
+     * build, i.e., when conflicts are discovered in the same order. Since the conflict counter
+     * doesn't have an upper limit the generated lambda name length can vary but the base hash is
+     * always 32 hex digits long.
      */
     private String findUniqueLambdaProxyName(String lambdaTargetName) {
         synchronized (uniqueLambdaProxyNames) {
@@ -91,7 +98,22 @@ private String findUniqueLambdaProxyName(String lambdaTargetName) {
         }
     }
 
+    /**
+     * Find the base name generated by {@link LambdaUtils#findStableLambdaName(ResolvedJavaType)} by
+     * subtracting the counter injected by {@link #findUniqueLambdaProxyName(String)}. Note that the
+     * counter can be on multiple digits but the base hash is always 32 hex digits long.
+     */
+    private static String getBaseName(String lambdaTargetName) {
+        int startIndexOfHash = lambdaTargetName.indexOf(LambdaUtils.ADDRESS_PREFIX) + LambdaUtils.ADDRESS_PREFIX.length();
+        return lambdaTargetName.substring(0, startIndexOfHash + 32);
+    }
+
+    /**
+     * Check if this lambda's name conflicted with any other entries, i.e., if the conflict counter
+     * ever got to 1.
+     */
     public boolean isNameAlwaysStable(String lambdaTargetName) {
-        return !uniqueLambdaProxyNames.contains(lambdaTargetName.substring(0, lambdaTargetName.length() - 1) + "1;");
+        String baseName = getBaseName(lambdaTargetName);
+        return !uniqueLambdaProxyNames.contains(baseName + "1;");
     }
 }