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

2020-03-27 Thread Chris Plummer
Hi Mandy, A couple of very minor nits in the jvmtiRedefineClasses.cpp comments:  153 // classes for primitives, arrays, hidden and vm unsafe anonymous classes  154 // cannot be redefined.  Check here so following code can assume these classes  155 // are InstanceKlass.  156 if

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

2020-03-27 Thread Dean Long
I looked at the AOT, C2, and JVMCI changes and I didn't find any issues. dl 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 (intrinsifica

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

2020-03-27 Thread Paul Sandoz
+1 Paul. > On Mar 27, 2020, at 3:22 PM, Mandy Chung wrote: > > Hi Paul, > > This is the delta incorporating your comment: > > http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-psandoz/ > >

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

2020-03-27 Thread David Holmes
Hi Mandy, On 28/03/2020 9:46 am, Mandy Chung wrote: On 3/27/20 4:01 PM, David Holmes wrote: Hi Mandy, On 28/03/2020 8:29 am, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release  >= 11. However this is about javac --release 14 and the com

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

2020-03-27 Thread Mandy Chung
On 3/27/20 4:01 PM, David Holmes wrote: Hi Mandy, On 28/03/2020 8:29 am, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release  >= 11. However this is about javac --release 14 and the compiled classes may run on JDK 14 that lambda and strin

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

2020-03-27 Thread Remi Forax
- Mail original - > De: "David Holmes" > À: "mandy chung" , "Vicente Romero" > , "jan lahoda" > > Cc: "serviceability-dev" , "hotspot-dev" > , > "core-libs-dev" , "valhalla-dev" > > Envoyé: Samedi 28 Mars 2020 00:01:58 > Objet: Re: Review Request: 8238358: Implementation of JEP 371: H

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

2020-03-27 Thread David Holmes
Hi Mandy, On 28/03/2020 8:29 am, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release >= 11. However this is about javac --release 14 and the compiled classes may run on JDK 14 that lambda and string concat spin classes that are not nestmat

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

2020-03-27 Thread Vicente Romero
Hi Mandy, On 3/27/20 6:29 PM, Mandy Chung wrote: Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release >= 11. I was not suggesting the use of `hasNestmateAccess` but to follow the same approach which is adding a new method at class `Target` to check if the new

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

2020-03-27 Thread Mandy Chung
Hi Vicente, hasNestmateAccess is about VM supports static nestmates on JDK release >= 11. However this is about javac --release 14 and the compiled classes may run on JDK 14 that lambda and string concat spin classes that are not nestmates. I have a patch with Jan's help: http://cr.openjdk

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

2020-03-27 Thread Mandy Chung
Hi Paul, This is the delta incorporating your comment: http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-psandoz/ This patch also took Alex's comment to make it clear that the hidden class is the lookup class of the returned Lookup object and drops the sentence

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

2020-03-27 Thread Vicente Romero
Hi Mandy, The patch for nestmates [1] could be used as a reference. There a new method was added to class `com.sun.tools.javac.jvm.Target`, named: `hasNestmateAccess` which checks if a target is ready for nestmates or not. I think that you can follow a similar approach here. Thanks, Vicente

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

2020-03-27 Thread Mandy Chung
On 3/27/20 11:59 AM, Paul Sandoz wrote: Hi Mandy, Very thorough, bravo! Thanks. Minor suggestions below. Paul. MethodHandleNatives.java — 142 143 /** 144 * Flags for Lookup.ClassOptions 145 */ 146 static final int 147 NESTMATE_CLA

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

2020-03-27 Thread Paul Sandoz
Hi Mandy, Very thorough, bravo! Minor suggestions below. Paul. MethodHandleNatives.java — 142 143 /** 144 * Flags for Lookup.ClassOptions 145 */ 146 static final int 147 NESTMATE_CLASS= 0x0001, 148 HIDDEN_CLASS

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

2020-03-27 Thread Aleksey Shipilev
On 3/27/20 6:15 PM, Chris Plummer wrote: > Yeah, I think given that it fixes a broken build it should be fine to > push right away. jdk-submit came clean, pushed. -- Thanks, -Aleksey signature.asc Description: OpenPGP digital signature

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

2020-03-27 Thread Chris Plummer
Yeah, I think given that it fixes a broken build it should be fine to push right away. Chris On 3/27/20 10:10 AM, Aleksey Shipilev wrote: Thanks! Trivial, right? If so, I'll push as soon as jdk-submit clears. -Aleksey On 3/27/20 6:08 PM, Chris Plummer wrote: +1 Chris On 3/27/20 10:03 AM,

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

2020-03-27 Thread Aleksey Shipilev
Thanks! Trivial, right? If so, I'll push as soon as jdk-submit clears. -Aleksey On 3/27/20 6:08 PM, Chris Plummer wrote: > +1 > > Chris > > On 3/27/20 10:03 AM, Roman Kennke wrote: >> Looks good to me, thanks! >> >> Roman >> >>> Bug: >>>https://bugs.openjdk.java.net/browse/JDK-8241750 >>>

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

2020-03-27 Thread Chris Plummer
+1 Chris On 3/27/20 10:03 AM, Roman Kennke wrote: Looks good to me, thanks! Roman Bug: https://bugs.openjdk.java.net/browse/JDK-8241750 Fix: diff -r fef47d126675 src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c --- a/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c Fri Mar

Re: RFR (XS) 8241750: x86_32 build failure after JDK-8227269

2020-03-27 Thread Roman Kennke
Looks good to me, thanks! Roman > Bug: > https://bugs.openjdk.java.net/browse/JDK-8241750 > > Fix: > > diff -r fef47d126675 src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c > --- a/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c Fri Mar 27 > 15:33:24 2020 +0100 > +++ b/src/jdk.

RFR (XS) 8241750: x86_32 build failure after JDK-8227269

2020-03-27 Thread Aleksey Shipilev
Bug: https://bugs.openjdk.java.net/browse/JDK-8241750 Fix: diff -r fef47d126675 src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c --- a/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c Fri Mar 27 15:33:24 2020 +0100 +++ b/src/jdk.jdwp.agent/share/native/libjdwp/classTrack.c Fr

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

2020-03-27 Thread Mandy Chung
Hi Jan, Good point.  The javac change only applies to JDK 15 and later and the lambda proxy class is not a nestmate when running on JDK 14 or earlier. I probably need the help from langtools team to fix this.  I'll give it a try. Mandy On 3/27/20 4:31 AM, Jan Lahoda wrote: Hi Mandy, Rega

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

2020-03-27 Thread forax
> De: "mandy chung" > À: "Remi Forax" > Cc: "valhalla-dev" , "core-libs-dev" > , "serviceability-dev" > , "hotspot-dev" > > Envoyé: Vendredi 27 Mars 2020 16:50:55 > Objet: Re: Review Request: 8238358: Implementation of JEP 371: Hidden Classes > On 3/27/20 5:00 AM, Remi Forax wrote: >> Hi Mandy

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

2020-03-27 Thread Mandy Chung
On 3/27/20 5:00 AM, Remi Forax wrote: Hi Mandy, in ReflectionFactory, why in the case of a constructor the check to the anonymous class is removed ? Good catch.  Fixed in BytecodeGenerator, the comment "// bootstrapping issue if using condy" can be promoted on top of clinit, because i ask my

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

2020-03-27 Thread Remi Forax
Hi Mandy, in ReflectionFactory, why in the case of a constructor the check to the anonymous class is removed ? in BytecodeGenerator, the comment "// bootstrapping issue if using condy" can be promoted on top of clinit, because i ask myself the same question seeing a static block was generated i

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

2020-03-27 Thread Jan Lahoda
Hi Mandy, Regarding the javac changes - should those be switched on/off depending the Target? Or, if one compiles with e.g. --release 14, will the newly generated output still work on JDK 14? Jan On 27. 03. 20 0:57, Mandy Chung wrote: Please review the implementation of JEP 371: Hidden Class

Re: RFR: 8241585: Remove unused _recursion_counter facility from PerfTraceTime

2020-03-27 Thread Claes Redestad
On 2020-03-27 03:18, David Holmes wrote: Yeah they confuse me. Which makes it hard to see what impact your changes may have. This patch removes some internal, unused code on the JVM end that is not observable via jstat / jvmstat. I'm happy if serviceability can weigh in though. The other

Re: RFR: 8240956: SEGV in DwarfParser::process_dwarf after JDK-8234624

2020-03-27 Thread Yasumasa Suenaga
retest is nice. Thanks Kevin On 27/03/2020 02:49, Yasumasa Suenaga wrote: All tests on submit repo has been passed. (mach5-one-ysuenaga-JDK-8240956-3-20200327-0003-9753265) Yasumasa On 2020/03/27 9:07, Yasumasa Suenaga wrote: Thanks Kevin and Serguei! and sorry for my English... I uplo

Re: RFR: 8240956: SEGV in DwarfParser::process_dwarf after JDK-8234624

2020-03-27 Thread Kevin Walls
tests on submit repo has been passed. (mach5-one-ysuenaga-JDK-8240956-3-20200327-0003-9753265) Yasumasa On 2020/03/27 9:07, Yasumasa Suenaga wrote: Thanks Kevin and Serguei! and sorry for my English... I uploaded new webrev:    http://cr.openjdk.java.net/~ysuenaga/JDK-8240956/webrev.05/ Diff fr