I agree that it is not necessary to mandate the DEFAULT service, it is enough to say ANY ManagedExecutorService.
-Markus
From: Ondrej Mihályi [mailto:ondrej.mihalyi_at_gmail.com]
Sent: Sonntag, 30. April 2017 11:42
To: jsr370-experts_at_jax-rs-spec.java.net
Subject: Re: [jax-rs-spec users] Re: Default ExecutorService used on the client
I agree with Markus in all points, except in Java EE, it would be enough to mandate that a managed executor is used, not necessarily the default one. The javadoc doesn't enforce usage of the default executor, only that it should be a container-managed, so it's already there. Though, I would improve the javadoc like this:
When running in a Java EE container, the default has to be a container-managed executor service, usually the default ManagedExecutorService.
A managed executor needs to be enforced by the specification to avoid that the application server creates unmanaged threads which is often the case currently (at least Jersey in GlassFish executes async callback in a plain unmanaged thread). This unspecified behavior then breaks the contract that all threads acquired from the server are managed.
A similar reasoning applies to Java SE - since there is no requirement on the thread context (as opposed to the Java EE environment), there's no reason to mandate which executor is chosen as the default one. Implementations can provide means of setting the default executor, giving more flexibility to the developers.
Ondrej
17-04-28 18:35 GMT+02:00 Markus KARG <markus_at_headcrashing.eu>:
The forced default to ManagedExecutorService on Java EE should stay, as it is the perfect and most expected choice on Java EE.
On Java SE it makes no sense to default to the ForkJoinPool.commonPool, as that pool is intended for CPU-bound algorithms, not for IO-bound ones like JAX-RS. Hence, we should say that the default on Java SE is decided by the vendor.
There should never be a force to manually set an executor; this should always be optional.
-Markus
-----Original Message-----
From: Sergey Beryozkin [mailto:sberyozkin_at_talend.com]
Sent: Freitag, 28. April 2017 18:02
To: jsr370-experts_at_jax-rs-spec.java.net
Subject: Re: [jax-rs-spec users] Default ExecutorService used on the client
Hi Santiago
Probably not, I'm reading
https://github.com/jax-rs/api/blob/master/jaxrs-api/src/main/java/javax/ws/rs/client/ClientBuilder.java#L245
so it all looks good to me, the executor is optional, the only possible ambiguity is specific to the EE clause in which case it does appear to be non-optional (though where is the ref to ForkJoinPool ?).
IMHO it would simplify things if we just drop the EE sentence, Can the users easily access an EE container-managed executor and set it themselves if they need it ?
Thanks, Sergey
On 28/04/17 15:56, Santiago Pericas-Geertsen wrote:
> Hi Sergey,
>
> When running inside an EE container, the option is clear. Are you suggesting that we leave the notion of a default executor service on the client unspecified and, thus, implementation specific?
>
> — Santiago
>
>> On Apr 28, 2017, at 7:23 AM, Sergey Beryozkin <sberyozkin_at_talend.com> wrote:
>>
>> Hi Pavel
>>
>> The use of the custom executors should be entirely optional, enforcing that some kind of the executor must be used by default would be a major restriction IMHO, ex, I'd still like to investigate how the asynchronous HTTP transport can help with its own pool, so it would be better to retain the flexibility and lift the requirement that the use of the ForkJoinPool.commonPool must be used by default in these cases
>>
>> Thanks, Sergey
>>
>> On 27/04/17 23:48, Pavel Bucek wrote:
>>> Dear EG members,
>>>
>>> when settable executor service on the client side was introduced in the API, there was also a default one specified - ManagedExecutorService when running on the Java EE container and ForkJoinPool.commonPool() when running on Java SE.
>>>
>>> The latter seems to be little problematic.
>>>
>>> We already have this implemented in Jersey and we noticed significant performance drop in some type of tests. After some investigation, it is caused by the ForkJoinPool.commonPool size. The default one is something like 2x number of cores, which might not be enough for multiple reactive invocations, not to mention that this pool is used by default by any CompletionStage/CompletableFuture, parallel streams and other Java SE classes/features, which makes the client performance not predictable and very much dependent on the actual environment.
>>>
>>> The obvious fix for these cases is to set ExecutorService for the client, but it seems like that would be almost required (if we keep commonPool as a default) for some usecases.
>>>
>>> Another reason why we might want to change this is that ForkJoinPool.commonPool seem to be mean for cpu intensive tasks and JAX-RS client is using these threads for I/O - which is generally not that much about CPU time. (just a summary: JAX-RS client is using that for async and reactive calls as well as for SseEventSource).
>>>
>>> What do you think about this issue? Should we lift the requirement to use ForkJoinPool.commonPool as a default one? Or should we rather highlight that fact in the documentation and examples and make super clear that using custom executor service is strongly suggested for advanced usecases?
>>>
>>> Thanks and regards,
>>> Pavel
>>>
A similar reasoning applies to Java SE - since there is no requirement on the thread context (as opposed to the Java EE environment), there's no reason to mandate which executor is chosen as the default one. Implementations can provide means of setting the default executor, giving more flexibility to the developers.