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