Re: RFR (12): 8213300: jaxp/unittest/transform/CR6551600Test.java fails due to exception in jdk/jdk CI

Hi Frank, The change looks good to me. Thanks for fixing the failure! Best, Joe On 12/4/18, 6:19 PM, Frank Yuan wrote: Hi all Would you like to review this patch? Bug: https://bugs.openjdk.java.net/browse/JDK-8213300 Webrev: http://cr.openjdk.java.net/~fyuan/8213300/webrev.00/ This

[12]RFR 8213127: Refactor test/java/util/ResourceBundle/Control/MissingResourceCauseTest.sh to plain java tests

Hello, Please help review the fix for refactor test/java/util/ResourceBundle/Control/MissingResourceCauseTest.sh to plain java tests. Thank you. Bug: https://bugs.openjdk.java.net/browse/JDK-8213127 Webrev: http://cr.openjdk.java.net/~dzhou/8213127/webrev.02/ Regards, Dora

RFR (12): 8213300: jaxp/unittest/transform/CR6551600Test.java fails due to exception in jdk/jdk CI

Hi all Would you like to review this patch? Bug: https://bugs.openjdk.java.net/browse/JDK-8213300 Webrev: http://cr.openjdk.java.net/~fyuan/8213300/webrev.00/ This patch made the following changes: 1. change the path of test file from the root directory of drive C to C:\temp

Re: 6516099: InputStream.skipFully(int k) to skip exactly k bytes

Hi Daniel, You are correct: at line 154 ’n’ should have been passed instead of ‘-1’ here: http://cr.openjdk.java.net/~bpb/6516099/webrev.08-delta/ Fixing that exposed an error in the

Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

On 12/4/18 12:32 AM, Claes Redestad wrote: These non-standard and tool/library specific names appear in the manifest of a great many jar files, for example on maven central, and including the most common non-standard manifest attributes significantly reduce allocation churn reading such

Re: RFR 8214794 : java.specification.version should be only the major version number

Looks good to me, too. /Magnus > 4 dec. 2018 kl. 20:34 skrev Mandy Chung : > > The revised webrev looks okay. > > Mandy > >> On 12/4/18 11:32 AM, Roger Riggs wrote: >> Hi Mandy, Martin, >> >> The new test is unnecessary, the case is covered by >> java/lang/System/Versions test >> and uses

Re: RFR(S)JDK-8214074: Ghash optimization using AVX instructions

I believe we have at least two approvals, Vladimir and me. Unless there is something else to be done, I'll do a final sanity check and push the code wednesday or thursday. Tony On 12/4/18 5:18 AM, Dmitry Chuyko wrote: Hello, AArch64 aes test runs pass in -Dmode=GCM. -Dmitry On 12/2/18

Re: RFR (JDK 12/java.base) 8213325: (props) Properties.loadFromXML does not fully comply with the spec

Thanks Naoto for the fast review! Best regards, Joe On 12/4/18, 1:53 PM, naoto.s...@oracle.com wrote: Hi Joe, Those changes in the resource bundles look fine to me. Naoto On 12/4/18 1:45 PM, Joe Wang wrote: Hi Naoto and all, Could you please take a look at the changes to the resource

Re: RFR (JDK 12/java.base) 8213325: (props) Properties.loadFromXML does not fully comply with the spec

Hi Joe, Those changes in the resource bundles look fine to me. Naoto On 12/4/18 1:45 PM, Joe Wang wrote: Hi Naoto and all, Could you please take a look at the changes to the resource files for tests of JDK-8172365 & JDK-8210408? Current webrevs:

Re: RFR (JDK 12/java.base) 8213325: (props) Properties.loadFromXML does not fully comply with the spec

Hi Naoto and all, Could you please take a look at the changes to the resource files for tests of JDK-8172365 & JDK-8210408? Current webrevs: http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v04/ Previous: http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v03/ Thanks, Joe On

Re: RFR 8214794 : java.specification.version should be only the major version number

> > LGTM

Re: RFR 8214794 : java.specification.version should be only the major version number

Looks good. /Erik On 2018-12-04 11:32, Roger Riggs wrote: Hi Mandy, Martin, The new test is unnecessary, the case is covered by java/lang/System/Versions test and uses the stronger comparison for the version numbers. It would not detect the problem unless the version included more than

Re: RFR 8214794 : java.specification.version should be only the major version number

Hi Mandy, Martin, The new test is unnecessary, the case is covered by java/lang/System/Versions test and uses the stronger comparison for the version numbers. It would not detect the problem unless the version included more than the major version. Webrev:

Re: RFR 8214794 : java.specification.version should be only the major version number

The revised webrev looks okay. Mandy On 12/4/18 11:32 AM, Roger Riggs wrote: Hi Mandy, Martin, The new test is unnecessary, the case is covered by java/lang/System/Versions test and uses the stronger comparison for the version numbers. It would not detect the problem unless the version

Re: RFR - JDK-8203442 String::transform (Code Review)

Thank you for following up - we all know the core-libs team has a busy workload, and naming topics are always tricky. I'm personally unconvinced that `transform()` is the best name out there. While transform is OK for String and maybe Optional, it is poor for List and Stream. In addition, I'm

Re: RFR 8214794 : java.specification.version should be only the major version number

Hi Roger, Looks fine. Brian > On Dec 4, 2018, at 8:23 AM, Roger Riggs wrote: > > Including build-dev for the change to GensrcMisc.gmk. > > thx. > > On 12/04/2018 11:16 AM, Roger Riggs wrote: >> Please review correctly setting the java.specification.version property >> with only the major

Re: [12] RFR: 8214770: java/time/test/java/time/format/TestNonIsoFormatter.java failed in non-english locales.

+1 Brian > On Dec 4, 2018, at 10:59 AM, Lance Andersen wrote: > > Looks OK Naoto

Re: [12] RFR: 8214770: java/time/test/java/time/format/TestNonIsoFormatter.java failed in non-english locales.

Looks OK Naoto > On Dec 4, 2018, at 1:52 PM, naoto.s...@oracle.com wrote: > > Hello, > > Please review this simple fix to the following issue: > > https://bugs.openjdk.java.net/browse/JDK-8214770 > > The proposed fix is located at: > > http://cr.openjdk.java.net/~naoto/8214770/webrev.00/ > >

[12] RFR: 8214770: java/time/test/java/time/format/TestNonIsoFormatter.java failed in non-english locales.

Hello, Please review this simple fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8214770 The proposed fix is located at: http://cr.openjdk.java.net/~naoto/8214770/webrev.00/ It is simply specifying Locale.ROOT to expect the locale independent era names. Naoto

Re: RFR 8214794 : java.specification.version should be only the major version number

On 12/4/18 8:16 AM, Roger Riggs wrote: Please review correctly setting the java.specification.version property with only the major version number.  A test is added to ensure the java spec version agrees with the major version. The symptoms are that jtreg would fail with a full version

Re: RFR - JDK-8203442 String::transform (Code Review)

Hi everybody, I've finally caught up with all of this. I see that several people are surprised by the way this has turned out. As the first-named reviewer on this changeset, I felt I should try to figure out what happened. While this API point stands on its own, this is really part of Jim's

Re: RFR 8177552: Compact Number Formatting support

Hi Nishit, Thanks for the update.  Looks fine. I have no more comments. Roger On 12/04/2018 10:50 AM, Nishit Jain wrote: Thanks Roger, Updated Webrev: http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.06/ Regards, Nishit Jain On 04-12-2018 00:58, Roger Riggs wrote: Hi Nishit,

Re: RFR 8171426 : java/lang/ProcessBuilder/Basic.java failed with Stream closed

Hi Roger, > On Dec 4, 2018, at 8:58 AM, Roger Riggs wrote: > > Thanks for the reviews. You’re welcome. > I simplify the description, the full details are in the bug report > and are more complete. > > Updated webrev: > http://cr.openjdk.java.net/~rriggs/webrev-basic-8171426-1/ >

Re: RFR 8171426 : java/lang/ProcessBuilder/Basic.java failed with Stream closed

I wasn’t as fussy about the original comment but this one is also fine. Sent from my iPhone > On Dec 4, 2018, at 12:58 PM, Roger Riggs wrote: > > Hi Brian, Jim, > > Thanks for the reviews. > > I simplify the description, the full details are in the bug report > and are more complete. > >

Re: RFR 8171426 : java/lang/ProcessBuilder/Basic.java failed with Stream closed

Hi Brian, Jim, Thanks for the reviews. I simplify the description, the full details are in the bug report and are more complete. Updated webrev:   http://cr.openjdk.java.net/~rriggs/webrev-basic-8171426-1/ Thanks, Roger On 12/03/2018 04:49 PM, Brian Burkhalter wrote: Hi Roger,

Re: RFR 8214794 : java.specification.version should be only the major version number

> > I would make a stronger assertion, that Runtime.version().feature() > converted to a String is equal to specVersion, catching bogus specVersions > like "+011"

Re: RFR 8214794 : java.specification.version should be only the major version number

Including build-dev for the change to GensrcMisc.gmk. thx. On 12/04/2018 11:16 AM, Roger Riggs wrote: Please review correctly setting the java.specification.version property with only the major version number.  A test is added to ensure the java spec version agrees with the major version. The

RFR 8214794 : java.specification.version should be only the major version number

Please review correctly setting the java.specification.version property with only the major version number.  A test is added to ensure the java spec version agrees with the major version. The symptoms are that jtreg would fail with a full version number. Webrev:  

Re: RFR: 8212794 IBM-964 is required for AIX default charset

Hi Ichiroh, Looks fine. I can sponsor that for you.  Tomorrow, though, to allow time for any other comments. Thanks, Roger On 12/04/2018 10:21 AM, Ichiroh Takiguchi wrote: Hello Roger. Thank you for your suggestion. Could you review the fix again ? Bug:   

Re: RFR 8177552: Compact Number Formatting support

Thanks Roger, Updated Webrev: http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.06/ Regards, Nishit Jain On 04-12-2018 00:58, Roger Riggs wrote: Hi Nishit, Thanks for the updates and cleanup. CompactNumberFormat.java: - 827: To locate a single character use:     if

Re: RFR: 8212794 IBM-964 is required for AIX default charset

Hello Roger. Thank you for your suggestion. Could you review the fix again ? Bug:https://bugs.openjdk.java.net/browse/JDK-8212794 Change: https://cr.openjdk.java.net/~itakiguchi/8212794/webrev.04/ Thanks, Ichiroh Takiguchi On 2018-12-04 23:36, Roger Riggs wrote: Hi Ichiroh, On

Re: RFR: 8212794 IBM-964 is required for AIX default charset

Hi Ichiroh, On 12/03/2018 10:30 PM, Ichiroh Takiguchi wrote: Hello Roger. Thank you for your suggestion. src/jdk.charsets/share/classes/sun/nio/cs/ext/IBM33722.java:     I think this is no longer needed since it only has imports.     By the way, the style is to import each specific class

Re: RFR 4947890 : Minimize JNI upcalls in system-properties initialization

Thanks Roger! On Tue, Dec 4, 2018 at 3:14 PM Roger Riggs wrote: > > Hi Thomas, > > No change was intended. > > I'll file a bug and fix it shortly. > > Roger > > > On 12/04/2018 08:07 AM, Thomas Stüfe wrote: > > Hi Roger, > > There is a difference now as to how java.specification.version is set. >

Re: RFR 4947890 : Minimize JNI upcalls in system-properties initialization

Hi Thomas, No change was intended. I'll file a bug and fix it shortly. Roger On 12/04/2018 08:07 AM, Thomas Stüfe wrote: Hi Roger, There is a difference now as to how java.specification.version is set. -PUTPROP(props, "java.specification.version", -VERSION_SPECIFICATION);

Re: RFR 8171426 : java/lang/ProcessBuilder/Basic.java failed with Stream closed

+1 > On Dec 3, 2018, at 5:45 PM, Roger Riggs wrote: > > Ping? > > On 11/27/2018 02:04 PM, Roger Riggs wrote: >> Please review a test update to address an intermittent test failure. >> The test is modified to accept the current implementation behavior of the >> input streams >> from processes

Re: RFR(S)JDK-8214074: Ghash optimization using AVX instructions

Hello, AArch64 aes test runs pass in -Dmode=GCM. -Dmitry On 12/2/18 3:37 AM, Vladimir Kozlov wrote: I am fine with Hotspot changes. But we need to verify changes on all platforms. I see that aarch64 also supports it in addition to SPARC. I will run compiler/codegen/aes/ test to make sure it

Re: RFR 4947890 : Minimize JNI upcalls in system-properties initialization

Hi Roger, There is a difference now as to how java.specification.version is set. -PUTPROP(props, "java.specification.version", -VERSION_SPECIFICATION); +props.setProperty("java.specification.version", VERSION_NUMBER); Was that intended? It seems to cause errors in our

Re: RFR: 8212794 IBM-964 is required for AIX default charset

Hello Magnus. IBM33722 should be in sun.nio.cs.ext package on jdk.charset module Because it's not used for default encoding on AIX platform. In case of template file, build tool checks make/data/charsetmapping/stdcs-aix file for this case. If class name is there, it will be migrated to

Re: RFR: 8212794 IBM-964 is required for AIX default charset

On 2018-12-04 04:30, Ichiroh Takiguchi wrote: Hello Roger. Thank you for your suggestion. src/jdk.charsets/share/classes/sun/nio/cs/ext/IBM33722.java: I think this is no longer needed since it only has imports. By the way, the style is to import each specific class and avoid wild

Re: RFR: JDK-8066619: String(byte[],int,int,int) in String has been deprecated in Manifest and Attributes

Hi Roger, I'm afraid the previous patch missed the tests, should be included this time. The intention of the patch is to solve only bug 8066619 about deprecation. I sincerely hope the changes are neutral. The new ValueUtf8Coding test heavily coincides/overlaps with 6202130 which is why I mentioned

Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

On 2018-12-04 08:42, Alan Bateman wrote: On 03/12/2018 16:50, Claes Redestad wrote: : The extra Names added to KNOWN_NAMES was my doing, and it was based on commonly used attributes in Jar file manifests scanned from a set commonly used applications as an alternative to building up a Name

Re: RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES

Hi David, unless my reading of JEP 334 (and JEP 303) is completely off then custom constants (say, Attributes$Name) would effectively be turned into condys, implying a bootstrap method call for them to be constructed. So would carry similar construction overhead as the current implementation.