Improves documentation for VirtualThreads boundedElastic behavior #3635
Conversation
Signed-off-by: OlegDokuka <[email protected]>
Signed-off-by: OlegDokuka <[email protected]>
OlegDokuka
changed the title
docs/asciidoc/advancedFeatures.adoc
Outdated
| @@ -624,6 +624,18 @@ invoked whenever a `Runnable` task submitted to a `Scheduler` throws an `Excepti | |||
| that if there is an `UncaughtExceptionHandler` set for the `Thread` that ran the task, | |||
| both the handler and the hook are invoked). | |||
|
|
|||
| [[scheduler-virtual-thread]] | |||
There was a problem hiding this comment.
Please review section 4.5 and consider updating the boundedElastic description instead of adding a new one here. The remark about newBoundedElastic can become part of the above section for factories, so it offers a guidance to the user how to replace the old behaviour with the new one in places that they need a non-global instance.
reactor-core/src/main/java/reactor/core/scheduler/Schedulers.java
Outdated
Show resolved
Hide resolved
| * @return the common <em>boundedElastic</em> instance, a {@link Scheduler} that dynamically creates workers with | ||
| * an upper bound to the number of backing threads and after that on the number of enqueued tasks, that reuses | ||
| * threads and evict idle ones | ||
| * threads and evict idle ones. If runs on Java 21 with |
There was a problem hiding this comment.
The @return section needn't be so detailed. It can just mention that a shared global boundedElastic instance is created and returned instead of repeating notes from the above.
Signed-off-by: OlegDokuka <[email protected]>
reactor-core/src/main/java/reactor/core/scheduler/Schedulers.java
Outdated
Show resolved
Hide resolved
|
|
||
| import reactor.util.annotation.NonNull; | ||
| import reactor.util.annotation.Nullable; | ||
|
|
||
| /** | ||
| * The noop {@link VirtualThread} Reactor {@link ThreadFactory} to be | ||
| * used with {@link ThreadPerTaskBoundedElasticScheduler}. It throws exceptions when is | ||
| * used with {@link BoundedElasticPerThreadScheduler}. It throws exceptions when is | ||
| * being created, so it indicates that current Java Runtime does not support | ||
| * {@link VirtualThread}s. |
There was a problem hiding this comment.
It may be worth saying something like:
This VirtualThreadFactory variant is included when Reactor is used with JDK versions lower than 21, and all methods raise an UnsupportedOperationException. An alternative variant is available for use on JDK 21+ where virtual threads are supported.
|
|
||
| ThreadPerTaskBoundedElasticScheduler(int maxThreads, int maxTaskQueuedPerThread, ThreadFactory factory) { | ||
| BoundedElasticPerThreadScheduler(int maxThreads, int maxTaskQueuedPerThread, ThreadFactory factory) { |
There was a problem hiding this comment.
Similarly here, something like:
This
BoundedElasticPerThreadSchedulervariant is included when Reactor is used with JDK versions lower than 21, and all methods raise an UnsupportedOperationException. An alternative variant is available for use on JDK 21+ where virtual threads are supported.
Co-authored-by: Rossen Stoyanchev <[email protected]>
Signed-off-by: OlegDokuka <[email protected]>
Signed-off-by: OlegDokuka <[email protected]>
Signed-off-by: OlegDokuka <[email protected]>
Signed-off-by: OlegDokuka <[email protected]>
reactor-core/src/main/java21/reactor/core/scheduler/BoundedElasticSchedulerSupplier.java
Outdated
Show resolved
Hide resolved
| * @deprecated in favor of | ||
| * {@link #newBoundedElasticThreadPerTask(int, int, ThreadFactory)}. | ||
| * Should be safely removed in 3.8.0 | ||
| * | ||
| * @return a new {@link Scheduler} that dynamically creates workers with an upper bound to | ||
| * the number of backing threads | ||
| */ | ||
| default Scheduler newThreadPerTaskBoundedElastic(int threadCap, int queuedTaskCap, ThreadFactory threadFactory) { |
There was a problem hiding this comment.
Now we have both threadPerTaskBoundedElastic and boundedElasticThreadPerTask... Even though the latter is more favourable, with the fact that we already released threadPerTaskBoundedElastic, I think it's just better to leave one variant.
Signed-off-by: OlegDokuka <[email protected]>
…sticSchedulerSupplier.java Co-authored-by: Dariusz Jędrzejczyk <[email protected]>
| @@ -63,6 +63,10 @@ | |||
| * Factories prefixed with {@code new} (eg. {@link #newBoundedElastic(int, int, String)} return a new instance of their flavor of {@link Scheduler}, | |||
| * while other factories like {@link #boundedElastic()} return a shared instance - which is the one used by operators requiring that flavor as their default Scheduler. | |||
| * All instances are returned in a {@link Scheduler#init() initialized} state. | |||
| * <p> | |||
| * Since 3.6.0 {@link #boundedElastic()} can run tasks on {@link VirtualThread}s if the application | |||
| * runs on a Java 21 runtime and the {@link #DEFAULT_BOUNDED_ELASTIC_ON_VIRTUAL_THREADS} | |||
There was a problem hiding this comment.
Please revisit all the places where we mention JDK 21 and consider if 21+ is more appropriate
Co-authored-by: Dariusz Jędrzejczyk <[email protected]>
Signed-off-by: OlegDokuka <[email protected]>
No description provided.