On 4/18/20 12:47 AM, Chris Plummer wrote:
It's a link to
https://download.java.net/java/early_access/jdk15/docs/specs/jni/types.html#type-signatures.
This is how the current JVM TI spec defines.
Since it looks like this reference was present before your changes, I
guess it's ok to leave
Hi Mandy,
Comments inline.
On 4/17/20 4:52 PM, Mandy Chung wrote:
On 4/17/20 3:51 PM, Chris Plummer wrote:
Hi Mandy,
Thanks for updating the svc specs. Some comments below:
In the JDWP spec update, you changed "JNI signature" to "type
signature" in one place, but left it as "JNI
On 4/17/20 9:37 PM, serguei.spit...@oracle.com wrote:
On 4/17/20 16:52, Mandy Chung wrote:
On 4/17/20 3:51 PM, Chris Plummer wrote:
Hi Mandy,
Thanks for updating the svc specs. Some comments below:
In the JDWP spec update, you changed "JNI signature" to "type
signature" in one place, but
On 4/17/20 16:52, Mandy Chung wrote:
On 4/17/20 3:51 PM, Chris Plummer wrote:
Hi Mandy,
Thanks for updating the svc specs. Some comments below:
In the JDWP spec update, you changed "JNI signature" to "type
signature" in one place, but left it as "JNI signature" everywhere
else. Should
On 4/17/20 3:51 PM, Chris Plummer wrote:
Hi Mandy,
Thanks for updating the svc specs. Some comments below:
In the JDWP spec update, you changed "JNI signature" to "type
signature" in one place, but left it as "JNI signature" everywhere
else. Should they all be changed?
JDWP signature
On 4/16/20 9:45 AM, Mandy Chung wrote:
On 4/14/20 11:51 AM, Paul Sandoz wrote:
Looks good to me (not familiar with all the code areas.
Minor suggestion:
MethodHandles.java
1811 * ASCII periods. For the instance of {@link
java.lang.Class} representing {@code C}:
1812 *
On 4/16/20 11:42 AM, serguei.spit...@oracle.com wrote:
Hi Mandy,
I have a couple of minor comments on the Serviceability spec update.
On 4/14/20 11:51 AM, Paul Sandoz wrote:
Looks good to me (not familiar with all the code areas.
Minor suggestion:
MethodHandles.java
1811 * ASCII periods. For the instance of {@link java.lang.Class}
representing {@code C}:
1812 *
1813 * {@link Class#getName()}
Looks good to me (not familiar with all the code areas.
Minor suggestion:
MethodHandles.java
1811 * ASCII periods. For the instance of {@link java.lang.Class}
representing {@code C}:
1812 *
1813 * {@link Class#getName()} returns the string {@code GN + "/"
+ },
Please review the delta patch:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.06-delta/
Incremental specdiff:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/specdiff-inc/overview-summary.html
This is to follow a discussion [1] on
On 4/6/20 11:54, Mandy Chung wrote:
On 4/6/20 9:56 AM, serguei.spit...@oracle.com wrote:
The suggested fix is:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/valhalla-jdi-regression-8242166.1/
This patch looks okay. I'll include in my local patch.
On 4/6/20 11:00 AM, Chris Plummer
On 4/6/20 9:56 AM, serguei.spit...@oracle.com wrote:
The suggested fix is:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/valhalla-jdi-regression-8242166.1/
This patch looks okay. I'll include in my local patch.
On 4/6/20 11:00 AM, Chris Plummer wrote:
I think that's fine but I don't
Hi Peter,
On 4/4/20 3:58 AM, Peter Levart wrote:
Here I think, you are not quite right. First I need to clarify that we
are talking about the case where the method reference in above example
is not converted to lambda by javac, so the proxy class needs to
invoke the superclass method
On 4/4/20 9:16 AM, Peter Levart wrote:
Hi Mandy,
Just another observation of the code in InnerClassLambdaMetafactory...
For the useImplMethodHandle case you generate the protectedImplMethod
static field to hold the MH and a static setter method:
mv =
Hi Mandy,
Just another observation of the code in InnerClassLambdaMetafactory...
For the useImplMethodHandle case you generate the protectedImplMethod
static field to hold the MH and a static setter method:
mv = cw.visitMethod(ACC_PRIVATE + ACC_STATIC,
Hi Mandy,
On 4/3/20 11:32 PM, Mandy Chung wrote:
Hi Peter,
I added a JBS comment [1] to describe this special case trying to put
the story together (let me know if it needs more explanation). More
comment inline below.
Thanks, this helps in establishing the historical context.
On
Hi Peter,
I added a JBS comment [1] to describe this special case trying to put
the story together (let me know if it needs more explanation). More
comment inline below.
On 4/3/20 4:40 AM, Peter Levart wrote:
Ok, I think I found one such use-case. In the following example:
package test;
Ok, I think I found one such use-case. In the following example:
package test;
public class LambdaTest {
protected void m() {
}
}
package test.sub;
public class LambdaTestSub extends test.LambdaTest {
public void test() {
Runnable r = this::m;
r.run();
}
}
Hi Mandy,
Good work.
I'm trying to find out which language use-case is covered by the
InnerClassLambdaMetafactory needing to inject method handle into the
generated proxy class to be invoked instead of proxy class directly
invoking the method:
useImplMethodHandle =
Hi Mandy,
Update seems fine.
Thanks,
David
On 3/04/2020 4:56 am, Mandy Chung wrote:
Hi David,
Thank you for checking thoroughly. I now grep on src/hotspot and clean
all of them.
Updated delta webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04-delta/
On
On 4/2/2020 2:56 PM, Mandy Chung wrote:
Hi David,
Thank you for checking thoroughly. I now grep on src/hotspot and
clean all of them.
Updated delta webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04-delta/
Patch looks good Mandy.
Lois
On 4/1/20
Hi David,
Thank you for checking thoroughly. I now grep on src/hotspot and clean
all of them.
Updated delta webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04-delta/
On 4/1/20 11:21 PM, David Holmes wrote:
Hi Mandy,
On 2/04/2020 3:17 pm, Mandy Chung
Hi Mandy,
On 2/04/2020 3:17 pm, Mandy Chung wrote:
Hi David,
Thanks for the edits to the comments. "weak hidden" were missed to be
changed to "non-strong hidden". Here is the patch fixing the comments.
There are 33 cases of "weak hidden" in the patch I reviewed and also
some "hidden
Hi David,
Thanks for the edits to the comments. "weak hidden" were missed to be
changed to "non-strong hidden". Here is the patch fixing the comments.
diff --git a/src/hotspot/share/ci/ciField.cpp
b/src/hotspot/share/ci/ciField.cpp
--- a/src/hotspot/share/ci/ciField.cpp
+++
Hi Mandy,
On 1/04/2020 1:01 pm, Mandy Chung wrote:
Thanks to the review feedbacks.
Updated webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04/
I've had a good look through all the hotspot related changes. All looks
good.
A few minor comments:
On 4/1/20 7:26 AM, Alan Bateman wrote:
Maybe I missed it going by, but why is the isHidden check in the field
offset methods on sun.misc.Unsafe rather than the internal Unsafe?
The reflection and VarHandle implementation uses
jdk.internal.misc.Unsafe to do field access (both read and
On 31/03/2020 20:25, Mandy Chung wrote:
Alex's feedback: rename isHiddenClass to isHidden as it can be a
hidden class or interface.
`isLocalClass` and `sAnonymousClass` are correct because the Java
language only has local classes and anon classes, not local interfaces
or anon.
Thanks to the review feedbacks.
Updated webrev:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.04/
Delta between webrev.03 and webrev.04:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta/
Summary of changes is:
1. Update javac to
Alex's feedback: rename isHiddenClass to isHidden as it can be a hidden
class or interface.
`isLocalClass` and `sAnonymousClass` are correct because the Java
language only has local classes and anon classes, not local interfaces
or anon. interfaces. `isHidden` is like `isSynthetic`, it
This patch addresses Joe's feedback on the CSR [1]:
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03-delta-jdarcy/
Specifically, it adds to the class specification of java.lang.Class to
describe how the relevant methods behave for hidden classes. In
addition, use
This is the patch to keep the JDK 14 behavior if target release to 14
(thanks to Jan for helping making change in javac to get the tests working)
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8171335/webrev-javac-target-release-14/
Mandy
On 3/27/20 9:29 AM, Mandy Chung wrote:
Hi Jan,
On 3/30/20 02:30, serguei.spit...@oracle.com wrote:
Hi Mandy,
I have just one comment so far.
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp.frames.html
356 void add_classes(LoadedClassInfo*
On 3/30/20 5:54 AM, David Holmes wrote:
Sorry to jump in on this but it caught my eye though I may be missing
a larger context ...
On 30/03/2020 7:30 pm, serguei.spit...@oracle.com wrote:
Hi Mandy,
I have just one comment so far.
Sorry to jump in on this but it caught my eye though I may be missing a
larger context ...
On 30/03/2020 7:30 pm, serguei.spit...@oracle.com wrote:
Hi Mandy,
I have just one comment so far.
Hi Mandy,
I have just one comment so far.
http://cr.openjdk.java.net/~mchung/valhalla/webrevs/hidden-classes/webrev.03/src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp.frames.html
356 void add_classes(LoadedClassInfo* first_class, int num_classes,
bool has_class_mirror_holder) {
Hi Mandy and Chris,
On 3/29/20 19:17, Mandy Chung wrote:
On 3/27/20 8:51 PM, Chris Plummer wrote:
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
On 3/27/20 8:51 PM, Chris Plummer wrote:
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
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
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
+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/
>
>
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
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
- Mail original -
> De: "David Holmes"
> À: "mandy chung" , "Vicente Romero"
> , "jan lahoda"
>
> Cc: "serviceability-dev" , "hotspot-dev"
> ,
> "core-libs-dev" , "valhalla-dev"
>
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
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
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:
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
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
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
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
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,
> 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: Imple
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
ot;
>
> Envoyé: Vendredi 27 Mars 2020 00:57:39
> Objet: Review Request: 8238358: Implementation of JEP 371: Hidden Classes
> 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 ja
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
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 (intrinsification of Class::isHiddenClass),
JFR, JDI, and jcmd. CSR [1]has been reviewed and is in the finalized
state (see
56 matches
Mail list logo
