Review Request: 8238358: Implementation of JEP 371: Hidden Classes
Mandy Chung
mandy.chung at oracle.com
Mon Apr 20 20:57:36 UTC 2020
On 4/20/20 1:06 PM, Chris Plummer wrote:
> On 4/18/20 3:28 PM, Mandy Chung wrote:
>> Hi Chris, Serguei,
>>
>> On 4/18/20 12:18 AM, Chris Plummer wrote:
>>>
>>> Yes. I'd like to see all this as part of the Class/Classloading spec
>>> in some sort of section that gives an overview of all these topics,
>>> but mostly clarifies what an initiating loader is, and the (non)
>>> relationship to hidden classes.
>>
>> We should refer initiating loader and class loading from JVMS 5.3.
>> JVM TI needs the clarification w.r.t. GetClassLoaderClasses that does
>> not include hidden classes because the initiating class loader cannot
>> find it.
>>
>> GetLoadedClasses is about class creation. While it seems tempted to
>> put the descriptions in some sort of overview section, I found the
>> clarification is specific for each function and hence inlining them
>> in each function helps the readers directly see the difference.
>>
>> I made a few changes that should ease your concern of duplication:
>> http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-svc-spec-changes.01/
>>
>>
>> - The class creation description added in GetLoadedClasses. Not in
>> JDWP, JDI and Instrumentation.
>> - The description in GetClassLoaderClasses,
>> Instrumentation::getInitiatedClasses,
>> ClassLoaderReference::visibleClasses is revised to take out the
>> details. Add links from JDWP and JDI to GetClassLoaderClasses.
>> - The details about hidden classes can be found from
>> `Class::isHidden` and `Lookup::defineHiddenClass`.
>>
>> Mandy
> Hi Mandy,
>
> This looks much better. Thank you for the consolidating and the cross
> references. I think that's a big improvement.
>
> One minor thing I noticed is a few "JNI-style signature" references.
> In LocalVariable.java you renamed "JNI signature" to "JNI-style
> signature", but in JDI Mirror.java you renamed "JNI-style signature"
> to "type signature".
You meant Type::signature. Type::signature may return the type
signature of a hidden class whereas LocalVariable::signature and
TypeComponent::signature return the field/method descriptor where no
hidden class will appear in a resolved field/variable/method.
> And then in JDI TypeComponent.java you left the "JNI-style signature"
> reference unchanged. Why the inconsistency?
>
I made LocalVariable::signature consistent with TypeComponent::signature
whereas Type::signature returns the string of the form described in
Class::descriptorString.
I tried to keep existing reference to use "JNI-style signature". Since I
touched on these files, I can change them to "type signature" which
applies to these cases but I will leave the link to the JNI spec as it is.
> In VirtualMachine.java:
>
> 145 * @see Class or interface creation specified in
> 146 * <a
> href="{@docRoot}/../specs/jvmti/jvmti.html#GetLoadedClasses">JVM TI
> GetLoadedClasses</a>
>
> How about:
>
> 145 * @a description of how Class and interface creation can be
> triggered is specified in
> 146 * <a
> href="{@docRoot}/../specs/jvmti/jvmti.html#GetLoadedClasses">JVM TI
> GetLoadedClasses</a>
>
* @see <a
href="{@docRoot}/../specs/jvmti/jvmti.html#GetLoadedClasses">
* JVM TI GetLoadedClasses</a> regarding how class and interface
creation can be triggered
> I read the following incorrectly a few times before I finally
> understood the meaning:
>
> 44 /**
> 45 * Returns the {@linkplain com.sun.jdi.Type#name() name of
> the class}
> 46 * that has been unloaded. The returned string may not be a
> 47 * <a
> href="${docRoot}/java.base/java/lang/ClassLoader.html#binary-name">binary
> name</a>.
> 48 *
> 49 * @see Class#getName()
> 50 */
> 51 public String className();
> 52
> 53 /**
> 54 * Returns the {@linkplain com.sun.jdi.Type#signature() type
> signature of the class}
> 55 * that has been unloaded. The returned string may not be a
> type descriptor
> 56 * specified in JVMS {@jvms 4.3.2}.
> 57 *
> 58 * @see java.lang.invoke.TypeDescriptor#descriptorString()
> 59 */
> 60 public String classSignature();
>
> "may not be" through me off. I read it as "is not allowed not be".
> Your intent was "might not be", which might be a better choice of words.
>
I make it clearer as:
* Returns the {@linkplain com.sun.jdi.Type#signature() type
signature of the class}
* that has been unloaded. The result is of the same
* form as the string returned by {@link Class#descriptorString()}.
* If this class can be described nominally, the returned string is a
* type descriptor conforming to JVMS {@jvms 4.3.2}; otherwise, the
returned string
* is not a type descriptor.
Similar clarification is also made in Type::signature:
* Returns the type signature for this type. The result is of the same
* form as the string returned by {@link Class#descriptorString()}.
* The returned string is a type descriptor conforming to JVMS
{@jvms 4.3.2}
* if this type can be described nominally. Otherwise, the
returned string
* is not a type descriptor.
Mandy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200420/c49cf200/attachment.htm>
More information about the serviceability-dev
mailing list