Re: Use single character replace variant in Resources.toPackageName(String)

Hi Ivan, I was indeed running with JDK 11 instead of JDK 13 (sorry), but like you I still think it's worthwhile. >BUGURL: https://bugs.openjdk.java.net/browse/JDK-8236705 >WEBREV: http://cr.openjdk.java.net/~igerasim/8236705/00/webrev/ > If we agree on the content of the fix, I can

Re: Use single character replace variant in Resources.toPackageName(String)

On 07/01/2020 07:09, Ivan Gerasimov wrote: So, I filed a Jira bug: BUGURL: https://bugs.openjdk.java.net/browse/JDK-8236705 WEBREV: http://cr.openjdk.java.net/~igerasim/8236705/00/webrev/ I added these source files in JDK 9, it was just an oversight that I didn't use the single char replace.

Re: Use single character replace variant in Resources.toPackageName(String)

So, I filed a Jira bug: BUGURL: https://bugs.openjdk.java.net/browse/JDK-8236705 WEBREV: http://cr.openjdk.java.net/~igerasim/8236705/00/webrev/ If we agree on the content of the fix, I can push it after a sanity build. With kind regards, Ivan On 1/6/20 10:41 PM, Ivan Gerasimov wrote: Hi

Re: RFR: JDK-8236125: Windows (MSVC 2013) build fails in jpackage: Need to include strsafe.h after tchar.h

On 1/6/20 1:48 PM, Alex Kashchenko wrote: > On 01/06/2020 10:54 AM, Aleksey Shipilev wrote: >> On 1/6/20 10:57 AM, Alex Kashchenko wrote: >>> On 12/24/2019 11:54 AM, Alex Kashchenko wrote: On 12/24/2019 01:39 AM, Philip Race wrote: > Have you verified this with VS 2017 ? > Not that I

Re: Use single character replace variant in Resources.toPackageName(String)

Hi Christoph! First, the same optimization can be done in src/java.base/share/classes/jdk/internal/module/ModulePath.java:     mainClass = mainClass.replace("/", "."); Second, I wonder what was the JDK version you ran your benchmark on? After the fix for JDK-8222955 the method

Re: JDK 14 RFR of JDK-8234783: Improve wording of spec of Record.equals

Looks good to me. -- Jon On 01/06/2020 04:23 PM, Joe Darcy wrote: Hi Jon, Thanks for the comments; second take on the re-wording up at     http://cr.openjdk.java.net/~darcy/8234783.1/ and copied below for convenience. Cheers, -Joe ---

Re: RFR: JDK-8236132: Add missing properties to msi installers

Hi Alexey, Can you add description for JpIcon to overrides.wxi, otherwise looks fine. Thanks, Alexander On 12/20/2019 12:16 PM, Alexey Semenyuk wrote: Please review fix [2] for jpackage bug [1]. Add properties to msi installers to properly display installation location and icon of the

Re: JDK 14 RFR of JDK-8234783: Improve wording of spec of Record.equals

Hi Jon, Thanks for the comments; second take on the re-wording up at     http://cr.openjdk.java.net/~darcy/8234783.1/ and copied below for convenience. Cheers, -Joe --- old/src/java.base/share/classes/java/lang/Record.java 2020-01-06 16:18:43.505405539 -0800 +++

Re: JDK 14 RFR of JDK-8236695: java.lang.Record should be declared with an explicit constructor

+1 Paul. > On Jan 6, 2020, at 3:22 PM, Joe Darcy wrote: > > Hello, > > The initial implementation of java.lang.Record uses a default constructor; an > explicit constructor should be added instead. Please review the code change > and CSR for this: > > JDK-8236695: java.lang.Record

Re: RFR: JDK-8232773: ClassLoading Debug Output for Specific Classes

Hi Adam, Thanks for the examples. I was hoping for some customer issues and how they identified the root causes to understand if there is any specific problem to be considered than improving the overall class loading diagnosability. The solution you propose is to add debug/trace statements

JDK 14 RFR of JDK-8236695: java.lang.Record should be declared with an explicit constructor

Hello, The initial implementation of java.lang.Record uses a default constructor; an explicit constructor should be added instead. Please review the code change and CSR for this:     JDK-8236695: java.lang.Record should be declared with an explicit constructor     CSR:

RE: [14] RFR: 8236495, open/test/jdk/java/util/Locale/LocaleProvidersRun.java failed on mac 10.14 with de_DE locale.

Hi Naoto, The change looks good. Thanks for fixing this. We're seeing it regularly in our test infrastructure where we're running Mac systems with German locale. Best regards Christoph > -Original Message- > From: i18n-dev On Behalf Of > naoto.s...@oracle.com > Sent: Montag, 6. Januar

Re: RFR: JDK-8236683 StringBuilder / StringBuffer capacity() doc is misleading (CSR)

Looks reasonable to me - reviewed. -Brent On 1/6/20 10:29 AM, Jim Laskey wrote: Please review the following CSR intended to clarify the true meaning of StringBuilder::capacity and StringBuffer::capacity. csr: https://bugs.openjdk.java.net/browse/JDK-8236683 diff --git

Re: JDK 14 RFR of JDK-8234783: Improve wording of spec of Record.equals

I like what you're trying to do, but the wording used to introduce the names `cr` and `cp` feels a bit contorted.  How about using a single name, c, introduced up front in this line: + * of the argument. Equality of a component {@code c} is determined as follows: with the rest of the

JDK 14 RFR of JDK-8234783: Improve wording of spec of Record.equals

Hello, Please review a clarification to the specification of Record.equals:     JDK-8234783: Improve wording of spec of Record.equals     http://cr.openjdk.java.net/~darcy/8234783.0/ Patch below. Thanks, -Joe --- old/src/java.base/share/classes/java/lang/Record.java 2020-01-06

Re: [14] Review Request: 8233827 Enable screenshots in the enhanced failure handler on Linux/macOS

On 1/6/20 8:41 pm, Philip Race wrote: How is this useful given that we disable jtreg failure handlers for the headful tests ? It is disabled only in mach5 for headful nightly and CI builds, but it is enabled for other builds, also it is enabled if the headful tests are executed standalone

RFR: JDK-8236683 StringBuilder / StringBuffer capacity() doc is misleading (CSR)

Please review the following CSR intended to clarify the true meaning of StringBuilder::capacity and StringBuffer::capacity. csr: https://bugs.openjdk.java.net/browse/JDK-8236683 diff --git a/src/java.base/share/classes/java/lang/AbstractStringBuilder.java

Re: [14] RFR: 8236495,open/test/jdk/java/util/Locale/LocaleProvidersRun.java failed on mac 10.14 with de_DE locale.

Thanks, Lance, for the explanation. I explored your suggestion. Unfortunately, the test invokes a Java process where each test is performed (LocaleProvidersRun.testRun() method), and it only checks the exit code of the Java process. So throwing SkippedException in each test will not be

Re: [14] RFR: 8236495,open/test/jdk/java/util/Locale/LocaleProvidersRun.java failed on mac 10.14 with de_DE locale.

Hi Naoto > On Jan 6, 2020, at 12:55 PM, naoto.s...@oracle.com wrote: > > Hi Lance, > > Thank you for the prompt review. > > On 1/6/20 9:14 AM, Lance Andersen wrote: >> Hi Naoto, >> The change looks OK. One thought was whether any thought was given to use >> SkippedException in the else block

Re: [14] RFR: 8236495,open/test/jdk/java/util/Locale/LocaleProvidersRun.java failed on mac 10.14 with de_DE locale.

Hi Lance, Thank you for the prompt review. On 1/6/20 9:14 AM, Lance Andersen wrote: Hi Naoto, The change looks OK.  One thought was whether any thought was given to use SkippedException in the else block starting at line 370 within LocaleProviders. I am not familiar with that exception.

Re: [14] Review Request: 8233827 Enable screenshots in the enhanced failure handler on Linux/macOS

That didn't answer all my questions, at least not in a way that I can understand. How is this useful given that we disable jtreg failure handlers for the headful tests ? -phil. On 12/30/19, 11:33 AM, Sergey Bylokhov wrote: On 12/23/19 9:15 pm, Phil Race wrote: I am not sure what the right

Use single character replace variant in Resources.toPackageName(String)

Hi and a happy new year, I recently noticed that jdk.internal.module.Resources.toPackageName(String) makes use of String.replace(CharSequence, CharSequence) while it could use the single char variant in my opinion: diff --git a/src/java.base/share/classes/jdk/internal/module/Resources.java

Re: [14] RFR: 8236495,open/test/jdk/java/util/Locale/LocaleProvidersRun.java failed on mac 10.14 with de_DE locale.

Hi Naoto, The change looks OK. One thought was whether any thought was given to use SkippedException in the else block starting at line 370 within LocaleProviders. Best Lance > On Jan 6, 2020, at 12:05 PM, naoto.s...@oracle.com wrote: > > Hi, > > Please review the fix to the following

[14] RFR: 8236495,open/test/jdk/java/util/Locale/LocaleProvidersRun.java failed on mac 10.14 with de_DE locale.

Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8236495 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8236495/webrev.00/ The test case for the fix to 8232860 was only intended for the US locale. Simply adding the default

Re: RFR: JDK-8236125: Windows (MSVC 2013) build fails in jpackage: Need to include strsafe.h after tchar.h

On 01/06/2020 10:54 AM, Aleksey Shipilev wrote: On 1/6/20 10:57 AM, Alex Kashchenko wrote: On 12/24/2019 11:54 AM, Alex Kashchenko wrote: On 12/24/2019 01:39 AM, Philip Race wrote: Have you verified this with VS 2017 ? Not that I can see a problem but I doubt we want to trade breaking 2017 to

Re: RFR: JDK-8236125: Windows (MSVC 2013) build fails in jpackage: Need to include strsafe.h after tchar.h

On 1/6/20 10:57 AM, Alex Kashchenko wrote: > On 12/24/2019 11:54 AM, Alex Kashchenko wrote: >> On 12/24/2019 01:39 AM, Philip Race wrote: >>> Have you verified this with VS 2017 ? >>> Not that I can see a problem but I doubt we want to trade breaking >>> 2017 to fix 2013 ... >> >> Yes, VS 2017

Re: RFR: JDK-8236125: Windows (MSVC 2013) build fails in jpackage: Need to include strsafe.h after tchar.h

On 12/24/2019 11:54 AM, Alex Kashchenko wrote: On 12/24/2019 01:39 AM, Philip Race wrote: Have you verified this with VS 2017 ? Not that I can see a problem but I doubt we want to trade breaking 2017 to fix 2013 ... Yes, VS 2017 compiles fine with this fix. [...] Bug: