On Thu, Jun 14, 2018 at 7:11 AM, David Holmes <david.hol...@oracle.com> wrote:
> 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.
>
Yes, you are right, that would be cleaner.
I was not sure how to set -Ds.r.noInflation, but I will check if this
can be done.
Thanks, Thomas
> 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.spit...@oracle.com
>> <serguei.spit...@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
>>>
>>>
>>>
>
