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.ch...@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
