Review Request: 8238358: Implementation of JEP 371: Hidden Classes

[Mandy Chung]

On 4/14/20 11:51 AM, Paul Sandoz wrote:
> Looks good to me (not familiar with all the code areas.
>
> Minor suggestion:
>
> MethodHandles.java
>
> 1811          * ASCII periods. For the instance of {@link java.lang.Class} representing {@code C}:
> 1812          * <ul>
> 1813          * <li> {@link Class#getName()} returns the string {@code GN + "/" + <suffix>},
> 1814          *      even though this is not a valid binary class or interface name.</li>
> 1815          * <li> {@link Class#descriptorString()} returns the string
> 1816          *      {@code "L" + N + ";" + "/" + <suffix> },
> 1817          *      even though this is not a valid type descriptor name.</li>
> 1818          * </ul>
>
> Add another bullet:
>
> “
> even though this is not a valid type descriptor name; and
>
> - therefore {@link Class#describeConstable} returns an empty {@code Optional}.
> “
>
> ?

OK.   I add this bullet:

- Class.describeConstable() returns an empty optional as C cannot be 
described in nominal form.

The webrev and spec was updated [1] for descriptor string to be of the 
form "Lfoo/Foo.1234;" to mitigate the compatibility risk.  Th

Specdiff with serviceability spec change:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff-inc/
Specdiff without svc spec change:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff-descriptor-string/overview-summary.html

Webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-descriptor-string/

Svc spec change webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-svc-spec-changes/

thanks
Mandy
[1] 
https://mail.openjdk.java.net/pipermail/valhalla-dev/2020-April/007155.html


>
> Paul.
>
>> On Apr 11, 2020, at 8:13 PM, Mandy Chung <mandy.chung at oracle.com> wrote:
>>
>> Please review the delta patch:
>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-delta/
>>
>> Incremental specdiff:
>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff-inc/overview-summary.html
>> This is to follow a discussion [1] on Class::descriptorString and MethodType::descriptorString for hidden classes.   The proposal is to define the descriptor string for a hidden class of this form:
>>      "L" + N + ";" + "/" + <suffix>
>>
>> The spec of `Lookup::defineHiddenClass`, `Class::descriptorString` and `MethodType::descriptorString` are updated to return the descriptor of this form for a hidden class.   To support hidden class, `java.lang.invoke.TypeDescriptor` spec is revised such that a `TypeDescriptor` object can represent an entity that may not be described in nominal form.     This change affects JVM TI, JDWP and JDI.    The spec change includes a couple other JVM TI and java.instrument spec clarification w.r.t. hidden classes that Serguei has been working on.
>>
>>
>>
>> Mandy
>> [1] https://mail.openjdk.java.net/pipermail/valhalla-dev/2020-April/007093.html
>>
>> ----------------
>> As a record, the above patch is applied on the top:
>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06/
>>
>> webrev.06 includes the following changes that have been reviewed:
>> 1. rename "weak hidden" and Klass::is_hidden_weak to is_non_strong_hidden
>> 2. remove unused `setImplMethod` method from lambda proxy class
>> 3. include Serguei's patch to fix JDK-8242166: regression in JDI ClassUnload events
>> 4. drop the uniqueness guarantee of the suffix of the hidden class's name
>>
>> On 3/31/20 8:01 PM, Mandy Chung wrote:
>>> Thanks to the review feedbacks.
>>>
>>> Updated webrev:
>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04/
>>> Delta between webrev.03 and webrev.04:
>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta/
>>>
>>> Summary of changes is:
>>> 1. Update javac to retain the previous behavior when compiling to target release <= 14 where lambda proxy class is not a nestmate
>>> 2. Rename Class::isHiddenClass to Class::isHidden
>>> 3. Address Joe's feedback on the CSR to document the behavior of the relevant methods in java.lang.Class for hidden classes
>>> 4. Add test case for unloadable class with nest host error
>>> 5. Add test cases for hidden classes with different kinds of class or interface
>>> 6. Update dcmd to drop "weak hidden class" and refer it as "hidden class"
>>> 7. Small changes in response to Remi, Coleen, Paul's review comments
>>> 8. Fix copyright headers
>>> 9. Fix @modules in tests
>>>
>>> Most of the changes above have also been reviewed as separate patches.
>>>
>>> Thanks
>>> Mandy
>>>
>>> On 3/26/20 4:57 PM, Mandy Chung wrote:
>>>> Please review the implementation of JEP 371: Hidden Classes. The main changes are in core-libs and hotspot runtime area.  Small changes are made in javac, VM compiler (intrinsification of Class::isHiddenClass), JFR, JDI, and jcmd.  CSR [1]has been reviewed and is in the finalized state (see specdiff and javadoc below for reference).
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03
>>>>
>>>> javadoc/specdiff
>>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/
>>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/
>>>>
>>>> JVMS 5.4.4 change:
>>>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JVMS-HiddenClasses.pdf
>>>>
>>>> CSR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8238359
>>>>
>>>> Thanks
>>>> Mandy
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8238359
>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8240338
>>>> [3] https://bugs.openjdk.java.net/browse/JDK-8230502

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200416/299050d3/attachment.htm>