On 3/30/20 5:54 AM, David Holmes wrote:
Sorry to jump in on this but it caught my eye though I may be missing a larger context ...
On 30/03/2020 7:30 pm, serguei.spitsyn@oracle.com wrote:
Hi Mandy,
I have just one comment so far.
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03...
356 void add_classes(LoadedClassInfo* first_class, int num_classes, bool has_class_mirror_holder) { 357 LoadedClassInfo** p_list_to_add_to; 358 bool is_hidden = first_class->_klass->is_hidden(); 359 if (has_class_mirror_holder) { 360 p_list_to_add_to = is_hidden ? &_hidden_weak_classes : &_anon_classes; 361 } else { 362 p_list_to_add_to = &_classes; 363 } 364 // Search tail. 365 while ((*p_list_to_add_to) != NULL) { 366 p_list_to_add_to = &(*p_list_to_add_to)->_next; 367 } 368 *p_list_to_add_to = first_class; 369 if (has_class_mirror_holder) { 370 if (is_hidden) { 371 _num_hidden_weak_classes += num_classes;
Why does hidden imply weak here?
has_class_mirror_holder() implies weak. Coleen
David -----
372 } else { 373 _num_anon_classes += num_classes; 374 } 375 } else { 376 _num_classes += num_classes; 377 } 378 }
Q1: I'm just curious, what happens if a cld has arrays of hidden classes? Is the bottom_klass always expected to be the first?
Thanks, Serguei
On 3/26/20 16:57, 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
Hidden class is created via `Lookup::defineHiddenClass`. From JVM's point of view, a hidden class is a normal class except the following:
- A hidden class has no initiating class loader and is not registered in any dictionary. - A hidden class has a name containing an illegal character `Class::getName` returns `p.Foo/0x1234` whereas `GetClassSignature` returns "Lp/Foo.0x1234;". - A hidden class is not modifiable, i.e. cannot be redefined or retransformed. JVM TI IsModifableClass returns false on a hidden. - Final fields in a hidden class is "final". The value of final fields cannot be overriden via reflection. setAccessible(true) can still be called on reflected objects representing final fields in a hidden class and its access check will be suppressed but only have read-access (i.e. can do Field::getXXX but not setXXX).
Brief summary of this patch:
1. A new Lookup::defineHiddenClass method is the API to create a hidden class. 2. A new Lookup.ClassOption enum class defines NESTMATE and STRONG option that can be specified when creating a hidden class. 3. A new Class::isHiddenClass method tests if a class is a hidden class. 4. Field::setXXX method will throw IAE on a final field of a hidden class regardless of the value of the accessible flag. 5. JVM_LookupDefineClass is the new JVM entry point for Lookup::defineClass and defineHiddenClass to create a class from the given bytes. 6. ClassLoaderData implementation is not changed. There is one primary CLD that holds the classes strongly referenced by its defining loader. There can be zero or more additional CLDs - one per weak class. 7. Nest host determination is updated per revised JVMS 5.4.4. Access control check no longer throws LinkageError but instead it will throw IAE with a clear message if a class fails to resolve/validate the nest host declared in NestHost/NestMembers attribute. 8. JFR, jcmd, JDI are updated to support hidden classes. 9. update javac LambdaToMethod as lambda proxy starts using nestmates and generate a bridge method to desuger a method reference to a protected method in its supertype in a different package
This patch also updates StringConcatFactory, LambdaMetaFactory, and LambdaForms to use hidden classes. The webrev includes changes in nashorn to hidden class and I will update the webrev if JEP 372 removes it any time soon.
We uncovered a bug in Lookup::defineClass spec throws LinkageError and intends to have the newly created class linked. However, the implementation in 14 does not link the class. A separate CSR [2] proposes to update the implementation to match the spec. This patch fixes the implementation.
The spec update on JVM TI, JDI and Instrumentation will be done as a separate RFE [3]. This patch includes new tests for JVM TI and java.instrument that validates how the existing APIs work for hidden classes.
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-JVM...
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