Hi Mandy,
On 1/04/2020 1:01 pm, Mandy Chung wrote:
Thanks to the review feedbacks.
Updated webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04/
I've had a good look through all the hotspot related changes. All looks
good.
A few minor comments:
src/hotspot/share/ci/ciField.cpp
// Trust VM hidden and unsafe anonymous classes.
should say
// Trust hidden and VM unsafe anonymous classes.
// the private API (jdk.internal.misc.Unsafe)
s/the/a/ or else change the () to commas or perhaps even better:
// the private jdk.internal.misc.Unsafe API,
---
src/hotspot/share/ci/ciInstanceKlass.cpp
// VM weak hidden and unsafe anonymous classes
should say
// weak hidden and VM unsafe anonymous classes
---
src/hotspot/share/classfile/classLoaderData.hpp
! // the CLDs lifecycle. For example, a non-strong hidden class or an
...
! // Used for weak hidden classes, unsafe anonymous classes and the
There is an inconsistency in terminology, so far most code comments
refer to "weak hidden" but now you are using "non-strong hidden".
! for an weak hidden
s/an/a/ multiple locations
---
src/hotspot/share/interpreter/linkResolver.cpp
Can you fix one of my comments please (in two places) :)
+ // For private access check if there was a problem with nest host
would read better as:
+ // For private access see if there was a problem with nest host
---
Thanks,
David
------
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
