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

2018-06-18 Thread Thomas Stüfe
Thanks, David! On Mon, Jun 18, 2018 at 8:21 AM, David Holmes wrote: > Test update looks good! > > Thanks, > David > > > On 14/06/2018 9:30 PM, Thomas Stüfe wrote: >> >> Hi all, >> >> hopefully last changes, with feedback added from Coleen and David. >> >> Only changes in the provided regression t

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

2018-06-17 Thread David Holmes
Test update looks good! Thanks, David On 14/06/2018 9:30 PM, Thomas Stüfe wrote: Hi all, hopefully last changes, with feedback added from Coleen and David. Only changes in the provided regression test: I run it now with -Dsun.reflect.noInflation to make the test independent from the reflectio

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

2018-06-14 Thread Thomas Stüfe
Thank you Serguei! On Thu, Jun 14, 2018 at 9:14 PM, serguei.spit...@oracle.com wrote: > Hi Thomas, > > The update looks good to me. > > Thanks, > Serguei > > > > On 6/14/18 04:30, Thomas Stüfe wrote: >> >> Hi all, >> >> hopefully last changes, with feedback added from Coleen and David. >> >> Only

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

2018-06-14 Thread serguei.spit...@oracle.com
Hi Thomas, The update looks good to me. Thanks, Serguei On 6/14/18 04:30, Thomas Stüfe wrote: Hi all, hopefully last changes, with feedback added from Coleen and David. Only changes in the provided regression test: I run it now with -Dsun.reflect.noInflation to make the test independent fro

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

2018-06-14 Thread Thomas Stüfe
Thanks Coleen! On Thu, Jun 14, 2018, 18:04 wrote: > > This was a good find of David's. Thank you for fixing the test. > Coleen > > On 6/14/18 7:30 AM, Thomas Stüfe wrote: > > Hi all, > > > > hopefully last changes, with feedback added from Coleen and David. > > > > Only changes in the provided

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

2018-06-14 Thread coleen . phillimore
This was a good find of David's.  Thank you for fixing the test. Coleen On 6/14/18 7:30 AM, Thomas Stüfe wrote: Hi all, hopefully last changes, with feedback added from Coleen and David. Only changes in the provided regression test: I run it now with -Dsun.reflect.noInflation to make the tes

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

2018-06-14 Thread Thomas Stüfe
Hi all, hopefully last changes, with feedback added from Coleen and David. Only changes in the provided regression test: I run it now with -Dsun.reflect.noInflation to make the test independent from the reflection inflation threshold. I also corrected the copyright dates. Delta: http://cr.openj

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

2018-06-13 Thread Thomas Stüfe
On Thu, Jun 14, 2018 at 7:11 AM, David Holmes 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 =

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

2018-06-13 Thread 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

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

2018-06-13 Thread Thomas Stüfe
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-

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

2018-06-12 Thread serguei.spit...@oracle.com
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 add

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

2018-06-12 Thread Thomas Stüfe
Could I have a second review, please? Thanks a lot, Thomas On Tue, Jun 12, 2018 at 7:36 AM, Thomas Stüfe wrote: > Ping. > > jdk-submit tests came back clean. > > Thanks, Thomas > > On Wed, Jun 6, 2018 at 6:05 PM, Thomas Stüfe wrote: >> Dear all, >> >> may I please have feedback and if possible

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

2018-06-11 Thread Thomas Stüfe
Ping. jdk-submit tests came back clean. Thanks, Thomas On Wed, Jun 6, 2018 at 6:05 PM, 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

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

2018-06-06 Thread Thomas Stüfe
On Wed, Jun 6, 2018 at 6:18 PM, Kirk Pepperdine wrote: > >> On Jun 6, 2018, at 6:05 PM, Thomas Stüfe wrote: >> >> Dear all, >> >> may I please have feedback and if possible reviews for this small addition: > > > I can see the need to visualize this but the output looks easily parsable so > it al

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

2018-06-06 Thread Kirk Pepperdine
> On Jun 6, 2018, at 6:05 PM, Thomas Stüfe wrote: > > Dear all, > > may I please have feedback and if possible reviews for this small addition: I can see the need to visualize this but the output looks easily parsable so it all looks good from my perspective. Not an official review — Kirk

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

2018-06-06 Thread Thomas Stüfe
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