RFR(s): 8203343: VM.{metaspace|classloaders|classhierarchy...} jcmd should show invocation targets for Generated{Method|Constructor}AccessorImpl classes

[David Holmes]

Hi Thomas,

Just a nit in the test:

  54         // Do some reflection, enough times to hit the 
sun.reflect.inflationThreshold and force
   55         // generation of reflection accessor classes.
    ...
   59         for (int i = 0; i < 100; i ++) {

The default threshold is only 15. The test is already assuming it knows 
what the threshold is so it may as well only run 15 (16?) times; or else 
explicitly set the threshold and use a loop count to match. Or even 
better disable inflation and you don't need a loop at all.

I didn't look at anything else.

Cheers,
David

On 13/06/2018 8:32 PM, Thomas Stüfe wrote:
> Hi Serguei,
> 
> thank you for your review!
> 
> You are right, a regression test makes sense here. I wrote one. See
> updated webrev (only added the test, nothing else did change):
> 
> http://cr.openjdk.java.net/~stuefe/webrevs/8203343-VM.metaspace-show-reflection-invocation-targets/webrev.01/webrev/
> 
> I am re-running the jdk-submit tests.
> 
> Thanks & Best Regards, Thomas
> 
> On Tue, Jun 12, 2018 at 11:01 PM, serguei.spitsyn at oracle.com
> <serguei.spitsyn at oracle.com> wrote:
>> Hi Thomas,
>>
>> It looks good to me.
>> But I'm not an expert in the area of Generated Accessors.
>>
>> How was this tested?
>> Does it make sense to add a unit test for this?
>>
>> Thanks,
>> Serguei
>>
>>
>>
>> On 6/6/18 09:05, Thomas Stüfe wrote:
>>>
>>> Dear all,
>>>
>>> may I please have feedback and if possible reviews for this small
>>> addition:
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8203343
>>> Webrev:
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203343-VM.metaspace-show-reflection-invocation-targets/webrev.00/webrev/
>>>
>>> (Note: this patch goes atop of
>>> https://bugs.openjdk.java.net/browse/JDK-8203682, which is currently
>>> up for RFR).
>>>
>>> ---
>>>
>>> When analyzing situations involving a lot of reflection, one often
>>> stares at walls of "GeneratedXXXAccessorXXX" classes. These names are
>>> generated and not at all helpful in analyzing the problem (e.g. which
>>> component in a server node does this much reflection and hence uses so
>>> much metaspace).
>>>
>>> This patch adds the ability to print out invocation targets
>>> additionally to class names if the class is a generated accessor - for
>>> now this ability has been added to VM.metaspace, VM.classloaders and
>>> VM.class_hierarchy.
>>>
>>> Example output from "VM.class_hierarchy":
>>>
>>> <snip>
>>> |--jdk.internal.reflect.MagicAccessorImpl/null
>>> <snip>
>>> |  |--jdk.internal.reflect.ConstructorAccessorImpl/null
>>> |  |
>>> |--jdk.internal.reflect.GeneratedConstructorAccessor18/0x00007f9ee8350c10
>>> (invokes:
>>> org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessorRegistrar::<init>
>>> ()V)
>>> |  |
>>> |--jdk.internal.reflect.GeneratedConstructorAccessor17/0x00007f9ee8349c00
>>> (invokes:
>>> org/springframework/boot/context/properties/EnableConfigurationPropertiesImportSelector$ConfigurationPropertiesBeanRegistrar::<init>
>>> ()V)
>>> <snip>
>>> |  |--jdk.internal.reflect.MethodAccessorImpl/null
>>> |  |  |--jdk.internal.reflect.GeneratedMethodAccessor23/0x00007f9ec8329b60
>>> (invokes: org/apache/tomcat/util/modeler/AttributeInfo::setIs (Z)V)
>>> |  |  |--jdk.internal.reflect.GeneratedMethodAccessor22/0x00007f9ee831bc70
>>> (invokes:
>>> org/springframework/boot/autoconfigure/SpringBootApplication::exclude
>>> ()[Ljava/lang/Class;)
>>> <snip>
>>>
>>> See here more examples:
>>>
>>> "VM.class_hierarchy"
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203343-VM.metaspace-show-reflection-invocation-targets/example-VM.class_hierarchy.txt
>>>
>>> "VM.classloaders show-classes"
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203343-VM.metaspace-show-reflection-invocation-targets/example-VM.classloaders.txt
>>>
>>>
>>> ----
>>>
>>> Note: I am not sure if this is a fit for hotspot-runtime or
>>> serviceability. Sorry for crossposting.
>>>
>>> Thank you,
>>>
>>> Thomas
>>
>>