Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Looks good.  Thanks for the analysis! -Joe On 6/1/2020 9:13 PM, naoto.s...@oracle.com wrote: Updated webrev: http://cr.openjdk.java.net/~naoto/8246261/webrev.01/ Naoto On 6/1/20 6:30 PM, naoto.s...@oracle.com wrote: Hi Joe, In fact, this bug was possibly revealed by the fix to 8242504, wher

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Updated webrev: http://cr.openjdk.java.net/~naoto/8246261/webrev.01/ Naoto On 6/1/20 6:30 PM, naoto.s...@oracle.com wrote: Hi Joe, In fact, this bug was possibly revealed by the fix to 8242504, where the system clock precision is now nanoseconds. Before that, it used to be millisecond precis

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Hi Joe, In fact, this bug was possibly revealed by the fix to 8242504, where the system clock precision is now nanoseconds. Before that, it used to be millisecond precision, so the first try for the exact match succeeded for most of the cases. Even with the nano precision fix, most of the cas

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Hi Naoto, The patch looks good to fix the failure. I'm just curious whether the 100-time comparison is necessary because of the existence of this assertion outside the loop that allowed the test to pass if the different was within a certain period of time. None of the tests had commented on t

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Hi Naoto, Looks reasonable to me :-) > On Jun 1, 2020, at 3:31 PM, naoto.s...@oracle.com wrote: > > Hello, > > Please review the fix to the following issue: > > https://bugs.openjdk.java.net/browse/JDK-8246261 > > The proposed changeset is located at: > > https://cr.openjdk.java.net/~naoto/8

Re: 8245867: Logger/bundleLeak/BundleTest.java fails due to "OutOfMemoryError: Java heap space"

Hi Daniel, Sorry the weekend got in the way :) On 29/05/2020 7:01 pm, Daniel Fuchs wrote: Hi David, Thanks for the feedback! On 29/05/2020 00:36, David Holmes wrote: This seems to be assuming that the GC will clear all SoftReferences when it needs to clear any SoftReference. I don't think th

Re: RFR: JDK-8246010: AdditionalLaunchersTest is not enabled, and fails.

Looks good. - Alexey On 6/1/2020 4:40 PM, Andy Herrick wrote: please review revised fix at [4] to issue [2]. After some discussion we think existing Error messages are sufficient and will just add the missing resource and modify test name after enabling. [4] - http://cr.openjdk.java.net/~h

Re: RFR: JDK-8246010: AdditionalLaunchersTest is not enabled, and fails.

Hi Andy, Looks good now. Thanks, Alexander On 6/1/20 2:43 PM, Andy Herrick wrote: yes - I will fix that comment in place. /Andy On 6/1/2020 5:18 PM, alexander.matv...@oracle.com wrote: Hi Andy, http://cr.openjdk.java.net/~herrick/8246010/webrev.03/test/jdk/tools/jpackage/share/AddLauncherT

Re: RFR: JDK-8246010: AdditionalLaunchersTest is not enabled, and fails.

yes - I will fix that comment in place. /Andy On 6/1/2020 5:18 PM, alexander.matv...@oracle.com wrote: Hi Andy, http://cr.openjdk.java.net/~herrick/8246010/webrev.03/test/jdk/tools/jpackage/share/AddLauncherTest.java.html * additionallauncherstest*.* installer. The output installer should p

Re: RFR: JDK-8246010: AdditionalLaunchersTest is not enabled, and fails.

Hi Andy, http://cr.openjdk.java.net/~herrick/8246010/webrev.03/test/jdk/tools/jpackage/share/AddLauncherTest.java.html * additionallauncherstest*.* installer. The output installer should provide the Should it be addlaunchertest*.*? Otherwise looks fine. Thanks, Alexander On 6/1/20 1:40 PM,

Re: RFR: 8246241: LambdaFormEditor should use a transform lookup key that is not a SoftReference

On 2020-06-01 22:31, Mandy Chung wrote: On 6/1/20 12:57 PM, Claes Redestad wrote: Adjusted the webrev in-place based on your comments (A few methods were public that should be private, etc..) This looks okay to me. Thanks, Mandy! /Claes

Re: RFR: JDK-8230744 Several classes throw OutOfMemoryError without message

On Mon, Jun 1, 2020 at 1:37 PM Brent Christian wrote: > "Unable to allocate buffer" seems vague in the context of an OOME. If > the problem is trying to create a too-large array, maybe something like, > "Buffer for \\Q...\\E sequence would exceed maximum array size". A generic message that does

Re: RFR: JDK-8246010: AdditionalLaunchersTest is not enabled, and fails.

please review revised fix at [4] to issue [2]. After some discussion we think existing Error messages are sufficient and will just add the missing resource and modify test name after enabling. [4] - http://cr.openjdk.java.net/~herrick/8246010/webrev.03/ /Andy On 5/31/2020 12:13 PM, Andy Herr

Re: RFR: JDK-8245432: Lookup::defineHiddenClass should throw UnsupportedClassVersionError if the given bytes are of an unsupported major or minor version

On 5/29/20 1:03 PM, Mandy Chung wrote: CSR: https://bugs.openjdk.java.net/browse/JDK-8246108 As CSR is approved,  I went ahead and pushed this patch: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8245432/webrev.03/ This version has fixed the range check issue Johannes caught and also added

Re: RFR: JDK-8230744 Several classes throw OutOfMemoryError without message

I continue to think JDK library code should avoid exposing VM implementation limits to users, i.e. exception detail messages should not include MAX_ARRAY_SIZE I think our grow code has been drifting to being inconsistent and incoherent over the years. In the code below, the detail message looks w

Re: RFR: JDK-8230744 Several classes throw OutOfMemoryError without message

One comment: src/java.base/share/classes/java/util/regex/Pattern.java --- 1679,1690 i += 2; int newTempLen; try { newTempLen = Math.addExact(j + 2, Math.multiplyExact(3, pLen - i)); } catch (ArithmeticException ae) { ! thro

Re: RFR: 8230743 - StringJoiner does not handle too large strings correctly

> On Jun 1, 2020, at 12:44 PM, Jim Laskey wrote: > > > >> On Jun 1, 2020, at 4:28 PM, Paul Sandoz wrote: >> >> Can we consolidate the use of Integer.MAX_VALUE - 8 in tests by referring to >> the constant jdk.internal.util.ArraysSupport.MAX_ARRAY_LENGTH? > > I suppose it's possible to ad

Re: RFR: 8246241: LambdaFormEditor should use a transform lookup key that is not a SoftReference

On 6/1/20 12:57 PM, Claes Redestad wrote: Adjusted the webrev in-place based on your comments (A few methods were public that should be private, etc..) This looks okay to me. Mandy

Re: RFR: 8230743 - StringJoiner does not handle too large strings correctly

HI Jim, StringJoiner.java: 173 and 241:  put the OutOfMemoryException name on the same line as the "new" (all one line). Otherwise looks ok. Thanks, Roger On 6/1/20 3:51 PM, Jim Laskey wrote: Thanks Martin. On Jun 1, 2020, at 4:39 PM, Martin Buchholz wrote: Missing "throw"; We should h

Re: RFR: 8246241: LambdaFormEditor should use a transform lookup key that is not a SoftReference

On 2020-06-01 20:48, Paul Sandoz wrote: Looks good. I sent some trivial comments offline. Thanks! Adjusted the webrev in-place based on your comments (A few methods were public that should be private, etc..) /Claes Paul. On Jun 1, 2020, at 4:07 AM, Claes Redestad wrote: Hi, the LambdaF

Re: RFR: 8230743 - StringJoiner does not handle too large strings correctly

Thanks Martin. > On Jun 1, 2020, at 4:39 PM, Martin Buchholz wrote: > > Missing "throw"; We should have an assertThrows test method (many > people have written such, including myself). > > +try { > +new StringJoiner(maxString, maxString, "").toString(); > +new As

Re: RFR: 8230743 - StringJoiner does not handle too large strings correctly

oops - just realized why. > On Jun 1, 2020, at 4:45 PM, Jim Laskey wrote: > > fail is not available. > >> On Jun 1, 2020, at 4:44 PM, Paul Sandoz wrote: >> >> Oh, so easy to miss. Better yet use TestNG's Assert.fail, or for simple >> cases declare the expected exception as an argument of th

Re: RFR: 8230743 - StringJoiner does not handle too large strings correctly

fail is not available. > On Jun 1, 2020, at 4:44 PM, Paul Sandoz wrote: > > Oh, so easy to miss. Better yet use TestNG's Assert.fail, or for simple cases > declare the expected exception as an argument of the test annotation. > > Paul. > >> On Jun 1, 2020, at 12:39 PM, Martin Buchholz wrote:

Re: RFR: 8230743 - StringJoiner does not handle too large strings correctly

Oh, so easy to miss. Better yet use TestNG's Assert.fail, or for simple cases declare the expected exception as an argument of the test annotation. Paul. > On Jun 1, 2020, at 12:39 PM, Martin Buchholz wrote: > > Missing "throw"; We should have an assertThrows test method (many > people have wr

Re: RFR: 8230743 - StringJoiner does not handle too large strings correctly

> On Jun 1, 2020, at 4:28 PM, Paul Sandoz wrote: > > Can we consolidate the use of Integer.MAX_VALUE - 8 in tests by referring to > the constant jdk.internal.util.ArraysSupport.MAX_ARRAY_LENGTH? I suppose it's possible to add module for java.base/jdk.internal.util.ArraysSupport to the test

Re: RFR: 8230743 - StringJoiner does not handle too large strings correctly

Missing "throw"; We should have an assertThrows test method (many people have written such, including myself). +try { +new StringJoiner(maxString, maxString, "").toString(); +new AssertionError("Should have thrown OutOfMemoryError"); On Mon, Jun 1, 2020 at 8:58 AM

[15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8246261 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8246261/webrev.00/ The test case compares two LocalTime objects, created with LocalTime.now(Clock/ZoneId). So inheren

Re: RFR: 8230743 - StringJoiner does not handle too large strings correctly

Can we consolidate the use of Integer.MAX_VALUE - 8 in tests by referring to the constant jdk.internal.util.ArraysSupport.MAX_ARRAY_LENGTH? StringJoiner.java — 164 @Override 165 public String toString() { 166 final String[] elts = this.elts; 167 if (elts == null && e

Re: RFR: JDK-8230744 Several classes throw OutOfMemoryError without message

To be honest I should (will) file another bug against the use of math exact here. I don't think the tests actually catch what they think they test. Looks good in theory but more often than not, the pos array above that code will blow up before you get there. > On Jun 1, 2020, at 4:09 PM, Pau

Re: RFR: JDK-8230744 Several classes throw OutOfMemoryError without message

+1 I quibble about the “ignored” variable name for the caught ArithmeticException. Paul. > On Jun 1, 2020, at 5:48 AM, Jim Laskey wrote: > > Cleans up every case of OutOfMemoryError without a message. Some logic > changes in String::repeat and ByteArrayChannel:: hugeCapacity. > > Thank you.

Re: RFR: 8246241: LambdaFormEditor should use a transform lookup key that is not a SoftReference

Looks good. I sent some trivial comments offline. Paul. > On Jun 1, 2020, at 4:07 AM, Claes Redestad wrote: > > Hi, > > the LambdaFormEditor uses a SoftReference-based cache to avoid > generating LambdaForms we have already generated. By introducing a > lookup key that is not a SoftReference w

[PATCH] 8245694 : java.util.Properties.entrySet() does not override java.lang.Object methods

Hi, Please help review the fix for JDK-8245694 . And this is my very first patch submission. I know it's not perfect. *BUG DESCRIPTION*: JDK-8245694

Re: RFR: 8246251: Adjust HelloClasslist after JDK-8230301

Thanks, Mandy! /Claes On 2020-06-01 18:30, Mandy Chung wrote: Hi Claes, This looks okay. Mandy On 6/1/20 4:55 AM, Claes Redestad wrote: Hi, comparing startup profiles on a few application I found a few cases where a few more classes are being generated at runtime compared to JDK 14. This

Re: RFR: 8246251: Adjust HelloClasslist after JDK-8230301

Hi Claes, This looks okay. Mandy On 6/1/20 4:55 AM, Claes Redestad wrote: Hi, comparing startup profiles on a few application I found a few cases where a few more classes are being generated at runtime compared to JDK 14. This is due to the removal of hard-coded defaults in GenerateJLIClasse

RFR: 8230743 - StringJoiner does not handle too large strings correctly

Change NegativeArraySizeException to OutOfMemoryError. Tests added. Cheers, -- Jim webrev: http://cr.openjdk.java.net/~jlaskey/8230743/webrev-00/index.html jbs: https://bugs.openjdk.java.net/browse/JDK-8230743

Re: RFR(S/T) : 8245874 : requires.extraPropDefns.vmOpts doesn't need -Xbootclasspath/a:bootClasses

ping? -- Igor > On May 27, 2020, at 10:25 AM, Igor Ignatyev wrote: > > http://cr.openjdk.java.net/~iignatyev//8245874/webrev.00 >> 8 lines changed: 2 ins; 0 del; 6 mod; > > Hi all, > > could you please review this small and trivial cleanup in TEST.ROOT files of > test/jdk/ and test/hotspot/jt

Re: RFR: 8232841: [TESTBUG] [macos] SigningPackageTest fails when untrusted certificates exist on machine

looks good. /Andy On 5/29/2020 7:39 PM, alexander.matv...@oracle.com wrote: Please review the jpackage fix for bug [1] at [2]. - I was not able to reproduce this issue. Not sure why this command returns non zero code in some cases. It still returns output which we need, so fixed by ignoring

Re: RFR: JDK-8225056 VM support for sealed classes

Hi David, Thanks for reviewing the latest changes. I'll create the follow on RFE's once the sealed classes code is in mainline. Harold On 5/31/2020 9:34 PM, David Holmes wrote: Hi Harold, On 1/06/2020 8:57 am, Harold Seigel wrote: Thanks for the comments. Here's version 3 of the JDK and VM

RFR: JDK-8230744 Several classes throw OutOfMemoryError without message

Cleans up every case of OutOfMemoryError without a message. Some logic changes in String::repeat and ByteArrayChannel:: hugeCapacity. Thank you. Cheers, -- Jim webrev: http://cr.openjdk.java.net/~jlaskey/8230744/webrev-00/index.html

Re: Sometimes constraints are questionable

My mistake it's ArraysSupport.newLength that's failing here, String::replace pos = Arrays.copyOf(pos, ArraysSupport.newLength(p, 1, p >> 1)); newLength should be causing the exception but it in turn is passing the OOM size on to Arrays.copyOf The logic in ArraysSupport::newLeng

Re: Sometimes constraints are questionable

In the same light the test in String::replace int resultLen; try { resultLen = Math.addExact(valLen, Math.multiplyExact(++p, replLen - targLen)); } catch (ArithmeticException ignored) { throw new OutOfMemoryError("Stri

RFR: 8246251: Adjust HelloClasslist after JDK-8230301

Hi, comparing startup profiles on a few application I found a few cases where a few more classes are being generated at runtime compared to JDK 14. This is due to the removal of hard-coded defaults in GenerateJLIClassesPlugin[1], in combination with how earlier implementations of SCF (used at th

Re: Sometimes constraints are questionable

Thanks David will run with that, > On May 31, 2020, at 8:34 PM, David Holmes wrote: > > On 31/05/2020 12:29 am, Jim Laskey wrote: >> I'm working through https://bugs.openjdk.java.net/browse/JDK-8230744 >> Several classes throw >> OutOfMemoryE

RFR: 8246241: LambdaFormEditor should use a transform lookup key that is not a SoftReference

Hi, the LambdaFormEditor uses a SoftReference-based cache to avoid generating LambdaForms we have already generated. By introducing a lookup key that is not a SoftReference we can improve this by allocating less up front and by inducing less work for the GC later on. I also refactored a few case