(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 should wait for delayed tasks to become 
enabled and execute, or to cancel them and terminate. The default (to wait) 
matches default settings of STPE. Also new method getDelayedTaskCount allows 
monitoring.

We don't expect any compatibility issues: In the unlikely event that someone 
else has added the four SES methods to a ForkJoinPool subclass, they will 
continue to work as overrides. It's hard to imagine that anyone has added a 
form of submitWithTimeout with the same signature (Callable<V> callable, long 
timeout, TimeUnit unit, Consumer<ForkJoinTask<V>> timeoutAction), or methods 
with names cancelDelayedTasksOnShutdown or getDelayedTaskCount.

A snag: for reasons that should now be deprecated, ForkJoinPool allow users to 
externally (via properties) set the commonPool to zero parallelism, disabling 
worker thread creation, which makes no sense if a DelayScheduler is used. This 
property setting was made available to JavaEE frameworks so they could ensure 
that no new threads would be created in service of parallelStream operations 
(which are structured to be (slowly) executable single-threadedly via caller 
joins). However, no other async uses would work, which led to workarounds in 
CompletableFuture (also SubmissionPublisher) to handle this case by generating 
other threads. It was another arguably wrong decision to do this as well. A 
better solution that is backward compatible is to internally override 
commonPool parallelism zero only if known-async methods are used. This 
preserves original intent, and passes jtreg/tck tests that check for lack of 
thread creation in parallelStreams and related usages that don't otherwise use a
 ny async j.u.c components (although with some changes in unnecessarily 
implementation-dependent tests that made assumptions about exactly which 
threads/pools were used). It does require some changes in the wording of 
disclaimers in CompletableFuture and elsewhere though. And eventually, all this 
should go away, although it's not clear know how to deprecate a property 
setting. For now, the class-level javadoc has been updated to discourage use.

One remaining issue is whether to expose the underlying ScheduledForkJoinTask 
type. It currently isn't, in part because it would also require exposing 
currently non-public intervening classes (mainly FJT.InterruptibleTask). The 
main disadvantage with not exposing is that the schedule methods merely 
document that the returned ScheduledFuture is a ForkJoinTask rather than 
include this in signature. This could be revisited in the future without 
introducing incompatibilities (but with some internal implementation challenges 
to remove reliance on non-public-ness).

As minor follow-ups, we might expand use of DelaySchedulers internally in j.u.c 
classes to replace some timed waits.

[Edit](https://bugs.openjdk.org/secure/EditComment!default.jspa?id=5114021&commentId=14754912)
[Delete](https://bugs.openjdk.org/secure/DeleteComment!default.jspa?id=5114021&commentId=14754912)

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

Commit messages:
 - Misc minor improvements and renamings for clarity
 - Add optional SubmitWithTimeout action
 - Merge remote-tracking branch 'refs/remotes/origin/JDK-8319447' into 
JDK-8319447
 - Merge branch 'openjdk:master' into JDK-8319447
 - Reduce garbage retention; use trailing padding; add tests
 - Better accommodate CompletableFuture; use 4-ary heap; add javadocs; other 
misc
 - Rename DelayedTask to ScheduledForkJoinTask; misc other improvements
 - Simplify policy methods; improve layout
 - Reduce memory accesses
 - Support STPE policy methods
 - ... and 24 more: https://git.openjdk.org/jdk/compare/7d11418c...53516e9d

Changes: https://git.openjdk.org/jdk/pull/23702/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23702&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8319447
  Stats: 1776 lines in 9 files changed: 1524 ins; 155 del; 97 mod
  Patch: https://git.openjdk.org/jdk/pull/23702.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23702/head:pull/23702

PR: https://git.openjdk.org/jdk/pull/23702

Reply via email to