On Tue, 25 Mar 2025 19:57:54 GMT, Doug Lea <d...@openjdk.org> wrote:

>> (Copied from https://bugs.openjdk.org/browse/JDK-8319447)
>> 
>> The problems addressed by this CR/PR are that ScheduledThreadPoolExecutor is 
>> both ill-suited for many (if not most) of its applications, and is a 
>> performance bottleneck (as seen especially in Loom and CompletableFuture 
>> usages). After considering many options over the years, the approach taken 
>> here is to connect (lazily, only if used) a form of ScheduledExecutorService 
>> (DelayScheduler) to any ForkJoinPool (including the commonPool), which can 
>> then use more efficient and scalable techniques to request and trigger 
>> delayed actions, periodic actions, and cancellations, as well as coordinate 
>> shutdown and termination mechanics (see the internal documentation in 
>> DelayScheduler.java for algotihmic details). This speeds up some Loom 
>> operations by almost an order of magnitude (and similarly for 
>> CompletableFuture). Further incremental improvements may be possible, but 
>> delay scheduling overhead is now unlikely to be a common performance concern.
>> 
>> We also introduce method submitWithTimeout to schedule a timeout that 
>> cancels or otherwise completes a submitted task that takes too long. Support 
>> for this very common usage was missing from the ScheduledExecutorService 
>> API, and workarounds that users have tried are wasteful, often leaky, and 
>> error-prone. This cannot be added to the ScheduledExecutorService interface 
>> because it relies on ForkJoinTask methods (such as completeExceptionally) to 
>> be available in user-supplied timeout actions. The need to allow a pluggable 
>> handler reflects experience with the similar CompletableFuture.orTimeout, 
>> which users have found not to be flexible enough, so might be subject of 
>> future improvements.
>> 
>> A DelayScheduler is optionally (on first use of a scheduling method) 
>> constructed and started as part of a ForkJoinPool, not any other kind of 
>> ExecutorService. It doesn't make sense to do so with the other j.u.c pool 
>> implementation ThreadPoolExecutor. ScheduledThreadPoolExecutor already 
>> extends it in incompatible ways (which is why we can't just improve or 
>> replace STPE internals). However, as discussed in internal documentation, 
>> the implementation isolates calls and callbacks in a way that could be 
>> extracted out into (package-private) interfaces if another j.u.c pool type 
>> is introduced.
>> 
>> Only one of the policy controls in ScheduledThreadPoolExecutor applies to 
>> ForkJoinPools with DelaySchedulers: new method cancelDelayedTasksOnShutdown 
>> controls whether quiescent shutdown sh...
>
> Doug Lea has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 49 commits:
> 
>  - Merge branch 'openjdk:master' into JDK-8319447
>  - Address review comments
>  - Merge branch 'openjdk:master' into JDK-8319447
>  - Match indent of naster changes
>  - Use TC_MASK in accord with https://bugs.openjdk.org/browse/JDK-8330017 
> (Unnecessarily for now.)
>  - Reword javadoc
>  - Use SharedSecrets for ThreadLocalRandomProbe; other tweaks
>  - Disambiguate caller-runs vs Interruptible
>  - Merge branch 'openjdk:master' into JDK-8319447
>  - Associate probes with carriers if Virtual (no doc updates yet)
>  - ... and 39 more: https://git.openjdk.org/jdk/compare/dbc620fb...4aabe6b0

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

> 3557:         return scheduleDelayedTask(
> 3558:             new ScheduledForkJoinTask<V>(
> 3559:                 unit.toNanos(delay), 0L, false, null,

Suggestion:

                unit.toNanos(delay), 0L, false, null, // implicit null check of 
unit

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

> 3610:         return scheduleDelayedTask(
> 3611:             new ScheduledForkJoinTask<Void>(
> 3612:                 unit.toNanos(initialDelay),

Suggestion:

                unit.toNanos(initialDelay), // implicit null check of unit

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

> 3659:         return scheduleDelayedTask(
> 3660:             new ScheduledForkJoinTask<Void>(
> 3661:                 unit.toNanos(initialDelay),

Suggestion:

                unit.toNanos(initialDelay), // implicit null check of unit

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

> 3722:         ScheduledForkJoinTask<Void> timeoutTask =
> 3723:             new ScheduledForkJoinTask<Void>(
> 3724:                 unit.toNanos(timeout), 0L, true,

Suggestion:

                unit.toNanos(timeout), 0L, true, // implicit null check of unit

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23702#discussion_r2014102436
PR Review Comment: https://git.openjdk.org/jdk/pull/23702#discussion_r2014103647
PR Review Comment: https://git.openjdk.org/jdk/pull/23702#discussion_r2014104126
PR Review Comment: https://git.openjdk.org/jdk/pull/23702#discussion_r2014105533

Reply via email to