Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v4]
On Sun, 8 May 2022 01:51:17 GMT, Martin Buchholz wrote:
>> 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);
> }
> }
@Martin-Buchholz thanks for test improvements. I'm leaving further FJP tweaks
for some other time to avoid possibility of problems after already merging with
loom commit
-
PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v4]
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]
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]
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 [v4]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8577/files - new: https://git.openjdk.java.net/jdk/pull/8577/files/c276385c..9a0d27f4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8577=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8577=02-03 Stats: 21 lines in 1 file changed: 12 ins; 4 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/8577.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8577/head:pull/8577 PR: https://git.openjdk.java.net/jdk/pull/8577
