Re: CharsetEncoder.maxBytesPerChar()

2019-09-20 Thread naoto . sato
Hi Mark, Thank you for the crystal clear explanation. I will go ahead and clarify the method description. Naoto On 9/20/19 3:03 PM, mark.reinh...@oracle.com wrote: 2019/9/20 13:25:38 -0700, naoto.s...@oracle.com: I am looking at the following bug: https://bugs.openjdk.java.net/browse/JDK-8

Re: RFR: JDK-8230651: Use version string from main module

2019-09-20 Thread Alexey Semenyuk
Looks good. Unfortunately the new test helper classes don't support --module option yet and the test can not be implemented based on them. - Alexey On 9/20/2019 5:54 PM, Alexander Matveev wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch bran

Re: CharsetEncoder.maxBytesPerChar()

2019-09-20 Thread mark . reinhold
2019/9/20 13:25:38 -0700, naoto.s...@oracle.com: > I am looking at the following bug: > > https://bugs.openjdk.java.net/browse/JDK-8230531 > > and hoping someone who is familiar with the encoder will clear things > out. As in the bug report, the method description reads: > > -- > Returns the ma

Re: RFR 8221623 : Add StackWalker micro benchmarks to jdk repo

2019-09-20 Thread Brent Christian
Hi, Claes On 9/19/19 1:40 PM, Claes Redestad wrote: FYI, you can control the number of forks etc run by make test: make test TEST="micro:java.lang.StackWalkBench" MICRO="FORK=1;WARMUP_ITER=2;OPTIONS=-p depth=128" (documented in doc/testing.md|html) Neat, thanks! Still, I agree it might

RFR: JDK-8230651: Use version string from main module

2019-09-20 Thread Alexander Matveev
Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). - Version from main module (if exist) will be used as --app-version if --app-version is not specified. [1] https://bugs.openjdk.java.net/browse/JDK-8

Re: RFR 8231314: java.time serialization warning cleanup

2019-09-20 Thread Joe Darcy
Hello, Looks fine to me too. As Naoto observed, the patch is on top of a changeset with annotations I haven't pushed yet; I think it would be fine if this changeset were pushed first and I'll adjust mine accordingly. Thanks, -Joe On 9/20/2019 2:03 PM, naoto.s...@oracle.com wrote: Hi Roger,

Re: RFR: JDK-8231277: Adjust Linux application image layout

2019-09-20 Thread Alexander Matveev
Looks good. On 9/20/2019 4:57 AM, Alexey Semenyuk wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). This fix: - directory layout of Linux app image adjusted to better comply with Linux FSH.

Re: RFR 8231314: java.time serialization warning cleanup

2019-09-20 Thread naoto . sato
Hi Roger, Looks good to me. There are some removed "@SuppressWarnings("serial")" which don't exist in the current JDK14 code base, e.g., line 115 of java/time/Ser.java, but I assumed they can be safely ignored. Naoto On 9/20/19 11:51 AM, Roger Riggs wrote: Please review code cleanup that wi

Re: RFR: 8231186: Replace html tag foo with javadoc tag {@code foo} in java.base

2019-09-20 Thread Brent Christian
Hi, Julia On 9/20/19 3:22 AM, Julia Boes wrote: Hi, Thanks for noticing the glitch in the sdiffs, Naoto and Brent. I see that there is indeed an issue with the webrev script and I'm looking into a workaround. The following classes are affected: src/java.base/share/classes/java/lang/Securit

CharsetEncoder.maxBytesPerChar()

2019-09-20 Thread naoto . sato
Hello, I am looking at the following bug: https://bugs.openjdk.java.net/browse/JDK-8230531 and hoping someone who is familiar with the encoder will clear things out. As in the bug report, the method description reads: -- Returns the maximum number of bytes that will be produced for each cha

RFR 8231314: java.time serialization warning cleanup

2019-09-20 Thread Roger Riggs
Please review code cleanup that will remove the need to suppress soon to be introduced warnings [1] about static typing of serialization related fields. A few of the implementation Ser classes that serialize java.time types are affected. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-warn-s

Re: RFR [14/java.xml] 8231083: Clarify SAX documentation

2019-09-20 Thread Joe Wang
Thanks Alan! Joe On 9/20/19 10:03 AM, Alan Bateman wrote: On 20/09/2019 17:46, Joe Wang wrote: Thanks Alan! Here's the updated webrev after changing the apiNote as you suggested: http://cr.openjdk.java.net/~joehw/jdk14/8231083/webrev02/index.html Looks good.

Re: RFR [14/java.xml] 8231083: Clarify SAX documentation

2019-09-20 Thread Alan Bateman
On 20/09/2019 17:46, Joe Wang wrote: Thanks Alan! Here's the updated webrev after changing the apiNote as you suggested: http://cr.openjdk.java.net/~joehw/jdk14/8231083/webrev02/index.html Looks good.

Re: RFR [14/java.xml] 8231083: Clarify SAX documentation

2019-09-20 Thread Joe Wang
Thanks Alan! Here's the updated webrev after changing the apiNote as you suggested: http://cr.openjdk.java.net/~joehw/jdk14/8231083/webrev02/index.html -Joe On 9/20/19 9:07 AM, Alan Bateman wrote: On 20/09/2019 06:15, Joe Wang wrote: Thanks Lance! Yes, saw them typos :-)  Also removed the ex

Re: RFR [14/java.xml] 8231083: Clarify SAX documentation

2019-09-20 Thread Alan Bateman
On 20/09/2019 06:15, Joe Wang wrote: Thanks Lance! Yes, saw them typos :-)  Also removed the extra space in apiNote. Updated webrev below, with removing the text in the javadoc instead of moving to the header. http://cr.openjdk.java.net/~joehw/jdk14/8231083/webrev/index.html Just a minor comm

Re: RFR [14/java.xml] 8231083: Clarify SAX documentation

2019-09-20 Thread Joe Wang
Thanks Lance!  It's a cleanup on top of cleanup :-) On 9/20/19 4:13 AM, Lance Andersen wrote: Round 2  even looks cleaner :-) On Sep 20, 2019, at 1:15 AM, Joe Wang > wrote: Thanks Lance! Yes, saw them typos :-)  Also removed the extra space in apiNote. Update

Re: RFR: 8231186: Replace html tag foo with javadoc tag {@code foo} in java.base

2019-09-20 Thread Julia Boes
Hi Roger, I ran webrev.ksh. The issue is reported here: https://bugs.openjdk.java.net/browse/CODETOOLS-7902519 (knowing that it's not a priority with git-webrev on the horizon). Regards, Julia On 20/09/2019 14:05, Roger Riggs wrote: Hi Julia, Did youuse webrev.ksh or the git-webrev availa

Re: JDK 14 RFR of JDK-8199424: consider removing ObjectInputStream and ObjectOutputStream native methods

2019-09-20 Thread Brian Burkhalter
+1 Brian > On Sep 20, 2019, at 6:38 AM, Roger Riggs wrote: > > Looks good, thanks for the cleanup. > > Roger > > > On 9/19/19 11:02 PM, Joe Darcy wrote: >> Hi Brian, >> >> Now including ObjectInputStream changes: >> >> http://cr.openjdk.java.net/~darcy/8199424.1/ >> >> Serialization

Re: RFR(s): 8228580: DnsClient TCP socket timeout

2019-09-20 Thread Milan Mimica
Pavel, Here it is: http://cr.openjdk.java.net/~mmimica/8228580/webrev.04/ I don't see the test is run twice when I execute "make test TEST=jtreg:test/jdk/com/sun/jndi/dns/ConfigTests/TcpTimeout.java". Am I missing something? On Thu, 19 Sep 2019 at 13:16, Pavel Rappo wrote: > > Milan, > > This lo

RE: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

2019-09-20 Thread Lindenmaier, Goetz
Hi Ralf, thanks for looking at this code and this thorough review! New webrevs: http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/18/ http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/18-incremental/ Find my comments inline: > In javaClasses.cpp: > > > #define CLASS_FIELDS_DO(ma

Re: JDK 14 RFR of JDK-8231202: Suppress warnings on non-serializable non-transient instance fields in serializable classes

2019-09-20 Thread Roger Riggs
Hi Rémi, On 9/19/19 4:45 PM, Remi Forax wrote: Hi Joe, hi Roger, these changes are not fine, SerializedLambda is a public class with a public constructor so it's an incompatible change to replace Object[] by Serializable[], moreover for an array, all values can be Serializable but the array

Re: JDK 14 RFR of JDK-8199424: consider removing ObjectInputStream and ObjectOutputStream native methods

2019-09-20 Thread Roger Riggs
Hi Joe, Looks good, thanks for the cleanup. Roger On 9/19/19 11:02 PM, Joe Darcy wrote: Hi Brian, Now including ObjectInputStream changes: http://cr.openjdk.java.net/~darcy/8199424.1/ Serialization regression tests still all pass. Thanks, -Joe On 9/19/2019 5:19 PM, Brian Burkhalter

Re: RFR: 8231186: Replace html tag foo with javadoc tag {@code foo} in java.base

2019-09-20 Thread Roger Riggs
Hi Julia, Did youuse webrev.ksh or the git-webrev available with Skara? If the later, then it can/should be reported and fixed. Roger On 9/20/19 6:22 AM, Julia Boes wrote: Hi, Thanks for noticing the glitch in the sdiffs, Naoto and Brent. I see that there is indeed an issue with the webrev

RFR: JDK-8231277: Adjust Linux application image layout

2019-09-20 Thread Alexey Semenyuk
Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). This fix: - directory layout of Linux app image adjusted to better comply with Linux FSH. - some unused code clean up. - added support to run JUnit

Re: RFR [14/java.xml] 8231083: Clarify SAX documentation

2019-09-20 Thread Lance Andersen
Round 2 even looks cleaner :-) > On Sep 20, 2019, at 1:15 AM, Joe Wang > wrote: > > Thanks Lance! > > Yes, saw them typos :-) Also removed the extra space in apiNote. > > Updated webrev below, with removing the text in the javadoc instead of moving > to the h

RE: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

2019-09-20 Thread Schmelter, Ralf
Hi Goetz, here are my review remarks: In javaClasses.cpp: > #define CLASS_FIELDS_DO(macro) \ > macro(classRedefinedCount_offset, k, "classRedefinedCount", int_signature, > false); \ > macro(_class_loader_offset, k, "classLoader", > classloader_signature, false); \ The

Re: RFR: 8231186: Replace html tag foo with javadoc tag {@code foo} in java.base

2019-09-20 Thread Julia Boes
Hi, Thanks for noticing the glitch in the sdiffs, Naoto and Brent. I see that there is indeed an issue with the webrev script and I'm looking into a workaround. The following classes are affected: src/java.base/share/classes/java/lang/SecurityManager.java src/java.base/share/classes/java/ut

Re: [RFR] 8231171: remove remaining sun.java.launcher.pid references - was RE: sun.java.launcher.pid property usage

2019-09-20 Thread David Holmes
That looks fine to me. Thanks, David On 20/09/2019 7:14 pm, Baesken, Matthias wrote: Hi David , I adjusted the test ( test/jdk/tools/launcher/TestSpecialArgs.java ) and removed the comments in os_bsd.cpp (suggested by you) . New webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8

RE: [RFR] 8231171: remove remaining sun.java.launcher.pid references - was RE: sun.java.launcher.pid property usage

2019-09-20 Thread Baesken, Matthias
Hi David , I adjusted the test ( test/jdk/tools/launcher/TestSpecialArgs.java ) and removed the comments in os_bsd.cpp (suggested by you) . New webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8231171.1/ Best regards, Matthias > Hi Matthias, > > On 20/09/2019 5:03 pm, Baesken,

Re: [RFR] 8231171: remove remaining sun.java.launcher.pid references - was RE: sun.java.launcher.pid property usage

2019-09-20 Thread David Holmes
Hi Matthias, On 20/09/2019 5:03 pm, Baesken, Matthias wrote: Hello, looks like on Linux there is a special check in TestSpecialArgs.java for launcherPidString = "launcher.pid=" that fails after 8231171 . Should I adjust the test ? Or keep the setting in the launcher on Lin

RE: [RFR] 8231171: remove remaining sun.java.launcher.pid references - was RE: sun.java.launcher.pid property usage

2019-09-20 Thread Baesken, Matthias
Hello, looks like on Linux there is a special check in TestSpecialArgs.java for launcherPidString = "launcher.pid=" that fails after 8231171 . Should I adjust the test ? Or keep the setting in the launcher on Linux ? tools/launcher/TestSpecialArgs.java /*