RE: 13 RFR (XXS) 8197927: Allow the system property `java.vendor.version` to be undefined

+1 Thanks for taking care of this. /Christoph > -Original Message- > From: core-libs-dev On Behalf > Of mark.reinh...@oracle.com > Sent: Donnerstag, 6. Juni 2019 01:07 > To: core-libs-dev@openjdk.java.net > Subject: 13 RFR (XXS) 8197927: Allow the system property > `java.vendor.version`

Re: RFR: JDK-8223333: Use try-with-resources where feasible

Hi Andy, A code like this is not safe try (Writer w = new BufferedWriter(new FileWriter( ... because new BufferedWriter may throw an OutOfMemoryError, in that case the file descriptor used by the FileWriter is not freed. There are two ways to fix that, either you need to expand the try-with-

Re: RFR(s): 8205131: remove Runtime trace methods

On 6/4/19 7:46 PM, Stuart Marks wrote: Hi all, Please review this changeset and CSR request that remove the traceInstructions() and traceMethodCalls() methods from java.lang.Runtime. These methods have been deprecated for removal since Java 9, and they do nothing. I've also removed a coupl

Re: RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

Hello, Stuart, Claes! Thanks for review. Indeed, you have a point about behavioral change. I filed a CSR: https://bugs.openjdk.java.net/browse/JDK-8225393 I wanted to generate a specdiff and attach to CSR, but it seems that I don't know how to do it :( I found nothing in OpenJDK Wiki about this.

Re: RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

Hello, Roger! Thank you for review. HashMap: > > 927: Can keysToArray() be 'final'? > No, it's overridden in LinkedHashMap (where it's declared as final). HashSet.toArray calls this method, and LinkedHashSet behavior should be different (order should be preserved), so it's overridden. Other sug

RFR: JDK-8224130: create additional automated tests for create-app-image

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). [1] https://bugs.openjdk.java.net/browse/JDK-8224130 [2] http://cr.openjdk.java.net/~herrick/8224130/ /Andy

RFR: JDK-8223212: Code cleanup found during jpackage review

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). [1] https://bugs.openjdk.java.net/browse/JDK-8223212 [2] http://cr.openjdk.java.net/~herrick/8223212/ /Andy

RFR: JDK-8223586: remove jpackage dead code and other cleanup

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). [1] https://bugs.openjdk.java.net/browse/JDK-8223586 [2] http://cr.openjdk.java.net/~herrick/8223586/ /Andy

Re: RFR: 8224974: Implement JEP 352

Hi Andrew, With version 6 [1] of the patch applied, tests throw this exception: Mapped java.lang.InternalError: java.lang.NullPointerException at java.base/jdk.internal.misc.ExtendedMapMode.newMapMode(ExtendedMapMode.java:58) at java.base/jdk.internal.misc.ExtendedMapMode.(Exten

RFR: JDK-8224748: --add-launcher option --add-modules

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). [1] https://bugs.openjdk.java.net/browse/JDK-8224748 [2] http://cr.openjdk.java.net/~herrick/8224748/ /Andy

RFR: JDK-8223953: Fix CLASSPATH parsing for sub-directorys containing spaces

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). [1] https://bugs.openjdk.java.net/browse/JDK-8223953 [2] http://cr.openjdk.java.net/~herrick/8223953/ /Andy

RFR: JDK-8223241: jpackage cleanup from code review

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). [1] https://bugs.openjdk.java.net/browse/JDK-8223241 [2] http://cr.openjdk.java.net/~herrick/8223241/ /Andy

RFR: JDK-8223334: Additional cleanup in jpackage tool

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). [1] https://bugs.openjdk.java.net/browse/JDK-8223334 [2] http://cr.openjdk.java.net/~herrick/8223334/ /Andy

RFR: JDK-8223333: Use try-with-resources where feasible

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). [1] https://bugs.openjdk.java.net/browse/JDK-822 [2] http://cr.openjdk.java.net/~herrick/822/ /Andy

RFR: JDK-8222901: different behavior when --name option not used

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). [1] https://bugs.openjdk.java.net/browse/JDK-8222901 [2] http://cr.openjdk.java.net/~herrick/8222901 /Andy

Re: RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

On 6/5/19 7:27 AM, Claes Redestad wrote: On 2019-06-05 15:49, Tagir Valeev wrote: In particular it's never mentioned in HashSet, HashMap or Collection spec that toArray implements a fail-fast behavior: this is said only about the iterator() method. that's not entirely true, since the @imp

RFR: JDK-8223080: Build team code review requests.

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). [1] https://bugs.openjdk.java.net/browse/JDK-8223080 [2] http://cr.openjdk.java.net/~herrick/8223080/ /Andy

Re: 13 RFR (XXS) 8197927: Allow the system property `java.vendor.version` to be undefined

Reviewed, both the patch and CSR. Mandy On 6/5/19 4:07 PM, mark.reinh...@oracle.com wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8197927 CSR: https://bugs.openjdk.java.net/browse/JDK-8225381 Revise the specification of the `java.lang.System::getProperties` method to allow the values of

Re: 13 RFR (XXS) 8197927: Allow the system property `java.vendor.version` to be undefined

The changes look good.as does the CSR. Added myself as a reviewer > On Jun 5, 2019, at 7:07 PM, mark.reinh...@oracle.com wrote: > > Bug: https://bugs.openjdk.java.net/browse/JDK-8197927 > CSR: https://bugs.openjdk.java.net/browse/JDK-8225381 > > Revise the specification of the `java.lang.System

Re: JEP 343: Packaging Tool

I haven’t used recent versions of this tool, but I have found it essential to be able to modify the image before the final package is created. There is no way that this tool can anticipate all of the custom configuration that might be needed, and I do not want to duplicate its ability to create

13 RFR (XXS) 8197927: Allow the system property `java.vendor.version` to be undefined

Bug: https://bugs.openjdk.java.net/browse/JDK-8197927 CSR: https://bugs.openjdk.java.net/browse/JDK-8225381 Revise the specification of the `java.lang.System::getProperties` method to allow the values of specific system properties to be undefined, and to allow the system property `java.vendor.vers

Re: JEP 343: Packaging Tool

A couple comments inline... > On Jun 5, 2019, at 5:16 PM, mark.reinh...@oracle.com wrote: > > I saw that you moved JEP 343 to “Proposed to Target,” so I spent a > couple of hours looking at it. You’ve made good progress but I don’t > think this is in the “nearly finished” state that we ask of fe

Re: RFR: 8225179: (regex) Minor Pattern cleanup

Hi, Claes I think the change looks fine. A couple suggestions: --- src/java.base/share/classes/java/util/regex/Pattern.java 3505 public boolean is(int ch) { Would it make sense to have @Override here? 4777 * The matchRef is used when a reference to this group is accessed later 4778

Re: 8219992: Correct the documentation of PrintWriter to refer System.lineSeparator

looking good Brian... > On Jun 5, 2019, at 5:31 PM, Brian Burkhalter > wrote: > > Updated patch based on CSR feedback: > > --- a/src/java.base/share/classes/java/io/PrintWriter.java > +++ b/src/java.base/share/classes/java/io/PrintWriter.java > @@ -704,9 +704,8 @@ > > /** > * Terminat

Re: 8219992: Correct the documentation of PrintWriter to refer System.lineSeparator

Updated patch based on CSR feedback: --- a/src/java.base/share/classes/java/io/PrintWriter.java +++ b/src/java.base/share/classes/java/io/PrintWriter.java @@ -704,9 +704,8 @@ /** * Terminates the current line by writing the line separator string. The - * line separator string is

Re: JEP 343: Packaging Tool

Hi Mark, Thank you for your detailed review of jpackage. Given that RDP1 is only a week away, I agree that we don't have time to address your feedback. Either Andy or I will move it back to "Candidate" and then proceed as you recommend. -- Kevin On 6/5/2019 2:16 PM, mark.reinh...@oracle.co

JEP 343: Packaging Tool

I saw that you moved JEP 343 to “Proposed to Target,” so I spent a couple of hours looking at it. You’ve made good progress but I don’t think this is in the “nearly finished” state that we ask of features in the six-month cadence. I suggest that you move this JEP back to Candidate for now and con

8219992: Correct the documentation of PrintWriter to refer System.lineSeparator

https://bugs.openjdk.java.net/browse/JDK-8219992 Proposed change included below (excluding copyright year update); CSR to follow. Thanks, Brian --- a/src/java.base/share/classes/java/io/PrintWriter.java +++ b/src/java.base/share/classes/java/i

Re: RFR 8224905 : java/lang/ProcessBuilder/Basic.java#id1 failed with stream closed

Looks good, Roger. I think the bug needs "noreg-self" label. Naoto On 6/5/19 12:18 PM, Roger Riggs wrote: Please review a test change to recognize an additional message that indicates asynchronous close in the process build testing. The same fix was applied to address a similar intermittent t

Re: RFR 8224905 : java/lang/ProcessBuilder/Basic.java#id1 failed with stream closed

Hi Roger, Looks good. Brian > On Jun 5, 2019, at 12:18 PM, Roger Riggs wrote: > > Please review a test change to recognize an additional message that indicates > asynchronous close > in the process build testing. > > The same fix was applied to address a similar intermittent test issue in >

Re: RFR 8224905 : java/lang/ProcessBuilder/Basic.java#id1 failed with stream closed

looks fine roger > On Jun 5, 2019, at 3:18 PM, Roger Riggs wrote: > > Please review a test change to recognize an additional message that indicates > asynchronous close > in the process build testing. > > The same fix was applied to address a similar intermittent test issue in > 8171426[1]. >

RFR 8224905 : java/lang/ProcessBuilder/Basic.java#id1 failed with stream closed

Please review a test change to recognize an additional message that indicates asynchronous close in the process build testing. The same fix was applied to address a similar intermittent test issue in 8171426[1]. Issue:   https://bugs.openjdk.java.net/browse/JDK-8224905 Webrev:   http://cr.op

Re: RFR: 8207851: Implement JEP 352

Yes, jcheck [1] is a good thing to run for this. Then normalizer [2] can be used to clean them up. Thanks, Brian [1] https://openjdk.java.net/projects/code-tools/jcheck/ [2] /make/scripts/normalizer.pl > On Jun 5, 2019, at 11:13 AM, Gusta

Re: RFR: 8207851: Implement JEP 352

Hi Andrew, On 05/24/2019 07:06 AM, Andrew Dinn wrote: Ping! Any takers for a review? I found some trailing space in v5 and it seems they are in v6 as well. You might want to check the followings: src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp +// data cache line writeback +StubRo

Re: RFR: 8224974: Implement JEP 352

Hi Andrew, Please ignore the content below: I see the changes are in version 6 which I had not looked through before posting my previous message. Thanks, Brian > On Jun 5, 2019, at 10:25 AM, Brian Burkhalter > wrote: > > I suppose these changes will be in version 7. If it is likely that com

Re: RFR: 8224974: Implement JEP 352

Hi Andrew, > On Jun 5, 2019, at 10:10 AM, Andrew Dinn wrote: > > Thanks very much for reviewing the patch. Also, commendations on your > eagle-eyed thoroughness. Thanks. :-) > I have happily accepted most corrections. I have commented (inline > below) Likewise. > in a few places where I thin

Re: RFR: 8224974: Implement JEP 352

Hi Brian, Thanks very much for reviewing the patch. Also, commendations on your eagle-eyed thoroughness. I have happily accepted most corrections. I have commented (inline below) in a few places where I think things might need to be discussed further. I will upload a new webrev after these points

Re: JDK 13 RFR of JDK-8225353: Add @jls links to java.lang.Enum

+1 -- Jon On 6/5/19 9:49 AM, Joe Darcy wrote: Hello, As noticed by Jon, the text for java.lang.Enum refers to a section of the JLS but does not use @jls tags: "This is the common base class of all Java language enumeration types. More information about enums, including descriptions of the

Re: JDK 13 RFR of JDK-8225353: Add @jls links to java.lang.Enum

+1 > On Jun 5, 2019, at 12:49 PM, Joe Darcy wrote: > > Hello, > > As noticed by Jon, the text for java.lang.Enum refers to a section of the JLS > but does not use @jls tags: > > "This is the common base class of all Java language enumeration types. More > information about enums, including de

JDK 13 RFR of JDK-8225353: Add @jls links to java.lang.Enum

Hello, As noticed by Jon, the text for java.lang.Enum refers to a section of the JLS but does not use @jls tags: "This is the common base class of all Java language enumeration types. More information about enums, including descriptions of the implicitly declared methods synthesized by the c

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

> On Jun 5, 2019, at 2:57 AM, Alan Bateman wrote: > > > > On 05/06/2019 03:29, Mikael Vidstedt wrote: >> I, too, agree. :) >> >> New webrev which adds a new ProblemList-aot.txt to be used when running >> tests with AOT. >> >> Webrev: >> http://cr.openjdk.java.net/~mikael/webrevs/8225305/

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

+1, Its essentially (mostly) dead code, but there is a risk of removing it. Queue it up for the beginning of JDK 14 if its worth even touching it. Better things to spend time on Roger On 06/05/2019 10:58 AM, Martin Buchholz wrote: On Wed, Jun 5, 2019 at 7:51 AM David Lloyd

Re: RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

My ancient benchmark test/jdk/java/util/Collection/IteratorMicroBenchmark.java compares many toArray implementations, but none for Set or Map, and I feel a little guilty about that (but I've been reluctant to give up the extra testing for collections with duplicates) (I used that benchmark to vali

Re: RFR: 8224974: Implement JEP 352

On 05/06/2019 09:43, Andrew Dinn wrote: On 04/06/2019 11:40, Alan Bateman wrote: On 03/06/2019 15:37, Andrew Dinn wrote: : The CSR and JEP have been updated accordingly CSR:  https://bugs.openjdk.java.net/browse/JDK-8224975 JEP:  https://bugs.openjdk.java.net/browse/JDK-8207851 The specific

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Wed, Jun 5, 2019 at 7:51 AM David Lloyd wrote: > If we're talking just Linux though, has this *ever* been an issue? You've > been able to call execve on a shell script since at least 2002 as far as I > can tell from a quick search. Maybe this should be conditional so that it > can be exclude

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

https://www.unix.com/man-page/redhat/2/execve/ is from 1997. On Wed, Jun 5, 2019 at 9:50 AM David Lloyd wrote: > If we're talking just Linux though, has this *ever* been an issue? You've > been able to call execve on a shell script since at least 2002 as far as I > can tell from a quick search.

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

If we're talking just Linux though, has this *ever* been an issue? You've been able to call execve on a shell script since at least 2002 as far as I can tell from a quick search. Maybe this should be conditional so that it can be excluded on known-good platforms? On Wed, Jun 5, 2019 at 9:40 AM M

Re: RFR: 8224974: Implement JEP 352

To clarify, the following comments were for the _internal_ class. These along with a number of my other comments are just stylistic and not absolutely necessary to change. Thanks, Brian > On Jun 5, 2019, at 7:36 AM, Brian Burkhalter > wrote: > > ExtendedMapMode > 13: Constant name should

RE: RFR 8213031: (zipfs) Add support for POSIX file permissions

Hi Lance, Alan, et al., as it seems we are getting close to CSR approval, the changeset should also be finally reviewed. Here is the most current version: http://cr.openjdk.java.net/~clanger/webrevs/8213031.14/ I have made some updates to ZipConstants as suggested by Lance in a private mail.

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html """Historically, there have been two ways that implementations can exec shell scripts. One common historical implementation is that the execl(), execv(), execle(), and execve() functions return an [ENOEXEC] error for any file no

Re: RFR: 8224974: Implement JEP 352

HI Andrew, I have some minor comments on webrev.05. (webrev.06 looks rather strange.) Thanks, Brian Note: Hotspot changes not reviewed here. General Check copyrights. Some headers are missing altogether, some have incorrect years. MappedByteBuffer.java 84: “any of other modes” -> “any of

Re: RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

Hello Tagir, On 2019-06-05 15:49, Tagir Valeev wrote: Hello, Claes! Yes, it's true that CME will not be thrown anymore. Depending on the concurrent modification kind one might face various ranges of incorrect behavior, including the AIOOBE, NPE or incorrect array returned. As Collection spec

RE: RFR 8213031: (zipfs) Add support for POSIX file permissions

Hi Alan, I've added the properties to the Specification section and made a few additional updates to get the document better structured and readable. Maybe you want to double check one more time. I've set the CSR to status "Finalized". Thanks for reviewing. /Christoph From: Alan Bateman Sent

Re: RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

Hi Tagir, A few comments: HashMap: 927: Can keysToArray() be 'final'? 953, 954, 1003:  Please put the body of the methods on a separate line and separate methods with a blank line to keep it from all running together. HashSet: 362, 366, Add @Override LinkedHashMap: 565, 566: Please put t

Re: RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

Hello, Claes! Yes, it's true that CME will not be thrown anymore. Depending on the concurrent modification kind one might face various ranges of incorrect behavior, including the AIOOBE, NPE or incorrect array returned. As Collection spec says [1]: > In the absence of a stronger guarantee by the i

Re: RFR(s): 8205131: remove Runtime trace methods

Hi Stuart, +1, Added Source Compatibility to the CSR. Roger On 06/05/2019 09:27 AM, Lance Andersen wrote: HI Stuart,, I added myself as a reviewer and the webrev looks fine. Best Lance On Jun 4, 2019, at 10:46 PM, Stuart Marks wrote: Hi all, Please review this changeset and CSR request

Re: RFR(s): 8205131: remove Runtime trace methods

HI Stuart,, I added myself as a reviewer and the webrev looks fine. Best Lance > On Jun 4, 2019, at 10:46 PM, Stuart Marks wrote: > > Hi all, > > Please review this changeset and CSR request that remove the > traceInstructions() and traceMethodCalls() methods from java.lang.Runtime. > These

Re: RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

Hi Tagir, how will this behave in the face of concurrent modification? I know the current implementations deals with this in a best-effort kind of way (since neither impl. is truly thread-safe). I.e., the toArray methods of AbstractCollection that you're overriding would catch this in iterator.n

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

On 05/06/2019 03:29, Mikael Vidstedt wrote: I, too, agree. :) New webrev which adds a new ProblemList-aot.txt to be used when running tests with AOT. Webrev: http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.01/open/webrev/

Re: RFR 8213031: (zipfs) Add support for POSIX file permissions

On 04/06/2019 15:25, Langer, Christoph wrote: Hi Alan, I made some updates to the CSR. The main update was that I transferred the new documentation part from module-info.java to the CSR’s Specification section. Can you please review the CSR? The CSR mostly looks good. One thing is that the

Re: RFR: 8224974: Implement JEP 352

On 04/06/2019 11:40, Alan Bateman wrote: > On 03/06/2019 15:37, Andrew Dinn wrote: >> : >> >> The CSR and JEP have been updated accordingly >> >> CSR:  https://bugs.openjdk.java.net/browse/JDK-8224975 >> JEP:  https://bugs.openjdk.java.net/browse/JDK-8207851 >> > The specification section in the CS

RFR JDK-8225339 Optimize HashMap.keySet()/HashMap.values()/HashSet toArray() methods

Hello! Please review the specialized implementation of toArray method in (Linked)HashMap/HashSet: https://bugs.openjdk.java.net/browse/JDK-8225339 http://cr.openjdk.java.net/~tvaleev/webrev/8225339/r1/ Here's shortened benchmark output for keySet().toArray() ( KeySetToArray ) and keySet().toArra

Re: RFR: 8225179: (regex) Minor Pattern cleanup

Hi Ivan, On 2019-06-05 03:10, Ivan Gerasimov wrote: Hi Claes! Looks good to me, thanks! thanks! LookBehindNode may be better named LookBehindEndNode, as it should only match at the very end of the look-behind token. yes - updated and sanity tested. /Claes