Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v4]

[Martin Buchholz]

On Sat, 7 May 2022 11:29:32 GMT, Doug Lea  wrote:

>> Changes ForkJoinPool.close spec and code to trap close as a no-op if called 
>> on common pool
>
> Doug Lea has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Accommodate restrictive SecurityManagers
>  - merge with loom updates
>Merge remote-tracking branch 'refs/remotes/origin/JDK-8286294' into 
> JDK-8286294
>  - Fix testLazySubmit; enable suite

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 3533:

> 3531: 
> 3532: /**
> 3533:  * Unless this is the {@link #commonPool()}, initiates an orderly

clearer everywhere is
{@linkplain #commonPool common pool}

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 3536:

> 3534:  * shutdown in which previously submitted tasks are executed, but
> 3535:  * no new tasks will be accepted, and waits until all tasks have
> 3536:  * completed execution and the executor has terminated.

slightly clearer is
s/executor/pool/

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 3548:

> 3546:  *
> 3547:  * @throws SecurityException if a security manager exists and
> 3548:  * shutting down this ExecutorService may manipulate

clearer and more consistent is
s/this ExecutorService/this pool/

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 3565:

> 3563: while (!terminated) {
> 3564: try {
> 3565: terminated = awaitTermination(1L, 
> TimeUnit.DAYS);

I would use untimed wait

-

PR: https://git.openjdk.java.net/jdk/pull/8577

Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v4]

[Martin Buchholz]

On Sat, 7 May 2022 11:29:32 GMT, Doug Lea  wrote:

>> Changes ForkJoinPool.close spec and code to trap close as a no-op if called 
>> on common pool
>
> Doug Lea has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Accommodate restrictive SecurityManagers
>  - merge with loom updates
>Merge remote-tracking branch 'refs/remotes/origin/JDK-8286294' into 
> JDK-8286294
>  - Fix testLazySubmit; enable suite

Here's a suggested strengthening of testCloseCommonPool:

- close should have "no effect", not just "not terminate".
- the submitted task will be run by the pool ... eventually  (which suggests 
that closing the common pool maybe should quiesce the pool before returning)
-  I might merge this with testCommonPoolShutDown

/**
 * Implicitly closing common pool using try-with-resources has no effect.
 */
public void testCloseCommonPool() {
ForkJoinTask f = new FibAction(8);
ForkJoinPool pool;
try (ForkJoinPool p = pool = ForkJoinPool.commonPool()) {
p.execute(f);
}

assertFalse(pool.isShutdown());
assertFalse(pool.isTerminating());
assertFalse(pool.isTerminated());

String prop = System.getProperty(
"java.util.concurrent.ForkJoinPool.common.parallelism");
if (! "0".equals(prop)) {
f.join();
checkCompletedNormally(f);
}
}

-

PR: https://git.openjdk.java.net/jdk/pull/8577

Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v4]

[Martin Buchholz]

On Sat, 7 May 2022 11:29:32 GMT, Doug Lea  wrote:

>> Changes ForkJoinPool.close spec and code to trap close as a no-op if called 
>> on common pool
>
> Doug Lea has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Accommodate restrictive SecurityManagers
>  - merge with loom updates
>Merge remote-tracking branch 'refs/remotes/origin/JDK-8286294' into 
> JDK-8286294
>  - Fix testLazySubmit; enable suite

testAdaptInterruptible_Callable_toString belongs in ForkJoinTaskTest.java.  Oh 
wait, it's already there, commented out!  Why not fix it there?

-

PR: https://git.openjdk.java.net/jdk/pull/8577

Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v2]

[Martin Buchholz]

On Fri, 6 May 2022 21:28:45 GMT, Martin Buchholz  wrote:

>> Tests in this file are not being executed.  I think you need:
>> 
>> --- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
>> +++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
>> @@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
>>  }
>>  
>>  public static Test suite() {
>> -return new TestSuite(ForkJoinPool8Test.class);
>> +return new TestSuite(ForkJoinPool19Test.class);
>>  }
>>  
>>  /**
>
> --- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
> +++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
> @@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
>  }
>  
>  public static Test suite() {
> -return new TestSuite(ForkJoinPool8Test.class);
> +return new TestSuite(ForkJoinPool19Test.class);
>  }
>  
>  /**

testLazySubmit will cause jtreg to start hanging.
If you wait patiently for 1000 seconds, you'll get a stack dump.

-

PR: https://git.openjdk.java.net/jdk/pull/8577

Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v2]

[Martin Buchholz]

On Fri, 6 May 2022 20:25:10 GMT, Martin Buchholz  wrote:

>> Doug Lea has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fix doc types
>
> test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java line 496:
> 
>> 494: 
>> 495: /**
>> 496:  * Implictly closing a new pool using try-with-resources terminates 
>> it
> 
> Typo "Implictly" - twice

Tests in this file are not being executed.  I think you need:

--- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
+++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
@@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
 }
 
 public static Test suite() {
-return new TestSuite(ForkJoinPool8Test.class);
+return new TestSuite(ForkJoinPool19Test.class);
 }
 
 /**

-

PR: https://git.openjdk.java.net/jdk/pull/8577

Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v2]

[Martin Buchholz]

On Fri, 6 May 2022 21:27:53 GMT, Martin Buchholz  wrote:

>> test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java line 496:
>> 
>>> 494: 
>>> 495: /**
>>> 496:  * Implictly closing a new pool using try-with-resources 
>>> terminates it
>> 
>> Typo "Implictly" - twice
>
> Tests in this file are not being executed.  I think you need:
> 
> --- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
> +++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
> @@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
>  }
>  
>  public static Test suite() {
> -return new TestSuite(ForkJoinPool8Test.class);
> +return new TestSuite(ForkJoinPool19Test.class);
>  }
>  
>  /**

--- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
+++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
@@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
 }
 
 public static Test suite() {
-return new TestSuite(ForkJoinPool8Test.class);
+return new TestSuite(ForkJoinPool19Test.class);
 }
 
 /**

-

PR: https://git.openjdk.java.net/jdk/pull/8577

Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins

[Martin Buchholz]

On Fri, 6 May 2022 15:05:57 GMT, Doug Lea  wrote:

> Changes ForkJoinPool.close spec and code to trap close as a no-op if called 
> on common pool

test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java line 496:

> 494: 
> 495: /**
> 496:  * Implictly closing a new pool using try-with-resources terminates 
> it

Typo "Implictly" - twice

-

PR: https://git.openjdk.java.net/jdk/pull/8577

Re: RFR: 8277090 : jsr166 refresh for jdk19 [v2]

[Martin Buchholz]

On Mon, 2 May 2022 13:33:33 GMT, Doug Lea  wrote:

>> This is the jsr166 refresh for jdk19. See 
>> https://bugs.openjdk.java.net/browse/JDK-8285450 and 
>> https://bugs.openjdk.java.net/browse/JDK-8277090
>
> Doug Lea has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Address review comments

src/java.base/share/classes/java/util/concurrent/CompletableFuture.java line 
2153:

> 2151: 
> 2152: @Override
> 2153: public Throwable exceptionNow() {

The unwrapping of CompletionExceptions should be documented

-

PR: https://git.openjdk.java.net/jdk/pull/8490

Re: RFR: 8277087: ZipException: zip END header not found at ZipFile#Source.findEND

[Martin Buchholz]

On Sat, 13 Nov 2021 23:16:22 GMT, Sergey Bylokhov  wrote:

> The ZipOutputStream class may create bogus zip data which cannot be opened by 
> the ZipFile. The root cause is how the comment field is stored by the 
> ZipOutputStream. According to the zip specification, the comment field should 
> not be longer than 0x bytes, and we try to validate the length of the 
> comment, but unfortunately, we do this after the comment was assigned 
> already. So if the application saves the comment based on the user's input 
> and then gets an exception from the ZipOutputStream.setComment() it may 
> assume that the comment is too long and it will be ignored, but it will be 
> saved as-is to the file.
> 
> Please take a look at 
> [this](https://github.com/openjdk/jdk/commit/c435a0905dfae827687ed46015269f9c1b36c239#diff-736e247f0ec294323891a77e16a9f0dba8bc1872131c857edf18c3f349e750eeL117)
>  refactoring, and note:
>  * The comment field is assigned before the length check
>  * The null comment is ignored
> 
> The current fix will move the length validation before being assigned and 
> will use the null comment as an empty text. Note that the behavior of the 
> null parameter is not specified in the method/class/package so we are free 
> here to implement it in any way, any thoughts/suggestions on which is better?

test/jdk/java/util/zip/ZipOutputStream/EmptyComment.java line 88:

> 86: byte[] data = baos.toByteArray();
> 87: // Since the comment is empty this will be two last bytes
> 88: int pos = data.length - ZipFile.ENDHDR + ZipFile.ENDCOM;

I would probably check that I could find Phil Katz' initials at data.length - 
ZipFile.ENDHDR

test/jdk/java/util/zip/ZipOutputStream/EmptyComment.java line 95:

> 93: }
> 94: }
> 95: }

Every source file should end in exactly one newline

-

PR: https://git.openjdk.java.net/jdk/pull/6380

Re: RFR: 8276348: Use blessed modifier order in java.base

[Martin Buchholz]

On Tue, 2 Nov 2021 19:14:23 GMT, Pavel Rappo  wrote:

>> Pragmatically, fix the script to ignore those keywords on comment lines.  
>> Learn Perl, its just a regular expression pattern match and replace 
>> expression.
>> 
>> All of the changes have to be manually reviewed by the author and then the 
>> reviewers.
>> Checking unneeded changes is part of every mechanical change.
>> 
>> The text being changed in the javadoc is the *spec*; that deserves special 
>> attention in review.
>> 
>> But having seen several reviewers be unmoved by the difference, the real 
>> pragmatic view
>> is to ignore the English.
>
>> Pragmatically, fix the script to ignore those keywords on comment lines. 
>> Learn Perl, its just a regular expression pattern match and replace 
>> expression.
> 
> I understand in principle how to modify that script to ignore doc comments. 
> The thing I was referring to when said "btw, how would we do that?" was this: 
> not all comment lines are prose. Some of those lines belong to snippets of 
> code, which I guess you would also like to be properly formatted.
>  
>> But having seen several reviewers be unmoved by the difference, the real 
>> pragmatic view is to ignore the English.
> 
> I'm sorry you feel that way. Would it be okay if I made it clear that those 
> two words are not English adjectives but are special symbols that happen to 
> use Latin script and originate from the English words they resemble? If so, I 
> could enclose each of them in `{@code ... }`. If not, I could drop that 
> particular change from this PR.

The blessed-modifier-order.sh script intentionally modifies comments, with the 
hope of finding code snippets (it did!)

Probably I manually deleted the change to Object.java back in 2015, to avoid 
the sort of controversy we're seeing now.
I don't have a strong feeling either way on changing that file.

I agree with @pavelrappo  that script-generated changes should not be mixed 
with manual changes.
I would also not update copyright years for such changes.

It's a feature of blessed-modifier-order.sh that all existing formatting is 
perfectly preserved.

-

PR: https://git.openjdk.java.net/jdk/pull/6213

Re: RFR: 8276348: Use blessed modifier order in java.base

[Martin Buchholz]

On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo  wrote:

> This PR follows up one of the recent PRs, where I used a non-canonical 
> modifier order. Since the problem was noticed [^1], why not to address it at 
> mass?
> 
> As far as I remember, the first mass-canonicalization of modifiers took place 
> in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 
> files. Since then modifiers have become a bit loose, and it makes sense to 
> re-bless (using the JDK-8136583 terminology) them.
> 
> This change was produced by running the below command followed by updating 
> the copyright years on the affected files where necessary:
> 
> $ sh ./bin/blessed-modifier-order.sh src/java.base
> 
> The resulting change is much smaller than that of 2015: 39 lines spanning 21 
> files.
> 
> [^1]: 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html
>  (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465)
> [^2]: 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html

Marked as reviewed by martin (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6213

Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v2]

[Martin Buchholz]

On Mon, 11 Oct 2021 18:52:07 GMT, Andrey Turbanov  wrote:

>> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
>   update javadoc of 'newCapacity' method to refer 
> ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead

We generally avoid  in javadoc.
Especially in private javadoc.
I would write this more simply/readably as

* {@code SOFT_MAX_ARRAY_LENGTH >> coder}

-

Marked as reviewed by martin (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5878

Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE

[Martin Buchholz]

On Sat, 9 Oct 2021 17:54:16 GMT, Andrey Turbanov 
 wrote:

> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE

JDK sources should not contain dead unused fields - thanks for fixing.

The change to use newLength in this file should have adjusted the javadoc of 
newCapacity, perhaps simply to refer to ArraysSupport.SOFT_MAX_ARRAY_LENGTH 
instead.

That sounds like a job for Jim Laskey as the author of
commit 03642a01af7123298d6524a98c99a3934d35c11b
Author: Jim Laskey 
Date:   Thu Jun 11 10:08:23 2020 -0300

8230744: Several classes throw OutOfMemoryError without message

Reviewed-by: psandoz, martin, bchristi, rriggs, smarks

If that is fixed (perhaps in a different commit), then this commit is good.

History has shown that capacity growth code is highly errorprone, so it's worth 
writing whitebox tests, as I did in e.g. 

./java/util/concurrent/ConcurrentHashMap/WhiteBox.java
./java/util/ArrayDeque/WhiteBox.java
./java/util/HashMap/WhiteBoxResizeTest.java

-

Marked as reviewed by martin (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5878

Re: RFR: 8274715: Implement forEach in Collections.CopiesList [v3]

[Martin Buchholz]

On Tue, 5 Oct 2021 17:51:31 GMT, Сергей Цыпанов 
 wrote:

>> Originally was proposed by Zheka Kozlov here: 
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057192.html
>> 
>> Just a tiny optimization: we can use for-i loop instead of 
>> `Iterable.forEach()` which is relying on iterator.
>> 
>> Simple benchmark demonstrates slight improvement:
>> 
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class NCopiesBenchmarks {
>>   @Param({"10", "50", "100"})
>>   int size;
>> 
>>   private List list;
>> 
>>   @Setup
>>   public void prepare() {
>> list = Collections.nCopies(size, new Object());
>>   }
>> 
>>   @Benchmark
>>   public void forEach(Blackhole bh) {
>> list.forEach(bh::consume);
>>   }
>> }
>> 
>> 
>> 
>> before
>> 
>> Benchmark  (size)  Mode  CntScore
>> Error   Units
>> NCopiesBenchmarks.forEach  10  avgt   50   40.737 ±  
>> 1.854   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   10  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  10  avgt   50   ≈ 10⁻⁴
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count10  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach  50  avgt   50  213.324 ±  
>> 3.784   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   50  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  50  avgt   50   ≈ 10⁻³
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count50  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach 100  avgt   50  443.171 ± 
>> 17.919   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate  100  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100  avgt   500.001 ±  
>> 0.001B/op
>> NCopiesBenchmarks.forEach:·gc.count   100  avgt   50  ≈ 0
>>counts
>> 
>> after
>> 
>> Benchmark  (size)  Mode  CntScore
>> Error   Units
>> NCopiesBenchmarks.forEach  10  avgt   50   36.838 ±  
>> 0.065   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   10  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  10  avgt   50   ≈ 10⁻⁴
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count10  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach  50  avgt   50  191.173 ±  
>> 0.570   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   50  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  50  avgt   50   ≈ 10⁻⁴
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count50  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach 100  avgt   50  376.675 ±  
>> 2.476   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate  100  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100  avgt   500.001 ±  
>> 0.001B/op
>> NCopiesBenchmarks.forEach:·gc.count   100  avgt   50  ≈ 0
>>counts
>
> Сергей Цыпанов has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8274715: Properly format NCopiesBenchmarks.java

I've never sponsored a github change and https://openjdk.java.net/sponsor/ is 
unhelpfully stale.
Oh wait ... I now see 
https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/sponsor

-

PR: https://git.openjdk.java.net/jdk/pull/2524

Re: RFR: 8274715: Implement forEach in Collections.CopiesList [v3]

[Martin Buchholz]

On Tue, 5 Oct 2021 17:51:31 GMT, Сергей Цыпанов 
 wrote:

>> Originally was proposed by Zheka Kozlov here: 
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057192.html
>> 
>> Just a tiny optimization: we can use for-i loop instead of 
>> `Iterable.forEach()` which is relying on iterator.
>> 
>> Simple benchmark demonstrates slight improvement:
>> 
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class NCopiesBenchmarks {
>>   @Param({"10", "50", "100"})
>>   int size;
>> 
>>   private List list;
>> 
>>   @Setup
>>   public void prepare() {
>> list = Collections.nCopies(size, new Object());
>>   }
>> 
>>   @Benchmark
>>   public void forEach(Blackhole bh) {
>> list.forEach(bh::consume);
>>   }
>> }
>> 
>> 
>> 
>> before
>> 
>> Benchmark  (size)  Mode  CntScore
>> Error   Units
>> NCopiesBenchmarks.forEach  10  avgt   50   40.737 ±  
>> 1.854   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   10  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  10  avgt   50   ≈ 10⁻⁴
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count10  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach  50  avgt   50  213.324 ±  
>> 3.784   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   50  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  50  avgt   50   ≈ 10⁻³
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count50  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach 100  avgt   50  443.171 ± 
>> 17.919   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate  100  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100  avgt   500.001 ±  
>> 0.001B/op
>> NCopiesBenchmarks.forEach:·gc.count   100  avgt   50  ≈ 0
>>counts
>> 
>> after
>> 
>> Benchmark  (size)  Mode  CntScore
>> Error   Units
>> NCopiesBenchmarks.forEach  10  avgt   50   36.838 ±  
>> 0.065   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   10  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  10  avgt   50   ≈ 10⁻⁴
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count10  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach  50  avgt   50  191.173 ±  
>> 0.570   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   50  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  50  avgt   50   ≈ 10⁻⁴
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count50  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach 100  avgt   50  376.675 ±  
>> 2.476   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate  100  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100  avgt   500.001 ±  
>> 0.001B/op
>> NCopiesBenchmarks.forEach:·gc.count   100  avgt   50  ≈ 0
>>counts
>
> Сергей Цыпанов has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8274715: Properly format NCopiesBenchmarks.java

Looks good!

-

Marked as reviewed by martin (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2524

Re: RFR: 8274715: Implement forEach in Collections.CopiesList [v2]

[Martin Buchholz]

On Tue, 5 Oct 2021 09:18:57 GMT, Сергей Цыпанов 
 wrote:

>> Originally was proposed by Zheka Kozlov here: 
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057192.html
>> 
>> Just a tiny optimization: we can use for-i loop instead of 
>> `Iterable.forEach()` which is relying on iterator.
>> 
>> Simple benchmark demonstrates slight improvement:
>> 
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class NCopiesBenchmarks {
>>   @Param({"10", "50", "100"})
>>   int size;
>> 
>>   private List list;
>> 
>>   @Setup
>>   public void prepare() {
>> list = Collections.nCopies(size, new Object());
>>   }
>> 
>>   @Benchmark
>>   public void forEach(Blackhole bh) {
>> list.forEach(bh::consume);
>>   }
>> }
>> 
>> 
>> 
>> before
>> 
>> Benchmark  (size)  Mode  CntScore
>> Error   Units
>> NCopiesBenchmarks.forEach  10  avgt   50   40.737 ±  
>> 1.854   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   10  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  10  avgt   50   ≈ 10⁻⁴
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count10  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach  50  avgt   50  213.324 ±  
>> 3.784   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   50  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  50  avgt   50   ≈ 10⁻³
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count50  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach 100  avgt   50  443.171 ± 
>> 17.919   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate  100  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100  avgt   500.001 ±  
>> 0.001B/op
>> NCopiesBenchmarks.forEach:·gc.count   100  avgt   50  ≈ 0
>>counts
>> 
>> after
>> 
>> Benchmark  (size)  Mode  CntScore
>> Error   Units
>> NCopiesBenchmarks.forEach  10  avgt   50   36.838 ±  
>> 0.065   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   10  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  10  avgt   50   ≈ 10⁻⁴
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count10  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach  50  avgt   50  191.173 ±  
>> 0.570   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate   50  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  50  avgt   50   ≈ 10⁻⁴
>>  B/op
>> NCopiesBenchmarks.forEach:·gc.count50  avgt   50  ≈ 0
>>counts
>> NCopiesBenchmarks.forEach 100  avgt   50  376.675 ±  
>> 2.476   ns/op
>> NCopiesBenchmarks.forEach:·gc.alloc.rate  100  avgt   500.001 ±  
>> 0.001  MB/sec
>> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100  avgt   500.001 ±  
>> 0.001B/op
>> NCopiesBenchmarks.forEach:·gc.count   100  avgt   50  ≈ 0
>>counts
>
> Сергей Цыпанов has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains four commits:
> 
>  - Merge branch 'master' into ncopies
>  - 8274715: Add NCopiesBenchmarks.java
>  - 8274715: Revert some irrelevant changes
>  - 8274715: Implement forEach in Collections.CopiesList

Thanks - TIL about Blackhole::consume .

All Java source files should end with exactly one newline.  Configure your 
editor to make it so.

-

Changes requested by martin (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2524

Re: RFR: 8274715: Implement forEach in Collections.CopiesList

[Martin Buchholz]

On Thu, 11 Feb 2021 13:28:49 GMT, Сергей Цыпанов 
 wrote:

> Originally was proposed by Zheka Kozlov here: 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057192.html
> 
> Just a tiny optimization: we can use for-i loop instead of 
> `Iterable.forEach()` which is relying on iterator.
> 
> Simple benchmark demonstrates slight improvement:
> 
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class NCopiesBenchmarks {
>   @Param({"10", "50", "100"})
>   int size;
> 
>   private List list;
> 
>   @Setup
>   public void prepare() {
> list = Collections.nCopies(size, new Object());
>   }
> 
>   @Benchmark
>   public void forEach(Blackhole bh) {
> list.forEach(bh::consume);
>   }
> }
> 
> 
> 
> before
> 
> Benchmark  (size)  Mode  CntScore
> Error   Units
> NCopiesBenchmarks.forEach  10  avgt   50   40.737 ±  
> 1.854   ns/op
> NCopiesBenchmarks.forEach:·gc.alloc.rate   10  avgt   500.001 ±  
> 0.001  MB/sec
> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  10  avgt   50   ≈ 10⁻⁴ 
> B/op
> NCopiesBenchmarks.forEach:·gc.count10  avgt   50  ≈ 0 
>   counts
> NCopiesBenchmarks.forEach  50  avgt   50  213.324 ±  
> 3.784   ns/op
> NCopiesBenchmarks.forEach:·gc.alloc.rate   50  avgt   500.001 ±  
> 0.001  MB/sec
> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  50  avgt   50   ≈ 10⁻³ 
> B/op
> NCopiesBenchmarks.forEach:·gc.count50  avgt   50  ≈ 0 
>   counts
> NCopiesBenchmarks.forEach 100  avgt   50  443.171 ± 
> 17.919   ns/op
> NCopiesBenchmarks.forEach:·gc.alloc.rate  100  avgt   500.001 ±  
> 0.001  MB/sec
> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100  avgt   500.001 ±  
> 0.001B/op
> NCopiesBenchmarks.forEach:·gc.count   100  avgt   50  ≈ 0 
>   counts
> 
> after
> 
> Benchmark  (size)  Mode  CntScore
> Error   Units
> NCopiesBenchmarks.forEach  10  avgt   50   36.838 ±  
> 0.065   ns/op
> NCopiesBenchmarks.forEach:·gc.alloc.rate   10  avgt   500.001 ±  
> 0.001  MB/sec
> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  10  avgt   50   ≈ 10⁻⁴ 
> B/op
> NCopiesBenchmarks.forEach:·gc.count10  avgt   50  ≈ 0 
>   counts
> NCopiesBenchmarks.forEach  50  avgt   50  191.173 ±  
> 0.570   ns/op
> NCopiesBenchmarks.forEach:·gc.alloc.rate   50  avgt   500.001 ±  
> 0.001  MB/sec
> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm  50  avgt   50   ≈ 10⁻⁴ 
> B/op
> NCopiesBenchmarks.forEach:·gc.count50  avgt   50  ≈ 0 
>   counts
> NCopiesBenchmarks.forEach 100  avgt   50  376.675 ±  
> 2.476   ns/op
> NCopiesBenchmarks.forEach:·gc.alloc.rate  100  avgt   500.001 ±  
> 0.001  MB/sec
> NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100  avgt   500.001 ±  
> 0.001B/op
> NCopiesBenchmarks.forEach:·gc.count   100  avgt   50  ≈ 0 
>   counts

Core collection classes should have optimized versions of forEach, so this is a 
good change in principle.  Although CopiesList.forEach is unlikely to be 
performance critical.

I implemented many similar optimizations for core collection classes in past 
years.
Many of them are benchmarked in 
test/jdk/java/util/Collection/IteratorMicroBenchmark.java
That was written pre-JMH.
I see a JMH benchmark was written, but it is not part of the commit.

There are a number of unrelated changes in this commit that look like they were 
suggested by a lint-like tool.  Such changes are good, but they belong in a 
separate cleanup commit applied across large portions of the jdk sources.

I would not use "var" here - more readable with concrete types.

Similarly, I prefer not using diamond for
return new Enumeration() {

-

Changes requested by martin (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2524

Re: RFR: 8274349: ForkJoinPool.commonPool() does not work with 1 CPU [v2]

[Martin Buchholz]

On Fri, 1 Oct 2021 05:10:17 GMT, David Holmes  wrote:

>> A regression introduced in Java 17 will give the default FJ pool a 
>> parallelism of zero in a uniprocessor environment. The fix restores this to 
>> a value of 1. See bug report for details.
>> 
>> Testing:
>>  - new regression test
>>  - tiers 1-3
>> 
>> Thanks,
>> David
>
> David Holmes has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated TCK test component from @martin

Marked as reviewed by martin (Reviewer).

I always try to avoid Thread.sleep or polling in tests, so I would write the 
test like this:

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ForkJoinPool;

public class ForkJoinUniProcessor {
public static void main(String[] args) throws InterruptedException {
// If the default parallelism were zero then this task would not
// complete and the test will timeout.
CountDownLatch ran = new CountDownLatch(1);
ForkJoinPool.commonPool().submit(() -> ran.countDown());
ran.await();
}
}

-

PR: https://git.openjdk.java.net/jdk/pull/5784

Integrated: 8260664: Phaser.arrive() memory consistency effects

[Martin Buchholz]

On Wed, 3 Feb 2021 21:55:54 GMT, Martin Buchholz  wrote:

> 8260664: Phaser.arrive() memory consistency effects

This pull request has now been integrated.

Changeset: eb4a8af5
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/eb4a8af5
Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod

8260664: Phaser.arrive() memory consistency effects

Reviewed-by: dl

-

PR: https://git.openjdk.java.net/jdk/pull/2388

Re: RFR: 8259800: timeout in tck test testForkJoin(ForkJoinPool8Test)

[Martin Buchholz]

On Sat, 20 Feb 2021 14:17:16 GMT, Doug Lea  wrote:

> This addresses interactions between parallelism-0 and new shutdown support in 
> https://bugs.openjdk.java.net/browse/JDK-8259800

Marked as reviewed by martin (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/2661

Integrated: 8259074: regex benchmarks and tests

[Martin Buchholz]

On Tue, 5 Jan 2021 03:15:56 GMT, Martin Buchholz  wrote:

> 8259074: regex benchmarks and tests

This pull request has now been integrated.

Changeset: 351d7888
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/351d7888
Stats: 483 lines in 5 files changed: 473 ins; 7 del; 3 mod

8259074: regex benchmarks and tests

Reviewed-by: redestad

-

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests [v5]

[Martin Buchholz]

On Tue, 2 Feb 2021 13:10:29 GMT, Claes Redestad  wrote:

>> Martin Buchholz has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   fix imports
>
> Marked as reviewed by redestad (Reviewer).

@cl4es Your comment sent me back to jmh re-education camp.  I never looked at 
warmup iterations numbers before, but I now see they can be very useful for 
eyeballing how long warmup takes.  Very JVM-dependent of course.  In practice I 
will stick with my rule of thumb that 10 seconds of warmup is good enough for 
any simple program, and that happens to be the JMH warmup time default.  OTOH I 
think more than one warmup iteration is overkill for most benchmarks.

-

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8260664: Phaser.arrive() memory consistency effects

[Martin Buchholz]

On Wed, 3 Feb 2021 21:55:54 GMT, Martin Buchholz  wrote:

> 8260664: Phaser.arrive() memory consistency effects

@DougLea

-

PR: https://git.openjdk.java.net/jdk/pull/2388

RFR: 8260664: Phaser.arrive() memory consistency effects

[Martin Buchholz]

8260664: Phaser.arrive() memory consistency effects

-

Commit messages:
 - JDK-8260664

Changes: https://git.openjdk.java.net/jdk/pull/2388/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2388=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260664
  Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2388.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2388/head:pull/2388

PR: https://git.openjdk.java.net/jdk/pull/2388

Re: RFR: 8259074: regex benchmarks and tests [v5]

[Martin Buchholz]

> 8259074: regex benchmarks and tests

Martin Buchholz has updated the pull request incrementally with one additional 
commit since the last revision:

  fix imports

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1940/files
  - new: https://git.openjdk.java.net/jdk/pull/1940/files/89638833..2f688a41

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1940=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1940=03-04

  Stats: 8 lines in 1 file changed: 0 ins; 7 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1940.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1940/head:pull/1940

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests

[Martin Buchholz]

On Tue, 2 Feb 2021 01:40:09 GMT, Martin Buchholz  wrote:

>> @cl4es I agree pruning is a good idea. I settled on 3 data points with 16x 
>> separations as good enough to clearly show the difference between O(1) O(N) 
>> O(N^2) and O(2^N) (although O(2^N) would "run forever").
>> 
>> (although ... please tell me you're not actually running these benchmarks in 
>> an automated fashion ... too expensive, and needs a human to interpret the 
>> results)
>
>> A manual exploration of a new set of micros would naturally start with
>> the default config, so if such a config runs forever, that would be poor
>> ergonomics IMHO. I don't think such configurations should be checked in
>> in an active state.
> 
> We're actually in agreement.  There's no actual O(2^N) operation here, and 
> jmh is similar to jtreg in having timeouts indicating failure.
> 
> Although I've been using shell loops as included in the class comments, I'll 
> make sure running the tests using all the defaults gives sensible results.
> 
> I'm surprised to see jmh use the same number (5) of warmup and measurement 
> iterations.  Unless you're looking for very small effects, one warmup run 
> should be sufficient.

I added annotations for sensible (faster) default, including @Warmup(iterations 
= 1)

-

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests [v4]

[Martin Buchholz]

> 8259074: regex benchmarks and tests

Martin Buchholz has updated the pull request incrementally with one additional 
commit since the last revision:

  add annotations for sensible defaults

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1940/files
  - new: https://git.openjdk.java.net/jdk/pull/1940/files/5faba5e7..89638833

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1940=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1940=02-03

  Stats: 15 lines in 4 files changed: 12 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1940.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1940/head:pull/1940

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests

[Martin Buchholz]

On Mon, 1 Feb 2021 20:52:12 GMT, Martin Buchholz  wrote:

>> The assertion discussion aside, the micros look fine to me. 
>> 
>> With an eye towards reducing total run time I'd ask you to consider if all 
>> parameter combinations are useful or if we can get the same value after some 
>> pruning.
>
> @cl4es I agree pruning is a good idea. I settled on 3 data points with 16x 
> separations as good enough to clearly show the difference between O(1) O(N) 
> O(N^2) and O(2^N) (although O(2^N) would "run forever").
> 
> (although ... please tell me you're not actually running these benchmarks in 
> an automated fashion ... too expensive, and needs a human to interpret the 
> results)

> A manual exploration of a new set of micros would naturally start with
> the default config, so if such a config runs forever, that would be poor
> ergonomics IMHO. I don't think such configurations should be checked in
> in an active state.

We're actually in agreement.  There's no actual O(2^N) operation here, and jmh 
is similar to jtreg in having timeouts indicating failure.

Although I've been using shell loops as included in the class comments, I'll 
make sure running the tests using all the defaults gives sensible results.

I'm surprised to see jmh use the same number (5) of warmup and measurement 
iterations.  Unless you're looking for very small effects, one warmup run 
should be sufficient.

-

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests [v2]

[Martin Buchholz]

On Mon, 1 Feb 2021 00:23:56 GMT, Martin Buchholz  wrote:

>> test/micro/org/openjdk/bench/java/util/regex/Trim.java line 119:
>> 
>>> 117: assert ! lookBehind_find();
>>> 118: assert ! find_loop_two_matchers();
>>> 119: assert ! find_loop_usePattern();
>> 
>> At a risk of muddling the profiles a bit, this can be rewritten as:
>> 
>>  if (!simple_find()) throw new IllegalStateException("simple_find is 
>> incorrect");
>>  ...
>
> Thanks Aleksey.  But I was hoping for something more magical.
> 
> We really want the checking of the result of the benchmark method to be 
> co-located in the source code with the method, but with zero disturbance to 
> the benchmark numbers.  Is such magic possible?

I refactored the correctness checking, no longer using the assert facility.

-

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests [v3]

[Martin Buchholz]

> 8259074: regex benchmarks and tests

Martin Buchholz has updated the pull request incrementally with two additional 
commits since the last revision:

 - refactor correctness checking as suggested by @shipilev
 - prune @Param values as suggested by Claes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1940/files
  - new: https://git.openjdk.java.net/jdk/pull/1940/files/abb6c672..5faba5e7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1940=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1940=01-02

  Stats: 89 lines in 3 files changed: 44 ins; 25 del; 20 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1940.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1940/head:pull/1940

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests

[Martin Buchholz]

On Mon, 1 Feb 2021 10:22:14 GMT, Claes Redestad  wrote:

>> @amalloy - you are invited to comment on regex content
>> @cl4es @shipilev - you are invited to point out my jmh bad practices
>
> The assertion discussion aside, the micros look fine to me. 
> 
> With an eye towards reducing total run time I'd ask you to consider if all 
> parameter combinations are useful or if we can get the same value after some 
> pruning.

@cl4es I agree pruning is a good idea. I settled on 3 data points with 16x 
separations as good enough to clearly show the difference between O(1) O(N) 
O(N^2) and O(2^N) (although O(2^N) would "run forever").

(although ... please tell me you're not actually running these benchmarks in an 
automated fashion ... too expensive, and needs a human to interpret the results)

-

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests

[Martin Buchholz]

On Mon, 1 Feb 2021 10:22:14 GMT, Claes Redestad  wrote:

>> @amalloy - you are invited to comment on regex content
>> @cl4es @shipilev - you are invited to point out my jmh bad practices
>
> The assertion discussion aside, the micros look fine to me. 
> 
> With an eye towards reducing total run time I'd ask you to consider if all 
> parameter combinations are useful or if we can get the same value after some 
> pruning.

@stuart-marks you are invited to review

-

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests [v2]

[Martin Buchholz]

On Sun, 31 Jan 2021 08:18:28 GMT, Aleksey Shipilev  wrote:

>> Martin Buchholz has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   address comments from @amalloy; introduce technically correct use of 
>> "numeral"
>
> test/micro/org/openjdk/bench/java/util/regex/Trim.java line 119:
> 
>> 117: assert ! lookBehind_find();
>> 118: assert ! find_loop_two_matchers();
>> 119: assert ! find_loop_usePattern();
> 
> At a risk of muddling the profiles a bit, this can be rewritten as:
> 
>  if (!simple_find()) throw new IllegalStateException("simple_find is 
> incorrect");
>  ...

Thanks Aleksey.  But I was hoping for something more magical.

We really want the checking of the result of the benchmark method to be 
co-located in the source code with the method, but with zero disturbance to the 
benchmark numbers.  Is such magic possible?

-

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests [v2]

[Martin Buchholz]

> 8259074: regex benchmarks and tests

Martin Buchholz has updated the pull request incrementally with one additional 
commit since the last revision:

  address comments from @amalloy; introduce technically correct use of "numeral"

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1940/files
  - new: https://git.openjdk.java.net/jdk/pull/1940/files/cf0922f4..abb6c672

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1940=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1940=00-01

  Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1940.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1940/head:pull/1940

PR: https://git.openjdk.java.net/jdk/pull/1940

Re: RFR: 8259074: regex benchmarks and tests

[Martin Buchholz]

On Tue, 5 Jan 2021 03:15:56 GMT, Martin Buchholz  wrote:

> 8259074: regex benchmarks and tests

@amalloy - you are invited to comment on regex content
@cl4es @shipilev - you are invited to point out my jmh bad practices

-

PR: https://git.openjdk.java.net/jdk/pull/1940

RFR: 8259074: regex benchmarks and tests

[Martin Buchholz]

8259074: regex benchmarks and tests

-

Commit messages:
 - more benchmarks
 - JDK-8259074

Changes: https://git.openjdk.java.net/jdk/pull/1940/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1940=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8259074
  Stats: 444 lines in 5 files changed: 442 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1940.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1940/head:pull/1940

PR: https://git.openjdk.java.net/jdk/pull/1940

Integrated: 8260461: Modernize jsr166 tck tests

[Martin Buchholz]

On Tue, 26 Jan 2021 21:23:25 GMT, Martin Buchholz  wrote:

> 8260461: Modernize jsr166 tck tests

This pull request has now been integrated.

Changeset: 81e9e6a7
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/81e9e6a7
Stats: 7616 lines in 71 files changed: 318 ins; 187 del; 7111 mod

8260461: Modernize jsr166 tck tests

Reviewed-by: dl

-

PR: https://git.openjdk.java.net/jdk/pull/2245

Re: RFR: 8260461: Modernize jsr166 tck tests [v3]

[Martin Buchholz]

On Wed, 27 Jan 2021 00:44:51 GMT, Doug Lea  wrote:

>> Martin Buchholz has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR. The pull request 
>> contains one new commit since the last revision:
>> 
>>   JDK-8260461
>
> Marked as reviewed by dl (Reviewer).

jcheck bot caught trailing whitespace in commit

-

PR: https://git.openjdk.java.net/jdk/pull/2245

Re: RFR: 8260461: Modernize jsr166 tck tests [v3]

[Martin Buchholz]

> 8260461: Modernize jsr166 tck tests

Martin Buchholz has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  JDK-8260461

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2245/files
  - new: https://git.openjdk.java.net/jdk/pull/2245/files/0cd05e2d..ed6b6f0f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2245=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2245=01-02

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2245.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2245/head:pull/2245

PR: https://git.openjdk.java.net/jdk/pull/2245

Re: RFR: 8260461: Modernize jsr166 tck tests [v2]

[Martin Buchholz]

> 8260461: Modernize jsr166 tck tests

Martin Buchholz has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains one commit:

  JDK-8260461

-

Changes: https://git.openjdk.java.net/jdk/pull/2245/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2245=01
  Stats: 7616 lines in 71 files changed: 318 ins; 187 del; 7111 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2245.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2245/head:pull/2245

PR: https://git.openjdk.java.net/jdk/pull/2245

RFR: 8260461: Modernize jsr166 tck tests

[Martin Buchholz]

8260461: Modernize jsr166 tck tests

-

Commit messages:
 - JDK-8260461

Changes: https://git.openjdk.java.net/jdk/pull/2245/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2245=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260461
  Stats: 7535 lines in 70 files changed: 314 ins; 183 del; 7038 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2245.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2245/head:pull/2245

PR: https://git.openjdk.java.net/jdk/pull/2245

Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v2]

[Martin Buchholz]

On Sun, 24 Jan 2021 20:14:04 GMT, Martin Buchholz  wrote:

>> Stuart Marks has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fix typo, clarify asserts disabled, test prefGrowth==0
>
> Marked as reviewed by martin (Reviewer).

The new docs and tests are superb.

-

PR: https://git.openjdk.java.net/jdk/pull/1617

Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v2]

[Martin Buchholz]

On Tue, 8 Dec 2020 06:14:35 GMT, Stuart Marks  wrote:

>> This rewrites the doc of ArraysSupport.newLength, adds detail to the 
>> exception message, and adds a test. In addition to some renaming and a bit 
>> of refactoring of the actual code, I also made two changes of substance to 
>> the code:
>> 
>> 1. I fixed a problem with overflow checking. In the original code, if 
>> oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this 
>> method could return a negative value. It turns out that writing tests helps 
>> find bugs!
>> 
>> 2. Under the old policy, if oldLength and minGrowth required a length above 
>> SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would 
>> return Integer.MAX_VALUE. That doesn't make any sense, because attempting to 
>> allocate an array of that length will almost certainly cause the Hotspot to 
>> throw OOME because its implementation limit was exceeded. Instead, if the 
>> required length is in this range, this method returns that required length.
>> 
>> Separately, I'll work on retrofitting various call sites around the JDK to 
>> use this method.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix typo, clarify asserts disabled, test prefGrowth==0

Marked as reviewed by martin (Reviewer).

test/jdk/jdk/internal/util/ArraysSupport/NewLength.java line 101:

> 99: fail("expected OutOfMemoryError, got normal return value of " 
> + r);
> 100: } catch (OutOfMemoryError oome) {
> 101: // ok

Instead of an //ok comment, I like to give the exception a name that makes the 
intent clear.

} catch (IllegalArgumentException success) {}

-

PR: https://git.openjdk.java.net/jdk/pull/1617

Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v2]

[Martin Buchholz]

On Fri, 4 Dec 2020 17:31:20 GMT, Stuart Marks  wrote:

>> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 654:
>> 
>>> 652: return SOFT_MAX_ARRAY_LENGTH;
>>> 653: } else {
>>> 654: return minLength;
>> 
>> Isn't this last `else if... then.. else` the same as:
>> `return Math.max(minLength, SOFT_MAX_ARRAY_LENGTH)`
>
> It is, and I considered replacing it, but I felt that it obscured what was 
> going on.

Competing parts of Martin's brain are voting for Roger's and Stuart's versions.

-

PR: https://git.openjdk.java.net/jdk/pull/1617

Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v2]

[Martin Buchholz]

On Tue, 8 Dec 2020 06:14:35 GMT, Stuart Marks  wrote:

>> This rewrites the doc of ArraysSupport.newLength, adds detail to the 
>> exception message, and adds a test. In addition to some renaming and a bit 
>> of refactoring of the actual code, I also made two changes of substance to 
>> the code:
>> 
>> 1. I fixed a problem with overflow checking. In the original code, if 
>> oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this 
>> method could return a negative value. It turns out that writing tests helps 
>> find bugs!
>> 
>> 2. Under the old policy, if oldLength and minGrowth required a length above 
>> SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would 
>> return Integer.MAX_VALUE. That doesn't make any sense, because attempting to 
>> allocate an array of that length will almost certainly cause the Hotspot to 
>> throw OOME because its implementation limit was exceeded. Instead, if the 
>> required length is in this range, this method returns that required length.
>> 
>> Separately, I'll work on retrofitting various call sites around the JDK to 
>> use this method.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix typo, clarify asserts disabled, test prefGrowth==0

src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 643:

> 641: return prefLength;
> 642: } else {
> 643: return hugeLength(oldLength, minGrowth);

Could add a comment like
// outline cold code

-

PR: https://git.openjdk.java.net/jdk/pull/1617

Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v2]

[Martin Buchholz]

On Tue, 8 Dec 2020 06:14:35 GMT, Stuart Marks  wrote:

>> This rewrites the doc of ArraysSupport.newLength, adds detail to the 
>> exception message, and adds a test. In addition to some renaming and a bit 
>> of refactoring of the actual code, I also made two changes of substance to 
>> the code:
>> 
>> 1. I fixed a problem with overflow checking. In the original code, if 
>> oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this 
>> method could return a negative value. It turns out that writing tests helps 
>> find bugs!
>> 
>> 2. Under the old policy, if oldLength and minGrowth required a length above 
>> SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would 
>> return Integer.MAX_VALUE. That doesn't make any sense, because attempting to 
>> allocate an array of that length will almost certainly cause the Hotspot to 
>> throw OOME because its implementation limit was exceeded. Instead, if the 
>> required length is in this range, this method returns that required length.
>> 
>> Separately, I'll work on retrofitting various call sites around the JDK to 
>> use this method.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix typo, clarify asserts disabled, test prefGrowth==0

src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 631:

> 629:  * @param minGrowth   minimum required growth amount (must be 
> positive)
> 630:  * @param prefGrowth  preferred growth amount
> 631:  * @return the new length of the array

Slightly better seems s/@return the new length of the array/@return the new 
array length/

-

PR: https://git.openjdk.java.net/jdk/pull/1617

Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v2]

[Martin Buchholz]

On Wed, 9 Dec 2020 00:32:37 GMT, Paul Sandoz  wrote:

>> Stuart Marks has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fix typo, clarify asserts disabled, test prefGrowth==0
>
> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 616:
> 
>> 614:  * greater than the soft maximum but does not exceed 
>> Integer.MAX_VALUE, the minimum
>> 615:  * required length is returned. Otherwise, the minimum required 
>> length exceeds
>> 616:  * Integer.MAX_VALUE, which can never be fulfilled, so this method 
>> throws OutOfMemoryError.
> 
> I think you can simplify with:
> 
> Suggestion:
> 
>  * If the preferred length exceeds the soft maximum, we use the minimum 
> growth
>  * amount. The minimum required length is determined by adding the 
> minimum growth
>  * amount to the current length. 
>  * If the minimum required length exceeds Integer.MAX_VALUE, then this 
> method 
>  * throws OutOfMemoryError. Otherwise, this method returns the soft 
> maximum or
>  * minimum required length, which ever is greater.
> 
> Then i think it follows that `Math.max` can be used in the implementation.

I agree with Paul's comment of Dec 8.  Especially the s/Since/If/

I would amend with s/which ever/whichever/ but that might be my dialect of 
North American

-

PR: https://git.openjdk.java.net/jdk/pull/1617

Re: RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v2]

[Martin Buchholz]

On Tue, 8 Dec 2020 06:14:35 GMT, Stuart Marks  wrote:

>> This rewrites the doc of ArraysSupport.newLength, adds detail to the 
>> exception message, and adds a test. In addition to some renaming and a bit 
>> of refactoring of the actual code, I also made two changes of substance to 
>> the code:
>> 
>> 1. I fixed a problem with overflow checking. In the original code, if 
>> oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this 
>> method could return a negative value. It turns out that writing tests helps 
>> find bugs!
>> 
>> 2. Under the old policy, if oldLength and minGrowth required a length above 
>> SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would 
>> return Integer.MAX_VALUE. That doesn't make any sense, because attempting to 
>> allocate an array of that length will almost certainly cause the Hotspot to 
>> throw OOME because its implementation limit was exceeded. Instead, if the 
>> required length is in this range, this method returns that required length.
>> 
>> Separately, I'll work on retrofitting various call sites around the JDK to 
>> use this method.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix typo, clarify asserts disabled, test prefGrowth==0

Many years ago, when I wrote the unfactored MAX_ARRAY_LENGTH code, I considered 
refactoring it, but didn't follow through because various implementations had 
too many small differences .  I'm glad you made it work.

I'm happy to see good tests added.  Testability is a benefit of refactoring.

I'm happy to see the name change to SOFT_MAX_ARRAY_LENGTH - that's a better 
name.

-

PR: https://git.openjdk.java.net/jdk/pull/1617

Re: RFR: 8252412: [macos11] system dynamic libraries removed from filesystem [v2]

[Martin Buchholz]

On Fri, 22 Jan 2021 22:56:08 GMT, Jiangli Zhou  wrote:

>> Martin Buchholz has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR.
>
> Marked as reviewed by jiangli (Reviewer).

Last call for a reviewer familiar with macos or smartcardio.  In case of 
crickets I will submit soon.

-

PR: https://git.openjdk.java.net/jdk/pull/2119

Re: RFR: 8252412: [macos11] system dynamic libraries removed from filesystem [v2]

[Martin Buchholz]

My github comment was mangled forwarding to core-libs.  I suspect the skara
bidirectional mailing list forwarding bot discards lines with leading "/"
.

Instead the bot should pass on unrecognized github comment commands
unmodified.

On Fri, Jan 22, 2021 at 12:12 PM Martin Buchholz 
wrote:

> On Fri, 22 Jan 2021 19:46:13 GMT, Jiangli Zhou 
> wrote:
>
> >> Martin Buchholz has refreshed the contents of this pull request, and
> previous commits have been removed. The incremental views will show
> differences compared to the previous content of the PR.
> >
> >
> src/java.smartcardio/unix/classes/sun/security/smartcardio/PlatformPCSC.java
> line 132:
> >
> >> 130: // existence of the containing directory instead of the
> file.
> >> 131: lib = PCSC_FRAMEWORK;
> >> 132: if (new File(lib).getParentFile().isDirectory()) {
> >
> > This is outside of my normal area, could you please explain why checking
> the existence of the containing directory is equivalent to checking the
> file here? Does it provide the expected behavior on all unix-like platforms
> or only macos?
> >
> > Could you please also provide a jtreg test for this change?
>
> The directory structure is intact - only the file is removed from the
> filesystem.
> More generally, for many frameworks, where there used to be
>
>
> the file is gone, but
>
>
> remains.
>
> I don't think we need a jtreg test, since any functionality associated
> with PCSC is broken on this platform.  I added label noreg-other
>
> -
>
> PR: https://git.openjdk.java.net/jdk/pull/2119
>

Re: RFR: 8252412: [macos11] system dynamic libraries removed from filesystem [v2]

[Martin Buchholz]

On Fri, 22 Jan 2021 19:46:13 GMT, Jiangli Zhou  wrote:

>> Martin Buchholz has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR.
>
> src/java.smartcardio/unix/classes/sun/security/smartcardio/PlatformPCSC.java 
> line 132:
> 
>> 130: // existence of the containing directory instead of the file.
>> 131: lib = PCSC_FRAMEWORK;
>> 132: if (new File(lib).getParentFile().isDirectory()) {
> 
> This is outside of my normal area, could you please explain why checking the 
> existence of the containing directory is equivalent to checking the file 
> here? Does it provide the expected behavior on all unix-like platforms or 
> only macos?
> 
> Could you please also provide a jtreg test for this change?

The directory structure is intact - only the file is removed from the 
filesystem.
More generally, for many frameworks, where there used to be


the file is gone, but 


remains.

I don't think we need a jtreg test, since any functionality associated with 
PCSC is broken on this platform.  I added label noreg-other

-

PR: https://git.openjdk.java.net/jdk/pull/2119

[jdk16] Integrated: 8259796: timed CompletableFuture.get may swallow InterruptedException

[Martin Buchholz]

On Sun, 17 Jan 2021 18:39:55 GMT, Martin Buchholz  wrote:

> 8259796: timed CompletableFuture.get may swallow InterruptedException

This pull request has now been integrated.

Changeset: f7b96d34
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk16/commit/f7b96d34
Stats: 88 lines in 2 files changed: 29 ins; 26 del; 33 mod

8259796: timed CompletableFuture.get may swallow InterruptedException

Reviewed-by: dl, alanb

-

PR: https://git.openjdk.java.net/jdk16/pull/126

Re: [jdk16] RFR: 8259796: timed CompletableFuture.get may swallow InterruptedException [v2]

[Martin Buchholz]

> 8259796: timed CompletableFuture.get may swallow InterruptedException

Martin Buchholz has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains one commit:

  JDK-8259796

-

Changes: https://git.openjdk.java.net/jdk16/pull/126/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk16=126=01
  Stats: 88 lines in 2 files changed: 29 ins; 26 del; 33 mod
  Patch: https://git.openjdk.java.net/jdk16/pull/126.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/126/head:pull/126

PR: https://git.openjdk.java.net/jdk16/pull/126

Re: [jdk16] RFR: 8259796: timed CompletableFuture.get may swallow InterruptedException [v2]

[Martin Buchholz]

On Sun, 17 Jan 2021 18:52:04 GMT, Doug Lea  wrote:

>> Martin Buchholz has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains one commit:
>> 
>>   JDK-8259796
>
> Marked as reviewed by dl (Reviewer).

we additionally tweaked timedGet with slightly cleaner code.

-

PR: https://git.openjdk.java.net/jdk16/pull/126

Re: RFR: 8252412: [macos11] system dynamic libraries removed from filesystem [v2]

[Martin Buchholz]

> 8252412: [macos11] system dynamic libraries removed from filesystem

Martin Buchholz has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  JDK-8252412: [macos11] system dynamic libraries removed from filesystem

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2119/files
  - new: https://git.openjdk.java.net/jdk/pull/2119/files/578329f0..85fdffde

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2119=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2119=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2119.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2119/head:pull/2119

PR: https://git.openjdk.java.net/jdk/pull/2119

RFR: 8252412: [macos11] File-based loading of dynamic libraries deprecated

[Martin Buchholz]

8252412: [macos11] File-based loading of dynamic libraries deprecated

-

Commit messages:
 - JDK-8252412: [macos11] File-based loading of dynamic libraries deprecated

Changes: https://git.openjdk.java.net/jdk/pull/2119/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2119=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8252412
  Stats: 19 lines in 1 file changed: 18 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2119.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2119/head:pull/2119

PR: https://git.openjdk.java.net/jdk/pull/2119

[jdk16] RFR: 8259796: timed CompletableFuture.get may swallow InterruptedException

[Martin Buchholz]

8259796: timed CompletableFuture.get may swallow InterruptedException

-

Commit messages:
 - JDK-8259796

Changes: https://git.openjdk.java.net/jdk16/pull/126/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk16=126=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8259796
  Stats: 87 lines in 2 files changed: 28 ins; 26 del; 33 mod
  Patch: https://git.openjdk.java.net/jdk16/pull/126.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/126/head:pull/126

PR: https://git.openjdk.java.net/jdk16/pull/126

Re: [jdk16] RFR: 8254350: CompletableFuture.get may swallow InterruptedException

[Martin Buchholz]

On Thu, 14 Jan 2021 13:59:50 GMT, Kezhu Wang 
 wrote:

>> 8254350: CompletableFuture.get may swallow InterruptedException
>
> @Martin-Buchholz @DougLea @AlanBateman Sorry for rough in.  After change 
> `future.get()` to `future.get(1, TimeUnit.DAYS)`, 
> `SwallowedInterruptedException` failed.

Thanks, Kezhu.  I filed:
https://bugs.openjdk.java.net/browse/JDK-8259796

-

PR: https://git.openjdk.java.net/jdk16/pull/17

Integrated: 8254973: CompletableFuture.ThreadPerTaskExecutor does not throw NPE in #execute

[Martin Buchholz]

On Sun, 10 Jan 2021 20:13:07 GMT, Martin Buchholz  wrote:

> 8254973: CompletableFuture.ThreadPerTaskExecutor does not throw NPE in 
> #execute

This pull request has now been integrated.

Changeset: 9154f643
Author:Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/9154f643
Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod

8254973: CompletableFuture.ThreadPerTaskExecutor does not throw NPE in #execute

Reviewed-by: dl

-

PR: https://git.openjdk.java.net/jdk/pull/2018

RFR: 8254973: CompletableFuture.ThreadPerTaskExecutor does not throw NPE in #execute

[Martin Buchholz]

8254973: CompletableFuture.ThreadPerTaskExecutor does not throw NPE in #execute

-

Commit messages:
 - JDK-8254973

Changes: https://git.openjdk.java.net/jdk/pull/2018/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2018=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8254973
  Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2018.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2018/head:pull/2018

PR: https://git.openjdk.java.net/jdk/pull/2018

Integrated: JDK-8258187 IllegalMonitorStateException in ArrayBlockingQueue

[Martin Buchholz]

On Sat, 9 Jan 2021 23:44:13 GMT, Martin Buchholz  wrote:

> JDK-8258187 IllegalMonitorStateException in ArrayBlockingQueue

This pull request has now been integrated.

Changeset: e7c17408
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/e7c17408
Stats: 30 lines in 2 files changed: 22 ins; 0 del; 8 mod

8258187: IllegalMonitorStateException in ArrayBlockingQueue

Reviewed-by: dl

-

PR: https://git.openjdk.java.net/jdk/pull/2015

Re: RFR: 8259518: Fixes for rare test failures due to 8246585: ForkJoin updates

[Martin Buchholz]

On Sun, 10 Jan 2021 12:01:44 GMT, Doug Lea  wrote:

>> 8259518: Fixes for rare test failures due to 8246585: ForkJoin updates
>
> Marked as reviewed by dl (Reviewer).

The code is good, but this is actually a fix for 
JDK-8258187 IllegalMonitorStateException in ArrayBlockingQueue
I will fiddle with the PR and JBS metadata.

-

PR: https://git.openjdk.java.net/jdk/pull/2015

Integrated: 8258217: PriorityBlockingQueue constructor spec and behavior mismatch

[Martin Buchholz]

On Sat, 9 Jan 2021 23:41:24 GMT, Martin Buchholz  wrote:

> 8258217: PriorityBlockingQueue constructor spec and behavior mismatch

This pull request has now been integrated.

Changeset: 11d5b047
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/11d5b047
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8258217: PriorityBlockingQueue constructor spec and behavior mismatch

Reviewed-by: dl

-

PR: https://git.openjdk.java.net/jdk/pull/2014

RFR: 8259518: Fixes for rare test failures due to 8246585: ForkJoin updates

[Martin Buchholz]

8259518: Fixes for rare test failures due to 8246585: ForkJoin updates

-

Commit messages:
 - JDK-8259518

Changes: https://git.openjdk.java.net/jdk/pull/2015/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2015=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8259518
  Stats: 30 lines in 2 files changed: 22 ins; 0 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2015.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2015/head:pull/2015

PR: https://git.openjdk.java.net/jdk/pull/2015

RFR: 8258217: PriorityBlockingQueue constructor spec and behavior mismatch

[Martin Buchholz]

8258217: PriorityBlockingQueue constructor spec and behavior mismatch

-

Commit messages:
 - JDK-8258217

Changes: https://git.openjdk.java.net/jdk/pull/2014/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2014=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8258217
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2014.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2014/head:pull/2014

PR: https://git.openjdk.java.net/jdk/pull/2014

Integrated: 8234131: Miscellaneous changes imported from jsr166 CVS 2021-01

[Martin Buchholz]

On Sun, 6 Dec 2020 02:57:03 GMT, Martin Buchholz  wrote:

> 8234131: Miscellaneous changes imported from jsr166 CVS 2021-01

This pull request has now been integrated.

Changeset: 270014ab
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/270014ab
Stats: 314 lines in 41 files changed: 107 ins; 41 del; 166 mod

8234131: Miscellaneous changes imported from jsr166 CVS 2021-01
8257671: ThreadPoolExecutor.Discard*Policy: rejected tasks are not cancelled

Reviewed-by: alanb, prappo, dl

-

PR: https://git.openjdk.java.net/jdk/pull/1647

Integrated: 8246677: LinkedTransferQueue and SynchronousQueue synchronization updates

[Martin Buchholz]

On Sun, 6 Dec 2020 02:56:04 GMT, Martin Buchholz  wrote:

> 8246677: LinkedTransferQueue and SynchronousQueue synchronization updates

This pull request has now been integrated.

Changeset: 63e3bd76
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/63e3bd76
Stats: 538 lines in 3 files changed: 143 ins; 294 del; 101 mod

8246677: LinkedTransferQueue and SynchronousQueue synchronization updates

Reviewed-by: alanb, dl

-

PR: https://git.openjdk.java.net/jdk/pull/1645

Integrated: 8246585: ForkJoin updates

[Martin Buchholz]

On Sun, 6 Dec 2020 02:56:34 GMT, Martin Buchholz  wrote:

> 8246585: ForkJoin updates

This pull request has now been integrated.

Changeset: 5cfa8c94
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/5cfa8c94
Stats: 3771 lines in 9 files changed: 1594 ins; 1097 del; 1080 mod

8246585: ForkJoin updates
8229253: forkjoin/FJExceptionTableLeak.java fails "AssertionError: failed to 
satisfy condition"

Reviewed-by: dl

-

PR: https://git.openjdk.java.net/jdk/pull/1646

Re: RFR: 8246585: ForkJoin updates [v5]

[Martin Buchholz]

> 8246585: ForkJoin updates

Martin Buchholz has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains one additional commit 
since the last revision:

  JDK-8246585

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1646/files
  - new: https://git.openjdk.java.net/jdk/pull/1646/files/b10694b5..cbd81886

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1646=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1646=03-04

  Stats: 20983 lines in 878 files changed: 3582 ins; 8199 del; 9202 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1646.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1646/head:pull/1646

PR: https://git.openjdk.java.net/jdk/pull/1646

Re: RFR: 8246585: ForkJoin updates [v4]

[Martin Buchholz]

> 8246585: ForkJoin updates

Martin Buchholz has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains one commit:

  JDK-8246585

-

Changes: https://git.openjdk.java.net/jdk/pull/1646/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1646=03
  Stats: 3580 lines in 9 files changed: 1603 ins; 893 del; 1084 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1646.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1646/head:pull/1646

PR: https://git.openjdk.java.net/jdk/pull/1646

Withdrawn: 8254350: CompletableFuture.get may swallow InterruptedException

[Martin Buchholz]

On Sun, 6 Dec 2020 22:12:54 GMT, Martin Buchholz  wrote:

> 8254350: CompletableFuture.get may swallow InterruptedException

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk/pull/1651

Re: RFR: 8254350: CompletableFuture.get may swallow InterruptedException [v2]

[Martin Buchholz]

On Tue, 8 Dec 2020 07:53:17 GMT, Alan Bateman  wrote:

>> Martin Buchholz has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR.
>
> Marked as reviewed by alanb (Reviewer).

This change went into jdk16 instead and was automatically forward-ported to 
jdk, so abandoning this PR.

-

PR: https://git.openjdk.java.net/jdk/pull/1651

Re: Impossible (?) code path resulting in IllegalStateException on jdk14+

[Martin Buchholz]

   1. JDK-8258187 

IllegalMonitorStateException in ArrayBlockingQueue

It's surprising that you can have a repro which is essentially just simple
producer-consumer.  Can we remove the forkjoinpool from the repro (just
threads?)

On Sun, Dec 13, 2020 at 6:28 PM David Holmes 
wrote:

> Hi Dawid,
>
> This appears to be a bug in AbstractQueuedSynchronizer and FJP
> interaction, so cc'ing core-libs as this is not a hotspot issue. Also
> cc'ing Martin and Doug as this is a j.u.c bug.
>
> Cheers,
> David
>
> On 12/12/2020 12:56 am, Dawid Weiss wrote:
> > So, I know what this is. Sort of.
> >
> > This isn't a compiler error (sigh of relief). It's a kind of interplay
> > between parallel streams (which uses the common ForkJoinPool) and the
> > queue's blocking operations.
> >
> > In our code streams use an op closure which sends items to an
> > ArrayBlockingQueue. This queue is being drained by a separate thread.
> > Everything works up until a certain moment when this happens on
> > queue.put() -- I've modified JDK 16 source code and recompiled it to
> > see what's happening:
> >
> > Suppressed: java.util.concurrent.RejectedExecutionException: Thread
> > limit exceeded replacing blocked worker
> > at
> java.base/java.util.concurrent.ForkJoinPool.tryCompensate(ForkJoinPool.java:1579)
> > at
> java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3124)
> > at
> java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(AbstractQueuedSynchronizer.java:1614)
> > at
> java.base/java.util.concurrent.ArrayBlockingQueue.put(ArrayBlockingQueue.java:371)
> >
> > This exception causes the try-finally block in ArrayBlockingQueue to
> > hit the finally block unexpectedly (without the attempt to re-acquire
> > the lock!), eventually leading to the original odd exception I
> > reported:
> >
> > Caused by: java.lang.IllegalMonitorStateException
> > at
> java.base/java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:175)
> > at
> java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1006)
> > at
> java.base/java.util.concurrent.locks.ReentrantLock.unlock(ReentrantLock.java:494)
> > at
> java.base/java.util.concurrent.ArrayBlockingQueue.put(ArrayBlockingQueue.java:378)
> >
> > This is then propagated up to the caller of queue.put()
> >
> > A full simplified repro (without streams) is here:
> > https://gist.github.com/dweiss/4787b7aa503ef5702e94d73178c3c842
> >
> > When you run it under JDK14+, it'll result in:
> >
> > java.lang.IllegalMonitorStateException
> >  at
> java.base/java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:175)
> > ...
> >
> > This code works on JDK11, by the way. What I find a bit
> > counterintuitive is that the original test (code) doesn't make any
> > explicit use of ForkJoinPool - it just collects items from a parallel
> > stream and involves throwing random exceptions from ops on that
> > stream... This was a bit unexpected.
> >
> > Dawid
> >
> >
> > On Thu, Dec 10, 2020 at 5:25 PM Dawid Weiss 
> wrote:
> >>
> >> Hello,
> >>
> >> I'm scratching my head again over a bug we encountered in randomized
> >> tests. The code is quite complex so before I start to try to isolate I
> >> thought I'd ask if it rings a bell for anybody.
> >>
> >> The bug is reproducible for certain seeds but happens only after some
> >> VM warmup - the same test is executed a few dozen times, then the
> >> problem starts showing up. This only happens on jdk 14 and jdk 15
> >> (didn't test on jdk 16 branch). Looks like something related to OSR/
> >> compilation races.
> >>
> >> In the end, we get this exception:
> >>
> >> java.lang.IllegalMonitorStateException
> >> at
> java.base/java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:175)
> >> at
> java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1006)
> >> at
> java.base/java.util.concurrent.locks.ReentrantLock.unlock(ReentrantLock.java:494)
> >> at
> java.base/java.util.concurrent.ArrayBlockingQueue.put(ArrayBlockingQueue.java:373)
> >> [stack omitted]
> >>
> >> This doesn't seem possible from Java code alone -- it's this snippet
> >> in ArrayBlockingQueue:
> >>
> >> lock.lockInterruptibly();
> >> try {
> >>  while (count == items.length)
> >>  notFull.await();
> >>  enqueue(e);
> >> } finally {
> >> lock.unlock(); // <<< bam...
> >> }
> >>
> >> If the code entered the lock-protected block it should never throw
> >> IllegalMonitorStateException, right?
> >>
> >> I'll start digging in spare moments but hints at how to isolate this/
> >> verify what this can be (other than bisecting git repo) would be very
> >> welcome!
> >>
> >> Dawid
>

Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v3]

[Martin Buchholz]

On Fri, 11 Dec 2020 15:07:45 GMT, Petr Janeček 
 wrote:

>> The changes to doc comments look good.
>> 
>> Thanks for incorporating my earlier code snippet suggestions published on 
>> [concurrency-interest](http://cs.oswego.edu/pipermail/concurrency-interest/2020-November/017264.html)!
>
> This also fixes issue 
> [JDK-8257671](https://bugs.openjdk.java.net/browse/JDK-8257671): 
> _ThreadPoolExecutor.Discard*Policy: rejected tasks are not cancelled_, by 
> updating the `TPE.Discard*Policy` JavaDocs.
> 
> I filed it days before the code went in as I did not see the JavaDoc update 
> in JDK, sorry for my impatience and thank you for the fix. I cannot update 
> the Jira, please link it to this PR, too.

@JanecekPetr Thank you very much.  I've updated JIRA and this pull request for 
JDK-8257671

-

PR: https://git.openjdk.java.net/jdk/pull/1647

Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v4]

[Martin Buchholz]

> 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12

Martin Buchholz has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains one commit:

  JDK-8234131

-

Changes: https://git.openjdk.java.net/jdk/pull/1647/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1647=03
  Stats: 314 lines in 41 files changed: 107 ins; 41 del; 166 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1647.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1647/head:pull/1647

PR: https://git.openjdk.java.net/jdk/pull/1647

[jdk16] Integrated: 8254350: CompletableFuture.get may swallow InterruptedException

[Martin Buchholz]

On Sun, 13 Dec 2020 03:31:44 GMT, Martin Buchholz  wrote:

> 8254350: CompletableFuture.get may swallow InterruptedException

This pull request has now been integrated.

Changeset: 43dc3f79
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk16/commit/43dc3f79
Stats: 178 lines in 3 files changed: 170 ins; 5 del; 3 mod

8254350: CompletableFuture.get may swallow InterruptedException

Reviewed-by: alanb, dl

-

PR: https://git.openjdk.java.net/jdk16/pull/17

[jdk16] RFR: 8254350: CompletableFuture.get may swallow InterruptedException

[Martin Buchholz]

8254350: CompletableFuture.get may swallow InterruptedException

-

Commit messages:
 - JDK-8254350

Changes: https://git.openjdk.java.net/jdk16/pull/17/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk16=17=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8254350
  Stats: 178 lines in 3 files changed: 170 ins; 5 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk16/pull/17.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/17/head:pull/17

PR: https://git.openjdk.java.net/jdk16/pull/17

Re: RFR: 8247402: Documentation for Map::compute contains confusing implementation requirements [v2]

[Martin Buchholz]

On Sat, 28 Nov 2020 08:35:20 GMT, John Lin 
 wrote:

>> This is from the mailing list: 
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2020-June/067190.html
>> 
>> -
>> ### Progress
>> - [x] Change must not contain extraneous whitespace
>> - [x] Commit message must refer to an issue
>> - [ ] Change must be properly reviewed
>> 
>> ### Testing
>> 
>> | | Linux x64 | Windows x64 | macOS x64 |
>> | --- | - | - | - |
>> | Build | ✔️ (5/5 passed) | ✔️ (2/2 passed) | ✔️ (2/2 passed) |
>> | Test (tier1) | ✔️ (9/9 passed) | ✔️ (9/9 passed) | ✔️ (9/9 passed) |
>> 
>> 
>> 
>> ### Download
>> `$ git fetch https://git.openjdk.java.net/jdk pull/714/head:pull/714`
>> `$ git checkout pull/714`
>
> John Lin has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains one commit:
> 
>   8247402: Documentation for Map::compute contains confusing implementation 
> requirements

Marked as reviewed by martin (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/714

Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v3]

[Martin Buchholz]

Thanks, Pavel!

I committed this to CVS, but am not planning to include it in this
integration:
(not sure if re-generating will invalidate my "Ready" status)
(Having to cast to Future makes this more problematic, but in typical
usages the user will know that the queue contains Futures)

--- src/main/java/util/concurrent/ThreadPoolExecutor.java 8 Dec 2020
20:33:21 - 1.195
+++ src/main/java/util/concurrent/ThreadPoolExecutor.java 9 Dec 2020
22:05:21 - 1.197
@@ -2066,10 +2066,12 @@
  *  {@code
  * new RejectedExecutionHandler() {
  *   public void rejectedExecution(Runnable r, ThreadPoolExecutor e) {
- * Future dropped = e.getQueue().poll();
- * if (dropped != null)
- *dropped.cancel(false); // also consider logging the failure
- * e.execute(r); // retry
+ * Runnable dropped = e.getQueue().poll();
+ * if (dropped instanceof Future) {
+ *   ((Future)dropped).cancel(false);
+ *   // also consider logging the failure
+ * }
+ * e.execute(r);  // retry
  * }}}
  */
 public static class DiscardOldestPolicy implements
RejectedExecutionHandler {

On Wed, Dec 9, 2020 at 10:46 AM Pavel Rappo  wrote:

> On Tue, 8 Dec 2020 21:15:48 GMT, Martin Buchholz 
> wrote:
>
> >> 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12
> >
> > Martin Buchholz has updated the pull request with a new target base due
> to a merge or a rebase. The pull request now contains one commit:
> >
> >   JDK-8234131
>
> The changes to doc comments look good.
>
> Thanks for incorporating my earlier code snippet suggestions published on
> [concurrency-interest](
> http://cs.oswego.edu/pipermail/concurrency-interest/2020-November/017264.html
> )!
>
> src/java.base/share/classes/java/util/concurrent/ThreadPoolExecutor.java
> line 2102:
>
> > 2100:  *dropped.cancel(false); // also consider logging the
> failure
> > 2101:  * e.execute(r); // retry
> > 2102:  * }}}
>
> I tried to reify that code snippet:
>
> $ cat MySnippet.java
> import java.util.concurrent.Future;
> import java.util.concurrent.RejectedExecutionHandler;
> import java.util.concurrent.ThreadPoolExecutor;
>
> public class MySnippet {
> public static void main(String[] args) {
> new RejectedExecutionHandler() {
> public void rejectedExecution(Runnable r, ThreadPoolExecutor
> e) {
> Future dropped = e.getQueue().poll();
> if (dropped != null)
> dropped.cancel(false); // also consider logging the
> failure
> e.execute(r);  // retry
> }
> };
> }
> }
> $ javac MySnippet.java
> MySnippet.java:9: error: incompatible types: Runnable cannot be converted
> to Future
> Future dropped = e.getQueue().poll();
>  ^
> 1 error
> $
> Separately, it seems that the `if` statement uses a 3-space indent which
> is surprising to see in the JSR 166 code base.
>
> -
>
> Marked as reviewed by prappo (Reviewer).
>
> PR: https://git.openjdk.java.net/jdk/pull/1647
>

Re: RFR: 8246585: ForkJoin updates [v3]

[Martin Buchholz]

> 8246585: ForkJoin updates

Martin Buchholz has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains one commit:

  JDK-8246585

-

Changes: https://git.openjdk.java.net/jdk/pull/1646/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1646=02
  Stats: 3291 lines in 7 files changed: 1312 ins; 890 del; 1089 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1646.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1646/head:pull/1646

PR: https://git.openjdk.java.net/jdk/pull/1646

Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v3]

[Martin Buchholz]

> 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12

Martin Buchholz has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains one commit:

  JDK-8234131

-

Changes: https://git.openjdk.java.net/jdk/pull/1647/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1647=02
  Stats: 312 lines in 41 files changed: 105 ins; 41 del; 166 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1647.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1647/head:pull/1647

PR: https://git.openjdk.java.net/jdk/pull/1647

Re: RFR: 8254350: CompletableFuture.get may swallow InterruptedException [v2]

[Martin Buchholz]

On Tue, Dec 8, 2020 at 3:54 AM Daniel Fuchs  wrote:

>
> > RandomFactory is probably overkill here as the test just needs to
> exercise untimed and timed get. So just a random boolean rather than a wide
> range of random values.
>
> OK.
>

There's a conflict between the desires to do more thorough testing and
avoid flaky intermittent failures.

Especially in j.u.concurrent we embrace test non-determinism, much of it
comes from the concurrency we're testing, and retrieving a random seed for
reproducibility is not worth the effort.

No one does a good job of race prevention through testing.

Re: RFR: 8254350: CompletableFuture.get may swallow InterruptedException [v2]

[Martin Buchholz]

On Mon, Dec 7, 2020 at 11:56 PM Alan Bateman  wrote:

>
> > 37: // TODO: Rewrite as a CompletableFuture tck test ?
>
> Do you want the  "TODO" in the commit?
>
> Yes!

Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v2]

[Martin Buchholz]

OK, rollback committed to CVS:

--- src/main/java/util/concurrent/ThreadPoolExecutor.java 27 Nov 2020
17:42:00 - 1.194
+++ src/main/java/util/concurrent/ThreadPoolExecutor.java 8 Dec 2020
20:31:54 -
@@ -1522,13 +1522,11 @@
 // As a heuristic, prestart enough new workers (up to new
 // core size) to handle the current number of tasks in
 // queue, but stop if queue becomes empty while doing so.
-/*
 int k = Math.min(delta, workQueue.size());
 while (k-- > 0 && addWorker(null, true)) {
 if (workQueue.isEmpty())
 break;
 }
-*/
 }
 }

On Tue, Dec 8, 2020 at 4:04 AM Doug Lea  wrote:

>
> On 12/8/20 3:56 AM, Alan Bateman wrote:
> >> 1558: break;
> >> 1559: }
> >> 1560: */
> > Is this meant to be commented out?
> Yes, but It should be marked as a possible improvement, not yet
> committed. While this (prestarting) would improve performance in some
> scenarios, it may also disrupt expectations and even tooling in some
> existing usages, which we haven't fully checked out.
>
>

Re: RFR: 8246585: ForkJoin updates

[Martin Buchholz]

On Tue, Dec 8, 2020 at 12:05 AM Aleksey Shipilev 
wrote:

> On Sun, 6 Dec 2020 02:56:34 GMT, Martin Buchholz 
> wrote:
>
> > 8246585: ForkJoin updates
>
> It would be nice to get it tested with GH Actions. "Checks" tabs should
> have those testing results automatically on push, but it is empty. Probably
> because your fork is not configured for it. Please go to
> https://github.com/Martin-Buchholz/jdk/actions -- and see if it says
> anything suspicious? Triggering the (only) workflow manually against your
> branch would help too.
>

Thanks for the handholding.  I visited
https://github.com/Martin-Buchholz/jdk/actions and it said

Workflows aren't being run on this forked repository

Because this repository contained workflow files when it was
forked, we have disabled them from running on this fork. Make sure
you understand the configured workflows and their expected usage
before enabling Actions on this repository.

They're now enabled. Do the skara docs need to clarify this?

Re: RFR: 8254350: CompletableFuture.get may swallow InterruptedException [v2]

[Martin Buchholz]

> 8254350: CompletableFuture.get may swallow InterruptedException

Martin Buchholz has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  JDK-8254350

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1651/files
  - new: https://git.openjdk.java.net/jdk/pull/1651/files/4e32f8c1..762a1715

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1651=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1651=00-01

  Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1651.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1651/head:pull/1651

PR: https://git.openjdk.java.net/jdk/pull/1651

Re: RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12 [v2]

[Martin Buchholz]

> 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12

Martin Buchholz has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  JDK-8234131

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1647/files
  - new: https://git.openjdk.java.net/jdk/pull/1647/files/ed43b3fe..a6d85863

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1647=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1647=00-01

  Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1647.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1647/head:pull/1647

PR: https://git.openjdk.java.net/jdk/pull/1647

Re: RFR: 8246585: ForkJoin updates [v2]

[Martin Buchholz]

> 8246585: ForkJoin updates

Martin Buchholz has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  JDK-8246585

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1646/files
  - new: https://git.openjdk.java.net/jdk/pull/1646/files/0ae46847..d025f68b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1646=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1646=00-01

  Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1646.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1646/head:pull/1646

PR: https://git.openjdk.java.net/jdk/pull/1646

Re: RFR: 8246677: LinkedTransferQueue and SynchronousQueue synchronization updates [v2]

[Martin Buchholz]

> 8246677: LinkedTransferQueue and SynchronousQueue synchronization updates

Martin Buchholz has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  JDK-8246677

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1645/files
  - new: https://git.openjdk.java.net/jdk/pull/1645/files/13eb0fa8..b8baa921

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1645=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1645=00-01

  Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1645.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1645/head:pull/1645

PR: https://git.openjdk.java.net/jdk/pull/1645

RFR: 8254350: CompletableFuture.get may swallow InterruptedException

[Martin Buchholz]

8254350: CompletableFuture.get may swallow InterruptedException

-

Commit messages:
 - JDK-8254350

Changes: https://git.openjdk.java.net/jdk/pull/1651/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1651=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8254350
  Stats: 178 lines in 3 files changed: 170 ins; 5 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1651.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1651/head:pull/1651

PR: https://git.openjdk.java.net/jdk/pull/1651

RFR: 8246585: ForkJoin updates

[Martin Buchholz]

8246585: ForkJoin updates

-

Commit messages:
 - JDK-8246585

Changes: https://git.openjdk.java.net/jdk/pull/1646/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1646=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8246585
  Stats: 3114 lines in 6 files changed: 1135 ins; 890 del; 1089 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1646.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1646/head:pull/1646

PR: https://git.openjdk.java.net/jdk/pull/1646

RFR: 8246677: LinkedTransferQueue and SynchronousQueue synchronization updates

[Martin Buchholz]

8246677: LinkedTransferQueue and SynchronousQueue synchronization updates

-

Commit messages:
 - JDK-8246677

Changes: https://git.openjdk.java.net/jdk/pull/1645/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1645=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8246677
  Stats: 538 lines in 3 files changed: 143 ins; 294 del; 101 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1645.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1645/head:pull/1645

PR: https://git.openjdk.java.net/jdk/pull/1645

RFR: 8234131: Miscellaneous changes imported from jsr166 CVS 2020-12

[Martin Buchholz]

8234131: Miscellaneous changes imported from jsr166 CVS 2020-12

-

Commit messages:
 - JDK-8234131

Changes: https://git.openjdk.java.net/jdk/pull/1647/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1647=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8234131
  Stats: 314 lines in 41 files changed: 107 ins; 41 del; 166 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1647.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1647/head:pull/1647

PR: https://git.openjdk.java.net/jdk/pull/1647

Integrated: 8243614: Typo in ReentrantLock's Javadoc

[Martin Buchholz]

On Fri, 4 Dec 2020 22:06:03 GMT, Martin Buchholz  wrote:

> Add missing semicolon
> 
> Martin's first github pr.

This pull request has now been integrated.

Changeset: c4339c30
Author:    Martin Buchholz 
URL:   https://git.openjdk.java.net/jdk/commit/c4339c30
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8243614: Typo in ReentrantLock's Javadoc

Reviewed-by: dholmes, alanb

-

PR: https://git.openjdk.java.net/jdk/pull/1633

RFR: 8243614: Typo in ReentrantLock's Javadoc

[Martin Buchholz]

Add missing semicolon

Martin's first github pr.

-

Commit messages:
 - JDK-8243614-typo

Changes: https://git.openjdk.java.net/jdk/pull/1633/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1633=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8243614
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1633.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1633/head:pull/1633

PR: https://git.openjdk.java.net/jdk/pull/1633

Re: RFR: 8247402: Documentation for Map::compute contains confusing implementation requirements

[Martin Buchholz]

On Fri, 4 Dec 2020 16:54:26 GMT, Pavel Rappo  wrote:

>> @pavelrappo Please see my updated CSR below. Thanks.
>> 
>> # Map::compute should have the implementation requirement match its default 
>> implementation
>> 
>> ## Summary
>> 
>> The implementation requirement of Map::compute does not match its default 
>> implementation. Besides, it has some other minor issues. We should fix it.
>> 
>> ## Problem
>> 
>> The documentation of the implementation requirements for Map::compute has 
>> the following problems:
>> 1. It doesn't match its default implementation.
>> 1. It lacks of the return statements for most of the if-else cases.
>> 1. The indents are 3 spaces, while the convention is 4 spaces.
>> 1. The if-else is overly complicated and can be simplified.
>> 1. The surrounding prose contains incorrect statements.
>> 
>> ## Solution
>> 
>> Rewrite the documentation of Map::compute to match its default 
>> implementation and solve the above mentioned problems.
>> 
>> ## Specification
>> 
>> diff --git a/src/java.base/share/classes/java/util/Map.java 
>> b/src/java.base/share/classes/java/util/Map.java
>> index b1de34b42a5..b30e3979259 100644
>> --- a/src/java.base/share/classes/java/util/Map.java
>> +++ b/src/java.base/share/classes/java/util/Map.java
>> @@ -1107,23 +1107,17 @@ public interface Map {
>>   *
>>   * @implSpec
>>   * The default implementation is equivalent to performing the following
>> - * steps for this {@code map}, then returning the current value or
>> - * {@code null} if absent:
>> + * steps for this {@code map}:
>>   *
>>   *  {@code
>>   * V oldValue = map.get(key);
>>   * V newValue = remappingFunction.apply(key, oldValue);
>> - * if (oldValue != null) {
>> - *if (newValue != null)
>> - *   map.put(key, newValue);
>> - *else
>> - *   map.remove(key);
>> - * } else {
>> - *if (newValue != null)
>> - *   map.put(key, newValue);
>> - *else
>> - *   return null;
>> + * if (newValue != null) {
>> + * map.put(key, newValue);
>> + * } else if (oldValue != null || map.containsKey(key)) {
>> + * map.remove(key);
>>   * }
>> + * return newValue;
>>   * }
>>   *
>>   * The default implementation makes no guarantees about detecting if 
>> the
>
> 1. @johnlinp I've created the CSR: 
> https://bugs.openjdk.java.net/browse/JDK-8257768
> 2. @dfuch, @stuart-marks, and 
> I-cannot-seem-to-find-Martin-Buchholz's-GitHub-username: care to review that 
> CSR?

Hello github - this is my first ever comment.

The javadoc specs for the various compute methods in all the Map classes should 
be maintained consistently and holistically.  Sorry for not having done so 
years ago.

Pavel is thinking about code snippets for all of OpenJDK today, while I have 
been thinking about code snippets in jsr166.  jsr166 code snippets have a 
consistent style that should also be used for Map.java, but we've had a mild 
case of maintainer style divergence.

-

PR: https://git.openjdk.java.net/jdk/pull/714

Re: RFR 8252537: Replace @exception with @throws for core-libs

[Martin Buchholz]

Thanks for doing this!

15 years ago I considered taking on this task, eventually backing away
because it was too much work (!).  But I made sure most of the classes
I maintained were using @throws.

When considering this, I thought that tidying the whitespace after
conversion would be a big part of the task, even though it is
technically non-essential.  Current use of whitespace with @throws is
inconsistent, as you have seen.

On Thu, Sep 3, 2020 at 12:34 PM Vipin Sharma  wrote:
>
> Hi,
>
> Please review and sponsor the fix for replacing @exception with @throws for
> core-libs.
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8252537
> Webrev: https://cr.openjdk.java.net/~vsharma/8252537
>
> As suggested in the previous discussion
> ,
> this webrev has a consolidated fix for all subtasks of the JDK-8252536
> .
> There is no change in the indentation as part of this webrev.
>
> Regards,
> Vipin

Re: JDK 16 RFR of JDK-8250240: Address use of default constructors in the java.util.concurrent

[Martin Buchholz]

I'm happy with this change whether or not the slightly more evocative
"protected" is used.

On Thu, Jul 23, 2020 at 5:43 PM Joe Darcy  wrote:
>
> Hi Martin,
>
> On 7/23/2020 5:24 PM, Martin Buchholz wrote:
> > So these are all abstract classes where the constructor can only be
> > called via super() ?
>
> Yep.
>
> > In which case one would expect the constructors to be protected, not public.
> > But I'm probably missing some reason why "protected" would not be 100%
> > compatible.
>
> It would be compatible (AFAICT), but the current (implicit) default
> constructors are public, since the classes are public, so I made the new
> explicit constructors public.
>
> On the assumption the upstream JDK 166-alpha repo would want to take in
> this change for as many releases as possible, the public constructors
> could be used for earlier release trains too.
>
> > Historically, we've preferred to put changes in via CVS, but in 2020,
> > we might prefer you make the change directly in openjdk.
> > Doug?
> >
> I'm happy to make the changes in OpenJDK for JDK 16.
>
> Thanks,
>
> -Joe
>

Re: JDK 16 RFR of JDK-8250240: Address use of default constructors in the java.util.concurrent

[Martin Buchholz]

So these are all abstract classes where the constructor can only be
called via super() ?
In which case one would expect the constructors to be protected, not public.
But I'm probably missing some reason why "protected" would not be 100%
compatible.

Historically, we've preferred to put changes in via CVS, but in 2020,
we might prefer you make the change directly in openjdk.
Doug?

On Thu, Jul 23, 2020 at 3:04 PM Joe Darcy  wrote:
>
> Hello,
>
> Martin, how would you prefer these changes, or equivalent ones, to get
> into the java.util.concurrent upstream sources?
>
> Several classes in java.util.concurrent rely on default constructors,
> which is not recommended.  Please review the patch below which removes
> them along with the corresponding CSR
> (https://bugs.openjdk.java.net/browse/JDK-8250241); webrev at
> http://cr.openjdk.java.net/~darcy/8250240.0/.
>
> Thanks,
>
> -Joe
>
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -77,6 +77,11 @@
>   public abstract class AbstractExecutorService implements ExecutorService {
>
>   /**
> + * Constructor for subclasses to call.
> + */
> +public AbstractExecutorService() {}
> +
> +/**
>* Returns a {@code RunnableFuture} for the given runnable and default
>* value.
>*
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
> --- a/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
> Thu Jul 23 20:25:41 2020 +0100
> +++ b/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -242,6 +242,11 @@
>   private static final int SIGNAL   = 1 << 16; // true if joiner waiting
>   private static final int SMASK= 0x;  // short bits for tags
>
> +/**
> + * Constructor for subclasses to call.
> + */
> +public ForkJoinTask() {}
> +
>   static boolean isExceptionalStatus(int s) {  // needed by subclasses
>   return (s & THROWN) != 0;
>   }
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -166,6 +166,11 @@
>   private static final long serialVersionUID = 5232453952276485070L;
>
>   /**
> + * Constructor for subclasses to call.
> + */
> +public RecursiveAction() {}
> +
> +/**
>* The main computation performed by this task.
>*/
>   protected abstract void compute();
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -69,6 +69,11 @@
>   private static final long serialVersionUID = 5232453952276485270L;
>
>   /**
> + * Constructor for subclasses to call.
> + */
> +public RecursiveTask() {}
> +
> +/**
>* The result of the computation.
>*/
>   @SuppressWarnings("serial") // Conditionally serializable
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -65,6 +65,11 @@
>
>   private static final long serialVersionUID = 7373984972572414692L;
>
> +/**
> + * Constructor for subclasses to call.
> + */
> +public AbstractQueuedLongSynchronizer() {}
> +
>   /*
>* To keep sources in sync, the remainder of this source file is
>* exactly cloned from AbstractQueuedSynchronizer, replacing class
>

Re: RFR 8249217: Unexpected StackOverflowError in "process reaper" thread still happens

[Martin Buchholz]

+1

On Thu, Jul 23, 2020 at 3:50 PM David Holmes  wrote:
>
> Still good.
>
> Thanks,
> David
>
> On 24/07/2020 12:11 am, Roger Riggs wrote:
> > Webrev updated with Martin's suggestion:
> >
> > http://cr.openjdk.java.net/~rriggs/webrev-stackoverflow-8249217-2/
> >
> > Thanks, Roger
> >
> >
> > On 7/22/20 8:35 PM, Martin Buchholz wrote:
> >> Roger: You're absolutely right!  I should have looked.
> >>
> >> On Wed, Jul 22, 2020 at 5:25 PM Roger Riggs 
> >> wrote:
> >>> Hi Martin,
> >>>
> >>> That's a good idea, but unless I miss your point, there is no Reaper
> >>> class,
> >>> only a lambda that is used as to create the Executor for the reaper
> >>> thread pool.
> >>>
> >>> Do you mean to put it before or after lines 91-93?  [1]
> >>>
> >>> Thanks, Roger
> >>>
> >>> [1]
> >>> http://cr.openjdk.java.net/~rriggs/webrev-stackoverflow-8249217/src/java.base/share/classes/java/lang/ProcessHandleImpl.java.sdiff.html
> >>>
> >>>
> >>>
> >>> On 7/22/20 8:06 PM, Martin Buchholz wrote:
> >>>> Looks good.
> >>>>
> >>>> I might move the static class preloading into a static method of the
> >>>> reaper thread class, to make it clearly the responsibility of the
> >>>> reaper thread class to enumerate and pre-load its dependencies.
> >>>>
> >>>> On Wed, Jul 22, 2020 at 7:59 AM Peter Levart
> >>>>  wrote:
> >>>>> Hi Roger,
> >>>>>
> >>>>>
> >>>>> I don't think resolving the ConcurrentHashMap.class ensures
> >>>>> initialization of the class. Just loading. But I think CHM is already
> >>>>> initialized at that time as it is used in ClassLoader to maintain
> >>>>> class
> >>>>> loading locks (per class name).
> >>>>>
> >>>>>
> >>>>> Regards, Peter
> >>>>>
> >>>>>
> >>>>> On 7/22/20 4:51 PM, Roger Riggs wrote:
> >>>>>> Please review a change to the Process reaper thread initialization to
> >>>>>> preemptively
> >>>>>> make sure classes ThreadLocalRandom and ConcurrentHashMap are
> >>>>>> initialized.
> >>>>>>   From the stack overflow failures, it seems that the classes have
> >>>>>> not
> >>>>>> been initialized
> >>>>>> before they are used during processing the termination of a process.
> >>>>>> When the initialization is performed on the smaller reaper stack, it
> >>>>>> occasionally
> >>>>>> exceeds the available stack.
> >>>>>>
> >>>>>> As an aid to diagnostics,
> >>>>>> -XX:AbortVMOnException=java.lang.StackOverflowError
> >>>>>> is added to TestHumongousNonArrayAllocation that has failed
> >>>>>> intermittently.
> >>>>>> If the problem happens again it will produce an hs_error file with
> >>>>>> useful details
> >>>>>> and will otherwise not change the test behavior.
> >>>>>>
> >>>>>> Webrev:
> >>>>>> http://cr.openjdk.java.net/~rriggs/webrev-stackoverflow-8249217/
> >>>>>>
> >>>>>> Issue:
> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8249217
> >>>>>>
> >>>>>> Thanks, Roger
> >>>>>>
> >