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

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes
From:       Mandy Chung <mandy.chung () oracle ! com>
Date:       2020-03-31 19:25:53
Message-ID: cfe9c4ae-f70e-6408-9d8b-70f02158ff3a () oracle ! com
[Download RAW message or body]

Alex's feedback:  rename isHiddenClass to isHidden as it can be a hidden 
class or interface.

`isLocalClass` and `sAnonymousClass` are correct because the Java 
language only has local classes and anon classes, not local interfaces 
or anon. interfaces.  `isHidden` is like `isSynthetic`, it could be a 
class or interface.

Although isHiddenClass seems clearer, I'm okay to rename it to `isHidden`.

Mandy

On 3/31/20 11:06 AM, Mandy Chung wrote:
> This patch addresses Joe's feedback on the CSR [1]:
>
> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-jdarcy/ 
>
>
> Specifically, it adds to the class specification of java.lang.Class to 
> describe how the relevant methods behave for hidden classes.  In 
> addition, use the new inline @jvms tag.
>
> Thanks
> Mandy
> [1] https://bugs.openjdk.java.net/browse/JDK-8238359
>
> 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 
>>
>>
>> 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-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
>


[Attachment #3 (text/html)]

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    Alex's feedback:  rename isHiddenClass to isHidden as it can be a
    hidden class or interface.<br>
    <br>
    `isLocalClass` and `sAnonymousClass` are correct because the Java
    language only has local classes and anon classes, not local
    interfaces or anon. interfaces.  `isHidden` is like `isSynthetic`,
    it could be a class or interface. <br>
    <br>
    Although isHiddenClass seems clearer, I'm okay to rename it to
    `isHidden`.<br>
    <br>
    Mandy<br>
    <br>
    <div class="moz-cite-prefix">On 3/31/20 11:06 AM, Mandy Chung wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:940c6907-612e-8744-376c-5362991d4a42@oracle.com">This
      patch addresses Joe's feedback on the CSR [1]:
      <br>
      <br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-del \
ta-jdarcy/">http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-jdarcy/</a>
  <br>
      <br>
      Specifically, it adds to the class specification of
      java.lang.Class to describe how the relevant methods behave for
      hidden classes.  In addition, use the new inline @jvms tag.
      <br>
      <br>
      Thanks
      <br>
      Mandy
      <br>
      [1] <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8238359">https://bugs.openjdk.java.net/browse/JDK-8238359</a>
  <br>
      <br>
      On 3/26/20 4:57 PM, Mandy Chung wrote:
      <br>
      <blockquote type="cite">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).
        <br>
        <br>
        Webrev:
        <br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03">http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03</a>
  <br>
        <br>
        Hidden class is created via `Lookup::defineHiddenClass`. From
        JVM's point
        <br>
        of view, a hidden class is a normal class except the following:
        <br>
        <br>
        - A hidden class has no initiating class loader and is not
        registered in any dictionary.
        <br>
        - A hidden class has a name containing an illegal character
        `Class::getName` returns `p.Foo/0x1234` whereas
        `GetClassSignature` returns "Lp/Foo.0x1234;".
        <br>
        - A hidden class is not modifiable, i.e. cannot be redefined or
        retransformed. JVM TI IsModifableClass returns false on a
        hidden.
        <br>
        - 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).
        <br>
        <br>
        Brief summary of this patch:
        <br>
        <br>
        1. A new Lookup::defineHiddenClass method is the API to create a
        hidden class.
        <br>
        2. A new Lookup.ClassOption enum class defines NESTMATE and
        STRONG option that
        <br>
           can be specified when creating a hidden class.
        <br>
        3. A new Class::isHiddenClass method tests if a class is a
        hidden class.
        <br>
        4. Field::setXXX method will throw IAE on a final field of a
        hidden class
        <br>
           regardless of the value of the accessible flag.
        <br>
        5. JVM_LookupDefineClass is the new JVM entry point for
        Lookup::defineClass
        <br>
           and defineHiddenClass to create a class from the given bytes.
        <br>
        6. ClassLoaderData implementation is not changed.  There is one
        primary CLD
        <br>
           that holds the classes strongly referenced by its defining
        loader.  There
        <br>
           can be zero or more additional CLDs - one per weak class.
        <br>
        7. Nest host determination is updated per revised JVMS 5.4.4.
        Access control
        <br>
           check no longer throws LinkageError but instead it will throw
        IAE with
        <br>
           a clear message if a class fails to resolve/validate the nest
        host declared
        <br>
           in NestHost/NestMembers attribute.
        <br>
        8. JFR, jcmd, JDI are updated to support hidden classes.
        <br>
        9. update javac LambdaToMethod as lambda proxy starts using
        nestmates
        <br>
           and generate a bridge method to desuger a method reference to
        a protected
        <br>
           method in its supertype in a different package
        <br>
        <br>
        This patch also updates StringConcatFactory, LambdaMetaFactory,
        and LambdaForms
        <br>
        to use hidden classes.  The webrev includes changes in nashorn
        to hidden class
        <br>
        and I will update the webrev if JEP 372 removes it any time
        soon.
        <br>
        <br>
        We uncovered a bug in Lookup::defineClass spec throws
        LinkageError and intends
        <br>
        to have the newly created class linked.  However, the
        implementation in 14
        <br>
        does not link the class.  A separate CSR [2] proposes to update
        the
        <br>
        implementation to match the spec.  This patch fixes the
        implementation.
        <br>
        <br>
        The spec update on JVM TI, JDI and Instrumentation will be done
        as
        <br>
        a separate RFE [3].  This patch includes new tests for JVM TI
        and
        <br>
        java.instrument that validates how the existing APIs work for
        hidden classes.
        <br>
        <br>
        javadoc/specdiff
        <br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/">http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/api/</a>
  <br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/">http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff/</a>
  <br>
        <br>
        JVMS 5.4.4 change:
        <br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JVMS-Hi \
ddenClasses.pdf">http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/Draft-JVMS-HiddenClasses.pdf</a>
  <br>
        <br>
        CSR:
        <br>
        <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8238359">https://bugs.openjdk.java.net/browse/JDK-8238359</a>
  <br>
        <br>
        Thanks
        <br>
        Mandy
        <br>
        [1] <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8238359">https://bugs.openjdk.java.net/browse/JDK-8238359</a>
  <br>
        [2] <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8240338">https://bugs.openjdk.java.net/browse/JDK-8240338</a>
  <br>
        [3] <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8230502">https://bugs.openjdk.java.net/browse/JDK-8230502</a>
  <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic