Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

2020-08-24 Thread Mandy Chung
On 8/24/20 3:53 PM, Ioi Lam wrote: : However, I suspect most people won't do this, because the benefit is relatively small. Also, jlink doesn't support classpath apps, so you would need to figure out what to use for "--add-modules". In the worst case, the custom JDK would be over 100MB

Re: RFR(M): 8248188: [PATCH] Add HotSpotIntrinsicCandidate and API for Base64 decoding

2020-08-24 Thread Corey Ashford
Here's a revised webrev which includes a JMH benchmark for the decode operation. http://cr.openjdk.java.net/~mhorie/8248188/webrev.03/ The added benchmark tries to be "fair" in that it doesn't prefer a large buffer size, which would favor the intrinsic. It pseudo-randomly (but reproducibly)

Re: RFR: JDK-8251988: jpackage --runtime-image fails on mac when using JDK11 based runtime.

2020-08-24 Thread alexander . matveev
Hi Andy, Looks good. Thanks, Alexander On 8/24/20 4:53 PM, Alexey Semenyuk wrote: Looks good! - Alexey On 8/24/2020 6:23 PM, Andy Herrick wrote: On 8/24/2020 5:08 PM, Alexey Semenyuk wrote: Andy, I'd propose instead of checking if the app is bundled with proper runtime in the test

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-24 Thread Joe Wang
Thanks again Lance, Naoto!  And sorry for yet another review request!  I saw Jon's comment on the other doclint thread about replacing @exception with @throws. As it happens, we have 172 of them in the sax package as well. Please note the changes are all in the org/xml/sax packages:

Re: RFR: JDK-8251988: jpackage --runtime-image fails on mac when using JDK11 based runtime.

2020-08-24 Thread Alexey Semenyuk
Looks good! - Alexey On 8/24/2020 6:23 PM, Andy Herrick wrote: On 8/24/2020 5:08 PM, Alexey Semenyuk wrote: Andy, I'd propose instead of checking if the app is bundled with proper runtime in the test case, enforce the test case to always create runtime for the app with

Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

2020-08-24 Thread Ioi Lam
Hi Yumin, This looks good overall. Here are my comments: = 6065 size_t new_id = Atomic::add(, (size_t)1); 6066 jio_snprintf(addr_buf, 20, INTPTR_FORMAT, new_id); I think this should be SIZE_FORMAT =   65 class KlassFactory : AllStatic {   66  

Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

2020-08-24 Thread Ioi Lam
On 8/20/20 5:10 PM, Mandy Chung wrote: On 8/19/20 10:14 PM, Yumin Qi wrote: HI, Mandy   Thanks for the review, I took one day off yesterday so just got a detail look of your reply. On 8/19/20 1:30 PM, Mandy Chung wrote: On 8/17/20 12:37 PM, Yumin Qi wrote: Hi, Ioi   Thanks for

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-24 Thread Lance Andersen
Looks OK Joe > On Aug 24, 2020, at 5:44 PM, Joe Wang wrote: > > Hi all, adding Roger's comment for the make file to webrev_02 (the only > change to webrev_01 is Docs.gmk): > > http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_02/ > > Thanks, > Joe > > On 8/21/20 12:49 PM,

Re: RFR: JDK-8251988: jpackage --runtime-image fails on mac when using JDK11 based runtime.

2020-08-24 Thread Andy Herrick
On 8/24/2020 5:08 PM, Alexey Semenyuk wrote: Andy, I'd propose instead of checking if the app is bundled with proper runtime in the test case, enforce the test case to always create runtime for the app with JPackageCommand.ignoreDefaultRuntime(true) call: --- JPackageCommand cmd =

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-24 Thread naoto . sato
Still looks good. Naoto On 8/24/20 2:44 PM, Joe Wang wrote: Hi all,  adding Roger's comment for the make file to webrev_02 (the only change to webrev_01 is Docs.gmk): http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_02/ Thanks, Joe On 8/21/20 12:49 PM, naoto.s...@oracle.com wrote:

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-24 Thread Joe Wang
Hi all,  adding Roger's comment for the make file to webrev_02 (the only change to webrev_01 is Docs.gmk): http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_02/ Thanks, Joe On 8/21/20 12:49 PM, naoto.s...@oracle.com wrote: +1 Naoto On 8/21/20 12:24 PM, Lance Andersen wrote: Hi Joe,

Re: RFR 8251989: Hex encoder and decoder utility

2020-08-24 Thread Corey Ashford
Just as a general comment, an SIMD-based encode() intrinsic would be an easy performance upgrade for this Hex class. I'd guess you'd get a 5x-10x improvement, depending on the specific CPU/arch being used. That said, the intrinsic would do better on larger buffers, and I don't know what the

Re: RFR: JDK-8251988: jpackage --runtime-image fails on mac when using JDK11 based runtime.

2020-08-24 Thread Alexey Semenyuk
Andy, I'd propose instead of checking if the app is bundled with proper runtime in the test case, enforce the test case to always create runtime for the app with JPackageCommand.ignoreDefaultRuntime(true) call: --- JPackageCommand cmd = JPackageCommand.helloAppImage(   71

Re: RFR 8251989: Hex encoder and decoder utility

2020-08-24 Thread Paul Sandoz
Hi Roger, A nice minimal addition. I agree with Mark’s naming suggestion. - Use appropriate code convention for static field names. - The encoder comes with two encoding production signatures, that returning a String and that for encoding into a StringBuilder. The decoder comes with just one

Re: RFR: JDK-8251988: jpackage --runtime-image fails on mac when using JDK11 based runtime.

2020-08-24 Thread Andy Herrick
please review revised webrev at: http://cr.openjdk.java.net/~herrick/8251988/webrev.03/ jpackage.app-version set in cfg file, and jpackage.app-path now set from native code. New testcase in BasicTest to check that they are set. /Andy On 8/20/2020 7:36 PM, Alexey Semenyuk wrote: Andy, I'd

Undocumented behaviour in CompletableFuture

2020-08-24 Thread Sebastian Stenzel
Hi all, I just stumbled upon a strange behaviour and I'm not sure if it is intended this way or whether it's a bug. It is also possible I'm using the API incorrectly. Let me explain with an example: ``` var originalException = new Exception("foo");

Re: Review request for JDK-8252124: Restore Dynalink tests

2020-08-24 Thread sundararajan . athijegannathan
Yes. no need for separate webrev. -Sundar On 23/08/20 6:02 pm, Attila Szegedi wrote: Im happy to update the copyright years before merging my changes. I’ll not put up a separate webrev for that though, if that’s okay. Attila. On 2020. Aug 21., at 07:32, Sundararajan Athijegannathan