On Sat, 22 Mar 2025 11:09:03 GMT, Doug Lea <[email protected]> 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 incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains 47 additional commits since
> the last revision:
>
> - 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)
> - Reduce volatile reads
> - Address review comments; reactivation tweak
> - ... and 37 more: https://git.openjdk.org/jdk/compare/2920e212...b552c225
test/jdk/java/util/concurrent/tck/ForkJoinPool20Test.java line 414:
> 412: final CountDownLatch done = new CountDownLatch(cycles);
> 413: final Runnable task = new CheckedRunnable() {
> 414: public void realRun() { done.countDown(); }};
This does not throw if the task executes *too many times*—maybe it should?
test/jdk/java/util/concurrent/tck/ForkJoinPool20Test.java line 456:
> 454: if (elapsedMillis >= 2 * d)
> 455: tryLongerDelay.set(true);
> 456: }
Suggestion:
if (elapsedMillis >= (done.getCount() == cycles ? d
: 2 * d))
tryLongerDelay.set(true);
test/jdk/java/util/concurrent/tck/ForkJoinPool20Test.java line 458:
> 456: }
> 457: previous.set(now);
> 458: done.countDown();
Should we test so that we don't count down below 0?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23702#discussion_r2012507729
PR Review Comment: https://git.openjdk.org/jdk/pull/23702#discussion_r2012511274
PR Review Comment: https://git.openjdk.org/jdk/pull/23702#discussion_r2012512697
