Re: RFR: 8184693: (opt) add Optional.isEmpty
OK, looks good! +1 from me. s'marks On 4/17/18 10:34 PM, Vivek Theeyarath wrote: Hi Stuart, Done with the changes http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.05/ . Regards Vivek -Original Message- From: Stuart Marks Sent: Wednesday, April 18, 2018 8:56 AM To: Vivek Theeyarath Cc: core-libs-dev ; Paul Sandoz Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty Hi Vivek, Thanks for the update. In the test files, please remove the unnecessary imports of List and the various Predicate types. In most cases it's not a problem to have unnecessary imports. I happened to notice in this case that they're left over from the previous version of the webrev, and looking at the current webrev by itself, it's clear they're unnecessary. Thanks, s'marks On 4/17/18 6:23 AM, Vivek Theeyarath wrote: Hi All, Please find the updated webrev http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04 Regards Vivek -Original Message- From: Stuart Marks Sent: Tuesday, April 17, 2018 5:11 AM To: Vivek Theeyarath Cc: core-libs-dev ; Paul Sandoz Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty Hi Vivek, Please add "@since 11" tags to the doc comments of the four Optional*.isEmpty() methods. Regarding the tests, I don't think the various newly added testIsEmpty() tests are useful. The setup in those test files already generates a fairly comprehensive set of Optional values from a variety of sources. All the assertion checking is performed in the checkEmpty() and checkPresent() methods. It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() -- as you've done -- and also to add an assertFalse(isEmpty()) call to checkPresent(). If these assertions are added, the additional testIsEmpty() test is unnecessary. This applies to all four of the regression test files. Thanks, s'marks On 4/16/18 11:08 AM, Vivek Theeyarath wrote: Hi All, Please find the updated webrev http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ . Here is the csr which I have raised for this change https://bugs.openjdk.java.net/browse/JDK-8201606 Regards Vivek -Original Message- From: Chris Hegarty Sent: Sunday, April 15, 2018 6:48 PM To: Vivek Theeyarath Cc: Remi Forax ; core-libs-dev Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty On 15 Apr 2018, at 11:25, Vivek Theeyarath wrote: Hi All, Please review http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/ This looks ok to me. For consistency, can you please update the copyright header year range in OptionalInt. -Chris. Regards Vivek -Original Message- From: Vivek Theeyarath Sent: Saturday, April 14, 2018 6:24 PM To: Remi Forax Cc: core-libs-dev Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty I missed that Remi. Thanks for pointing it out. Will address those and get back. Regards Vivek -Original Message- From: Remi Forax [mailto:fo...@univ-mlv.fr] Sent: Saturday, April 14, 2018 2:58 PM To: Vivek Theeyarath Cc: core-libs-dev Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty Hi Vivek, OptionalInt, OptionalLong and OptionalDouble should be changed too. Rémi - Mail original - De: "Vivek Theeyarath" À: "core-libs-dev" Envoyé: Samedi 14 Avril 2018 08:22:50 Objet: RFR: 8184693: (opt) add Optional.isEmpty Hi All, Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8184693 Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/ The related jtreg test was run and the test passed . Regards Vivek
RE: RFR: 8184693: (opt) add Optional.isEmpty
Hi Stuart, Done with the changes http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.05/ . Regards Vivek -Original Message- From: Stuart Marks Sent: Wednesday, April 18, 2018 8:56 AM To: Vivek Theeyarath Cc: core-libs-dev ; Paul Sandoz Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty Hi Vivek, Thanks for the update. In the test files, please remove the unnecessary imports of List and the various Predicate types. In most cases it's not a problem to have unnecessary imports. I happened to notice in this case that they're left over from the previous version of the webrev, and looking at the current webrev by itself, it's clear they're unnecessary. Thanks, s'marks On 4/17/18 6:23 AM, Vivek Theeyarath wrote: > Hi All, > Please find the updated webrev > http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04 > Regards > Vivek > > -Original Message- > From: Stuart Marks > Sent: Tuesday, April 17, 2018 5:11 AM > To: Vivek Theeyarath > Cc: core-libs-dev ; Paul Sandoz > > Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty > > Hi Vivek, > > Please add "@since 11" tags to the doc comments of the four > Optional*.isEmpty() methods. > > Regarding the tests, I don't think the various newly added testIsEmpty() > tests are useful. The setup in those test files already generates a fairly > comprehensive set of Optional values from a variety of sources. All the > assertion checking is performed in the checkEmpty() and checkPresent() > methods. > It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() > -- as you've done -- and also to add an assertFalse(isEmpty()) call to > checkPresent(). If these assertions are added, the additional testIsEmpty() > test is unnecessary. > > This applies to all four of the regression test files. > > Thanks, > > s'marks > > On 4/16/18 11:08 AM, Vivek Theeyarath wrote: >> Hi All, >> Please find the updated webrev >> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ . >> Here is the csr which I have raised for this change >> https://bugs.openjdk.java.net/browse/JDK-8201606 >> >> Regards >> Vivek >> -Original Message- >> From: Chris Hegarty >> Sent: Sunday, April 15, 2018 6:48 PM >> To: Vivek Theeyarath >> Cc: Remi Forax ; core-libs-dev >> >> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty >> >> >> >>> On 15 Apr 2018, at 11:25, Vivek Theeyarath >>> wrote: >>> >>> Hi All, >>> Please review >>> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/ >> >> This looks ok to me. >> >> For consistency, can you please update the copyright header year range in >> OptionalInt. >> >> -Chris. >> >>> Regards >>> Vivek >>> -Original Message- >>> From: Vivek Theeyarath >>> Sent: Saturday, April 14, 2018 6:24 PM >>> To: Remi Forax >>> Cc: core-libs-dev >>> Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty >>> >>> I missed that Remi. Thanks for pointing it out. Will address those and get >>> back. >>> >>> Regards >>> Vivek >>> -Original Message- >>> From: Remi Forax [mailto:fo...@univ-mlv.fr] >>> Sent: Saturday, April 14, 2018 2:58 PM >>> To: Vivek Theeyarath >>> Cc: core-libs-dev >>> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty >>> >>> Hi Vivek, >>> OptionalInt, OptionalLong and OptionalDouble should be changed too. >>> >>> Rémi >>> >>> - Mail original - De: "Vivek Theeyarath" À: "core-libs-dev" Envoyé: Samedi 14 Avril 2018 08:22:50 Objet: RFR: 8184693: (opt) add Optional.isEmpty >>> Hi All, Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8184693 Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/ The related jtreg test was run and the test passed . Regards Vivek >>
Re: JDK 11 RFR of JDK-8201766: Mark TimSortStackSize2.java as intermittently failing
Looks fine to me Joe! Thanks, David On 18/04/2018 1:59 PM, joe darcy wrote: Hello, Please review the patch below to address JDK-8201766: Mark TimSortStackSize2.java as intermittently failing which marks the test as failing intermittently and demotes it to tier 2 from tier 1. Thanks, -Joe diff -r 4c77b1453427 test/jdk/TEST.groups --- a/test/jdk/TEST.groups Tue Apr 17 16:13:30 2018 -0700 +++ b/test/jdk/TEST.groups Tue Apr 17 20:57:01 2018 -0700 @@ -1,4 +1,4 @@ -# Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -34,12 +34,14 @@ java/nio/Buffer \ com/sun/crypto/provider/Cipher \ :jdk_math \ - tools/pack200 + tools/pack200 \ + -java/util/Arrays/TimSortStackSize2.java tier2 = \ :tier2_part1 \ :tier2_part2 \ - :tier2_part3 + :tier2_part3 \ + java/util/Arrays/TimSortStackSize2.java # com/sun/crypto/provider/Cipher is in tier1 because of JDK-8132855 tier2_part1 = \ diff -r 4c77b1453427 test/jdk/java/util/Arrays/TimSortStackSize2.java --- a/test/jdk/java/util/Arrays/TimSortStackSize2.java Tue Apr 17 16:13:30 2018 -0700 +++ b/test/jdk/java/util/Arrays/TimSortStackSize2.java Tue Apr 17 20:57:01 2018 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -34,6 +34,7 @@ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions * -XX:+WhiteBoxAPI TimSortStackSize2 * @summary Test TimSort stack size on big arrays + * @key intermittent */ import java.util.ArrayList; import java.util.Arrays;
JDK 11 RFR of JDK-8201766: Mark TimSortStackSize2.java as intermittently failing
Hello, Please review the patch below to address JDK-8201766: Mark TimSortStackSize2.java as intermittently failing which marks the test as failing intermittently and demotes it to tier 2 from tier 1. Thanks, -Joe diff -r 4c77b1453427 test/jdk/TEST.groups --- a/test/jdk/TEST.groups Tue Apr 17 16:13:30 2018 -0700 +++ b/test/jdk/TEST.groups Tue Apr 17 20:57:01 2018 -0700 @@ -1,4 +1,4 @@ -# Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -34,12 +34,14 @@ java/nio/Buffer \ com/sun/crypto/provider/Cipher \ :jdk_math \ - tools/pack200 + tools/pack200 \ + -java/util/Arrays/TimSortStackSize2.java tier2 = \ :tier2_part1 \ :tier2_part2 \ - :tier2_part3 + :tier2_part3 \ + java/util/Arrays/TimSortStackSize2.java # com/sun/crypto/provider/Cipher is in tier1 because of JDK-8132855 tier2_part1 = \ diff -r 4c77b1453427 test/jdk/java/util/Arrays/TimSortStackSize2.java --- a/test/jdk/java/util/Arrays/TimSortStackSize2.java Tue Apr 17 16:13:30 2018 -0700 +++ b/test/jdk/java/util/Arrays/TimSortStackSize2.java Tue Apr 17 20:57:01 2018 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -34,6 +34,7 @@ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions * -XX:+WhiteBoxAPI TimSortStackSize2 * @summary Test TimSort stack size on big arrays + * @key intermittent */ import java.util.ArrayList; import java.util.Arrays;
Re: RFR: 8184693: (opt) add Optional.isEmpty
Hi Vivek, Thanks for the update. In the test files, please remove the unnecessary imports of List and the various Predicate types. In most cases it's not a problem to have unnecessary imports. I happened to notice in this case that they're left over from the previous version of the webrev, and looking at the current webrev by itself, it's clear they're unnecessary. Thanks, s'marks On 4/17/18 6:23 AM, Vivek Theeyarath wrote: Hi All, Please find the updated webrev http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04 Regards Vivek -Original Message- From: Stuart Marks Sent: Tuesday, April 17, 2018 5:11 AM To: Vivek Theeyarath Cc: core-libs-dev ; Paul Sandoz Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty Hi Vivek, Please add "@since 11" tags to the doc comments of the four Optional*.isEmpty() methods. Regarding the tests, I don't think the various newly added testIsEmpty() tests are useful. The setup in those test files already generates a fairly comprehensive set of Optional values from a variety of sources. All the assertion checking is performed in the checkEmpty() and checkPresent() methods. It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() -- as you've done -- and also to add an assertFalse(isEmpty()) call to checkPresent(). If these assertions are added, the additional testIsEmpty() test is unnecessary. This applies to all four of the regression test files. Thanks, s'marks On 4/16/18 11:08 AM, Vivek Theeyarath wrote: Hi All, Please find the updated webrev http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ . Here is the csr which I have raised for this change https://bugs.openjdk.java.net/browse/JDK-8201606 Regards Vivek -Original Message- From: Chris Hegarty Sent: Sunday, April 15, 2018 6:48 PM To: Vivek Theeyarath Cc: Remi Forax ; core-libs-dev Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty On 15 Apr 2018, at 11:25, Vivek Theeyarath wrote: Hi All, Please review http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/ This looks ok to me. For consistency, can you please update the copyright header year range in OptionalInt. -Chris. Regards Vivek -Original Message- From: Vivek Theeyarath Sent: Saturday, April 14, 2018 6:24 PM To: Remi Forax Cc: core-libs-dev Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty I missed that Remi. Thanks for pointing it out. Will address those and get back. Regards Vivek -Original Message- From: Remi Forax [mailto:fo...@univ-mlv.fr] Sent: Saturday, April 14, 2018 2:58 PM To: Vivek Theeyarath Cc: core-libs-dev Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty Hi Vivek, OptionalInt, OptionalLong and OptionalDouble should be changed too. Rémi - Mail original - De: "Vivek Theeyarath" À: "core-libs-dev" Envoyé: Samedi 14 Avril 2018 08:22:50 Objet: RFR: 8184693: (opt) add Optional.isEmpty Hi All, Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8184693 Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/ The related jtreg test was run and the test passed . Regards Vivek
Re: RFR: 81820709 - Container Awareness JEP
On 4/3/18 10:09 PM, Bob Vandette wrote: WEBREV: http://cr.openjdk.java.net/~bobv/8182070/v01/webrev I reviewed the webrev and look okay in general. I will look through the javadoc next. Metrics.java 37 * 1. All processes, including the current process within a container. includes the numbering. You can drop "1." and other numbers. 42 * or This adds a bullet. Maybe dropping this line. 81 * @return The name of the provider or null if Metrics are 82 * not enabled. 85 public String getProvider(); Should this method always return non-null name? For optional metric (when it's not available), the method returns 0. For example: 533 * @return The number of bytes transferred or 0 if this metric is not available. How does the client know if the metrics is not available or zero? Or the client does not care? jdk/internal/platform/cgroupv1/Metrics.java 274 return SubSystem.getLongValue(cpuacct, "cpuacct.usage"); Should this be an instance method? like cpuacct.getLongValue("cpuacct.usage"); final field name can be made all caps. I know you are going to include regression tests. WEBREV including a Prototype MBEAN for exposing these Metrics: This prototype will not be integrated as part of this JEP. It’s for information only. http://cr.openjdk.java.net/~bobv/8182070/v01/mbean-proto/ This feature adds a new -XshowSetting option “system” which displays the available system Metrics. What does java --help-extra show? The help message should include -XshowSettings:system only on Linux. % java -XshowSettings:system I expect this option shows static/configuration information rather than timing statistics e.g. CPU time and usage. It may be a smaller set but it may be good information though. It's more appropriate for monitoring tools to show the timing statistics and resource consumption rather than the launcher. Mandy
Re: 8194734 : Handle to jimage file inherited into child processes (win)
Alan, thanks a lot for applying my patch! Now, do I need to try and remove the magic from Unix process creation code? Or you'd rather have it untouched?
[11] RFR: 8181157: CLDR Timezone name fallback implementation
Hello, Please review the changes to the following issue: https://bugs.openjdk.java.net/browse/JDK-8181157 The proposed fix is located at: http://cr.openjdk.java.net/~naoto/8181157/webrev.05/ This RFE is to implement CLDR time zone names fallback mechanism [1]. Prior to this fix, time zone names are substituted with English names. With this change, it is generated according to the logic defined in TR 35. Here are the highlight of the changes: CLDRConverter: - CLDRConverter has changed to not substitute/invent time zone display names, except English bundle for compatibility & runtime performance. - For English bundle, copy over COMPAT's names as before. - CLDRConverer now parses regionFormat/gmtZeroFormat/exemplarCity CLDR provider: - CLDRTimeZoneNameProviderImpl is introduced to generate fallback names at runtime. - If COMPAT provider is present, use it also as a fallback, before the last resort (GMT offset format) Naoto [1] http://www.unicode.org/reports/tr35/tr35-dates.html#Time_Zone_Names
Re: RFR: 8201609 - Split test/jdk/:tier2 to enable better parallel execution
I’ll add a comment in TEST.groups referencing these bugs > On Apr 17, 2018, at 7:01 38AM, Alan Bateman wrote: > > On 17/04/2018 11:57, Seán Coffey wrote: >> The Cipher tests were re-grouped via >> https://bugs.openjdk.java.net/browse/JDK-8132855 >> >> NIO re-grouped via https://bugs.openjdk.java.net/browse/JDK-8132854 >> >> All appears connected with exercising hotspot intrinsics in tier1 testing. > Thanks for digging into this. We should probably add a comment to these test > group definitions as the same question will come up each time that there is > re-balancing. > > -Alan
Re: RFR(S): 8201540: [AIX] Extend the set of supported charsets in java.base
Hi Volker, Thank you for reviewing the patch. > you change looks good, although I can't really verify all the charset > aliases. For example Wikipedia mentions that "ibm-932" is equivalent > to "ibm-942" [1] but you made it an alias for "ibm-942C". What's > actually the difference between "ibm-942C" and "ibm-942"? IBM-942C is a customized version of IBM-942, in which following characters are replaced with ASCII thus making first 96 character mappings same as ASCII. 0x1A is mapped to 0x1C (in IBM-942) and to 0x1A (in IBM-942C) 0x1C is mapped to 0x7F (in IBM-942) and to 0x1C (in IBM-942C) 0x5C is mapped to 0xA5 (in IBM-942) and to 0x5C (in IBM-942C) 0x7E is mapped to 0x203E (in IBM-942) and to 0x7E (in IBM-942C) 0x7F is mapped to 0x1A (in IBM-942) and to 0x7F (in IBM-942C) Similarly, IBM-943C is a customization for IBM-943 in which character mappings for Yen(¥) and overline(‾) are replaced by their ASCII equivalents backslash (\) and tilde (~). So, we should be mapping OS code-page IBM-943 to code-page IBM-943C in Java. I am working on fixing these inconsistencies in another defect in-order not to confuse things (I hope it is alright). Current patch mainly address moving default codepage from extended codepage list to standard codepage list. Also, There are few codepages which are missing in OpenJDK. > I can sponsor your change although I would appreciate if somebody else > from IBM could have another look at your change. I tried to compare > with "IBM Java 9" but it doesn't seem to exist. They only refer to > AdoptOpenJDK and AdoptOpenJDK just uses a vanilla version of OpenJDK. Right! OpenJ9 version of JDK9 in AdoptOpenJDK is vanilla version of OpenJDK with OpenJ9. I've picked aliases for this patch from IBM JDK 8. > Finally, I hope you won't mind if I update the copyright years on the > files you changed before pushing (this is a convention in the OpenJDK > project). Sorry, I forgot to take care of copyright. Please change it this time before pushing. I will take care of it henceforth. Thanks, Bhaktavatsal Reddy -Volker Simonis wrote: - To: Bhaktavatsal R Maram From: Volker Simonis Date: 04/17/2018 08:30PM Cc: Alan Bateman , Tim Ellison , ppc-aix-port-...@openjdk.java.net, Java Core Libs Subject: Re: RFR(S): 8201540: [AIX] Extend the set of supported charsets in java.base Hi Bhaktavatsal Reddy, you change looks good, although I can't really verify all the charset aliases. For example Wikipedia mentions that "ibm-932" is equivalent to "ibm-942" [1] but you made it an alias for "ibm-942C". What's actually the difference between "ibm-942C" and "ibm-942"? I can sponsor your change although I would appreciate if somebody else from IBM could have another look at your change. I tried to compare with "IBM Java 9" but it doesn't seem to exist. They only refer to AdoptOpenJDK and AdoptOpenJDK just uses a vanilla version of OpenJDK. Finally, I hope you won't mind if I update the copyright years on the files you changed before pushing (this is a convention in the OpenJDK project). Best regards, Volker [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__en.wikipedia.org_wiki_Code-5Fpage-5F932-5F-28IBM-29&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=KUVGEwJiRVpNtQ9wUhGP6BKqzSTV1OWX31WWPdQMmqg&m=DencrOI40Trgt_TxNW4dYVWqYtpT7dPnHzaSOEsw_ZQ&s=xYfspcI7N7ZAbVMqyjM7YIb_kd-RsFPn6pINIFz_Oa4&e= On Mon, Apr 16, 2018 at 1:10 PM, Bhaktavatsal R Maram wrote: > Hi All, > > I've regenerated webrev using "hg rename" to create template files. webrev > looks much neat now.. Thanks Alan for suggestion. > > webrev - > https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Egromero_8201540_v2_&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=KUVGEwJiRVpNtQ9wUhGP6BKqzSTV1OWX31WWPdQMmqg&m=DencrOI40Trgt_TxNW4dYVWqYtpT7dPnHzaSOEsw_ZQ&s=mDikak1wXAwU-a0yd6dJml9X5N1DJg-GkQmgPl4v_5g&e= > > Thanks, > Bhaktavatsal Reddy > > > -"core-libs-dev" wrote: - > To: Alan Bateman > From: "Bhaktavatsal R Maram" > Sent by: "core-libs-dev" > Date: 04/16/2018 02:38PM > Cc: Tim Ellison , ppc-aix-port-...@openjdk.java.net, > Java Core Libs > Subject: Re: RFR(S): 8201540: [AIX] Extend the set of supported charsets in > java.base > > Hi Alan, > > I deleted IBM943C.java (using hg remove) and added new file > IBM943C.java.template (using hg add). I now understand that using "hg rename" > is giving more meaningful representation in webrev/index.html. > > I will re-generate webrev by renaming source files to templates using "hg > rename" > > Thanks, > Bhaktavatsal Reddy > > > > -Alan Bateman wrote: - > To: Bhaktavatsal R Maram , Volker Simonis > > From: Alan Bateman > Date: 04/16/2018 02:16PM > Cc: Java Core Libs , Tim Ellison > , ppc-aix-port-...@openjdk.java.net > Subject: Re: RFR(S): 8201540: [AIX] Extend the set of supported charsets in > java.base > > > On 16/04/2018 09:22, Bhaktavatsal R Maram wrote: >> >> 3. Source files for IBM-942C and IBM-943C are changed to template to support >>
Re: RFR: JDK-8194750,Console.readPassword does not save/restore tty settings
On 04/17/2018 12:13 AM, Alan Bateman wrote: On 17/04/2018 06:15, Xueming Shen wrote: Thanks! webrev has been updated accordingly as suggested. http://cr.openjdk.java.net/~sherman/8194750/webrev Just catching on this one. The changes looks good, I'm just wondering if there is any way to create a reliable test for this. -Alan It's possible to add something into the regression test (in closed now) that I wrote before built on expect to verify the "echo" status before and after. But given "expect" itself is not that reliable on all platforms and it's in closed, it might not be worth the effort. Any suggestion? sherman
Re: RFR(S): 8201540: [AIX] Extend the set of supported charsets in java.base
Hi Bhaktavatsal Reddy, you change looks good, although I can't really verify all the charset aliases. For example Wikipedia mentions that "ibm-932" is equivalent to "ibm-942" [1] but you made it an alias for "ibm-942C". What's actually the difference between "ibm-942C" and "ibm-942"? I can sponsor your change although I would appreciate if somebody else from IBM could have another look at your change. I tried to compare with "IBM Java 9" but it doesn't seem to exist. They only refer to AdoptOpenJDK and AdoptOpenJDK just uses a vanilla version of OpenJDK. Finally, I hope you won't mind if I update the copyright years on the files you changed before pushing (this is a convention in the OpenJDK project). Best regards, Volker [1] https://en.wikipedia.org/wiki/Code_page_932_(IBM) On Mon, Apr 16, 2018 at 1:10 PM, Bhaktavatsal R Maram wrote: > Hi All, > > I've regenerated webrev using "hg rename" to create template files. webrev > looks much neat now.. Thanks Alan for suggestion. > > webrev - http://cr.openjdk.java.net/~gromero/8201540/v2/ > > Thanks, > Bhaktavatsal Reddy > > > -"core-libs-dev" wrote: - > To: Alan Bateman > From: "Bhaktavatsal R Maram" > Sent by: "core-libs-dev" > Date: 04/16/2018 02:38PM > Cc: Tim Ellison , ppc-aix-port-...@openjdk.java.net, > Java Core Libs > Subject: Re: RFR(S): 8201540: [AIX] Extend the set of supported charsets in > java.base > > Hi Alan, > > I deleted IBM943C.java (using hg remove) and added new file > IBM943C.java.template (using hg add). I now understand that using "hg rename" > is giving more meaningful representation in webrev/index.html. > > I will re-generate webrev by renaming source files to templates using "hg > rename" > > Thanks, > Bhaktavatsal Reddy > > > > -Alan Bateman wrote: - > To: Bhaktavatsal R Maram , Volker Simonis > > From: Alan Bateman > Date: 04/16/2018 02:16PM > Cc: Java Core Libs , Tim Ellison > , ppc-aix-port-...@openjdk.java.net > Subject: Re: RFR(S): 8201540: [AIX] Extend the set of supported charsets in > java.base > > > On 16/04/2018 09:22, Bhaktavatsal R Maram wrote: >> >> 3. Source files for IBM-942C and IBM-943C are changed to template to support >> #1 >> > You might want to double check the webrev as it looks like you've added > templates where as I assume you mean to use "hg rename" to rename > IBM942C.java and IBM943C.java. > > -Alan > > >
RE: RFR: 8184693: (opt) add Optional.isEmpty
Hi All, Please find the updated webrev http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04 Regards Vivek -Original Message- From: Stuart Marks Sent: Tuesday, April 17, 2018 5:11 AM To: Vivek Theeyarath Cc: core-libs-dev ; Paul Sandoz Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty Hi Vivek, Please add "@since 11" tags to the doc comments of the four Optional*.isEmpty() methods. Regarding the tests, I don't think the various newly added testIsEmpty() tests are useful. The setup in those test files already generates a fairly comprehensive set of Optional values from a variety of sources. All the assertion checking is performed in the checkEmpty() and checkPresent() methods. It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() -- as you've done -- and also to add an assertFalse(isEmpty()) call to checkPresent(). If these assertions are added, the additional testIsEmpty() test is unnecessary. This applies to all four of the regression test files. Thanks, s'marks On 4/16/18 11:08 AM, Vivek Theeyarath wrote: > Hi All, > Please find the updated webrev > http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ . > Here is the csr which I have raised for this change > https://bugs.openjdk.java.net/browse/JDK-8201606 > > Regards > Vivek > -Original Message- > From: Chris Hegarty > Sent: Sunday, April 15, 2018 6:48 PM > To: Vivek Theeyarath > Cc: Remi Forax ; core-libs-dev > > Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty > > > >> On 15 Apr 2018, at 11:25, Vivek Theeyarath >> wrote: >> >> Hi All, >> Please review >> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/ > > This looks ok to me. > > For consistency, can you please update the copyright header year range in > OptionalInt. > > -Chris. > >> Regards >> Vivek >> -Original Message- >> From: Vivek Theeyarath >> Sent: Saturday, April 14, 2018 6:24 PM >> To: Remi Forax >> Cc: core-libs-dev >> Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty >> >> I missed that Remi. Thanks for pointing it out. Will address those and get >> back. >> >> Regards >> Vivek >> -Original Message- >> From: Remi Forax [mailto:fo...@univ-mlv.fr] >> Sent: Saturday, April 14, 2018 2:58 PM >> To: Vivek Theeyarath >> Cc: core-libs-dev >> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty >> >> Hi Vivek, >> OptionalInt, OptionalLong and OptionalDouble should be changed too. >> >> Rémi >> >> - Mail original - >>> De: "Vivek Theeyarath" >>> À: "core-libs-dev" >>> Envoyé: Samedi 14 Avril 2018 08:22:50 >>> Objet: RFR: 8184693: (opt) add Optional.isEmpty >> >>> Hi All, >>> >>> Please review. >>> >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693 >>> >>> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/ >>> >>> >>> >>> The related jtreg test was run and the test passed . >>> >>> >>> >>> Regards >>> >>> Vivek >
Re: RFR: 8201609 - Split test/jdk/:tier2 to enable better parallel execution
On 17/04/2018 11:57, Seán Coffey wrote: The Cipher tests were re-grouped via https://bugs.openjdk.java.net/browse/JDK-8132855 NIO re-grouped via https://bugs.openjdk.java.net/browse/JDK-8132854 All appears connected with exercising hotspot intrinsics in tier1 testing. Thanks for digging into this. We should probably add a comment to these test group definitions as the same question will come up each time that there is re-balancing. -Alan
Re: RFR: 8201609 - Split test/jdk/:tier2 to enable better parallel execution
The Cipher tests were re-grouped via https://bugs.openjdk.java.net/browse/JDK-8132855 NIO re-grouped via https://bugs.openjdk.java.net/browse/JDK-8132854 All appears connected with exercising hotspot intrinsics in tier1 testing. regards, Sean. On 17/04/2018 08:10, Alan Bateman wrote: On 16/04/2018 21:20, Christian Tornqvist wrote: Do you know what sun/nio/cs/ISO8859x.java and com/sun/crypto/provider/Cipher are special cased? Just wondering if there is a historical or current reason to not run those. No idea. It would be useful to find out why is was setup like this, if we can. The reason is that it will come up every time there is re-balancing of the tiers. -Alan
(M) RFR: 8200167: Validate more special case invocations
Bug: https://bugs.openjdk.java.net/browse/JDK-8200167 webrev: http://cr.openjdk.java.net/~dholmes/8200167/webrev/ Credit to John Rose and Vladimir Ivanov for the form of the MH fix; and to Tobias Hartmann for the C1 fix. Their help was very much appreciated (and needed!). tl;dr version: there are some places where badly formed interface method calls (which are not detected by the verifier) were missing runtime checks on the type of the receiver. Similar issues have been fixed in the past and this was a remaining hole in relation to invokespecial semantics and interface calls via MethodHandles. It also turned out there was an issue in C1 that affected direct invokespecial calls. Description of changes: - src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java Added a new form LF_INVSPECIAL_IFC for invokespecial of interface methods. - src/java.base/share/classes/java/lang/invoke/MethodHandles.java Renamed callerClass parameter to boundCallerClass, where it originates from findBoundCallerClass. The name "callerClass" is misleading because most of the time it is NULL! In getDirectMethodCommon, pass the actual caller class (lookupClass) to DirectMethodHandle.make. And correct the comment about restrictReceiver. - src/java.base/share/classes/java/lang/invoke/DirectMethodHandle.java DirectMethodHandle.make now takes a callerClass parameter (which may be null). DirectMethodHandle make "receiver" parameter is renamed "refc" as it is not the receiver (it's the resolved type of the method holder ie REFC). The Special subclass now takes a "checkClass" parameter which is either refc, or callerClass. This is what we will check the receiver against. In preparedLambdaForm we switch from LF_INVSPECIAL to LF_INVSPECIAL_IFC if the target method is in an interface. In makePreparedLambdaForm we augment the needsReceiverCheck test to include LF_INVSPECIAL_IFC. (So we will not be doing a new receiver type check on all invokespecials, just those involving interface methods.) Added DMH.checkReceiver which is overridden by both the Special and Interface subclasses. - src/hotspot/share/c1/c1_Canonicalizer.cpp Don't optimize away the checkcast when it is an invokespecial receiver check. - test/jdk/java/lang/invoke/SpecialInterfaceCall.java (I've moved this test back and forth between hotspot/runtime and j.l.invoke as it spans direct calls and MH calls. But I think on balance it lands better here.) The test sets up direct interface method calls for which invokespecial is generated by javac, and similarly it creates MethodHandles with invokespecial semantics. The should-not-throw cases are trivial. The should-throw cases involve MH wizardry. The test is broken into three parts to check the behaviour on first use (when the method has to be initially resolved); on second use (to test we don't use the cpcache in a way that is invalid); and again after forcing JIT compilation of the calls. The test is run 5 times to exercise the interpreter (yes the multiple runs internal to the test are redundant in this case, but it's much simpler to just run it all than try to work out what needs to be run); the three variants of using C1, plus a C2 only run. --- Testing: - the new test of course - hotspot/runtime/* - hotspot/compiler/jsr292 - jdk/java/lang/invoke - hs tiers 1 and 2 - jdk tiers 1, 2 and 3 Plus some related closed tests. Thanks, David -
Re: Clean-room implementation of Double::toString(double) and Float::toString(float)
Thanks Mark, in your position as the Chief Architect, Java Platform Group at Oracle, I take yours as the most authoritative answer I've got so far. Tomorrow I should find the time to finally upload the code. Thanks to everybody who kindly replied to this thread. Greetings Raffaello On 2018-04-17 00:52, mark.reinh...@oracle.com wrote: 2018/4/12 1:12:36 -0700, raffaello.giulie...@gmail.com: my code is now ready to be uploaded to the OpenJDK reps. Currently it resides on GitHub and is under the "GPLv2 + Classpath Exception" license, with myself as the copyright holder. I asked Oracle about how the copyright notice should be adapted to meet the OCA requirements but, as of today, I got no answer. Is there any official reference about the copyright notice on OpenJDK contributions? For library code, the template is in $JDK/make/templates/gpl-cp-header [1]. It begins: Copyright (c) %YEARS% Oracle and/or its affiliates. All rights reserved. DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. This code is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License version 2 only, as ... You can just use the Oracle copyright line or, at your option, replace it with your own. In any case %YEARS% should be replaced with the year in which the file was created, and if it was modified in any later year then the creation year should be followed by the latest year in which the file was modified, separated by a comma. (We can better advise you on the details once we see the actual code.) - Mark [1] http://hg.openjdk.java.net/jdk/jdk/file/tip/make/templates/gpl-cp-header
Re: RFR: JDK-8194750,Console.readPassword does not save/restore tty settings
On 17/04/2018 06:15, Xueming Shen wrote: Thanks! webrev has been updated accordingly as suggested. http://cr.openjdk.java.net/~sherman/8194750/webrev Just catching on this one. The changes looks good, I'm just wondering if there is any way to create a reliable test for this. -Alan
Re: RFR: 8201609 - Split test/jdk/:tier2 to enable better parallel execution
On 16/04/2018 21:20, Christian Tornqvist wrote: Do you know what sun/nio/cs/ISO8859x.java and com/sun/crypto/provider/Cipher are special cased? Just wondering if there is a historical or current reason to not run those. No idea. It would be useful to find out why is was setup like this, if we can. The reason is that it will come up every time there is re-balancing of the tiers. -Alan