Review Request: 8242012: Drop the uniqueness guarantee from the suffix of the name of a hidden class

[Mandy Chung]

On 4/1/20 7:34 PM, John Rose wrote:
> On Apr 1, 2020, at 6:19 PM, Mandy Chung <mandy.chung at oracle.com 
> <mailto:mandy.chung at oracle.com>> wrote:
>>
>> I filehttps://bugs.openjdk.java.net/browse/JDK-8242012to track this 
>> issue.
>
> I would prefer to say that the suffix is simply an unspecified
> simple name.  Then leave it open for implementations to make it more
> or less unpredictable, as a matter of QoS.
> The standard of quality we
> should aim at is that of System.identityHashCode—nothing much more
> than that is desirable, and it’s easy to get at least that much quality.
>
> The java doc for Object.hashCode says this about the QoS I want here:
>
>>      * As far as is reasonably practical, the {@code hashCode} method 
>> defined
>>      * by class {@code Object} returns distinct integers for distinct 
>> objects.
>
> So, the behavior I would like to keep open is something that cannot
> readily be distinguished from functions like these…
>
> static String nextSuffix1() { return new 
> Integer.toHexString(Object().hashCode()); }
> static String nextSuffix2() { return new 
> Integer.toUnsignedString(Object().hashCode(), 36); }
> static String nextSuffix3() { return nextSuffix1()+nextSuffix2(); }
>
> …and many others.
>
> And I would settle for nextSuffix1, which (after all) is also a suffix of
> the already specified value of new Object().toString().  To me, the result
> of HC.toString() (where HC is a hidden class) should not be more deeply
> informative than new Object().toString().  Also, the behavior currently
> coded (in mangle_hidden_class_name) is consistent with nextSuffix1.
>
> (Observation:  The JVM has no reason to choose this suffix, beyond the
> simple reason that it is the first party to peek into the byte[] array of
> the classfile image.  It would be nice if the JDK platform code could
> take responsibility for generating that extra suffix; it would make QoS
> adjustments easier down the road.  We could even go all the way and
> guarantee that all such suffixes are truly unique; it would be a page of
> Java code but several pages of HotSpot code.  But, that’s inconvenient,
> so I don’t recommend that now.)
>
> A parting shot:  Yes, technically the implementation of the suffix
> choosing (mangle_hidden_class_name) probably upholds almost the full
> guarantee.  (Although, it can fail if the JVM unloads and reloads a HC
> of the same name at the same metadata address.)  But if we make the
> full guarantee part of the API, we set ourselves up to maintain that 
> strong
> behavior even if we move to another implementation in which metadata
> pointers are allowed to change.  I know we don’t want to do that, but
> this is an example of how an overly strong guarantee that seems easy
> to do in *one* JVM implementation can create headaches for *other*
> JVM implementations.  What’s easy to do in our implementation is
> *often* close to the right primitive to specify, but we have a 
> responsibility
> to remove unnecessary promises and other implementation artifacts
> from our specification.
>
> — John

Thanks John for this.

This is the spec update.   I leave the mangle_hidden_class_name as is.  
We can move out the suffix assignment from the VM to the library code as 
a future cleanup.

diff --git a/src/java.base/share/classes/java/lang/Class.java 
b/src/java.base/share/classes/java/lang/Class.java
--- a/src/java.base/share/classes/java/lang/Class.java
+++ b/src/java.base/share/classes/java/lang/Class.java
@@ -799,8 +799,7 @@
       *      where {@code N} is the <a 
href="ClassLoader.html#binary-name">binary name</a>
       *      indicated by the {@code class} file passed to
       *      {@link 
java.lang.invoke.MethodHandles.Lookup#defineHiddenClass(byte[], boolean, 
MethodHandles.Lookup.ClassOption...)
-     *      Lookup::defineHiddenClass} and {@code <suffix>} is an 
unqualified name
-     *      that is guaranteed to be unique during this execution of 
the JVM.
+     *      Lookup::defineHiddenClass}, and {@code <suffix>} is an 
unqualified name.
       * </ul>
       *
       * <p> If this {@code Class} object represents an array class, then
diff --git 
a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java 
b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
@@ -1781,8 +1781,7 @@
           * denote a class or interface in the same package as the 
lookup class.</li>
           *
           * <li> Let {@code CN} be the string {@code N + "." + <suffix>},
-         * where {@code <suffix>} is an unqualified name that is 
guaranteed to be unique
-         * during this execution of the JVM.
+         * where {@code <suffix>} is an unqualified name.
           *
           * <p> Let {@code newBytes} be the {@code ClassFile} structure 
given by
           * {@code bytes} with an additional entry in the {@code 
constant_pool} table,

thanks
Mandy