jsr370-experts@jax-rs-spec.java.net

Re: [jax-rs-spec users] Re: Default ExecutorService used on the client

From: Ondrej Mihályi <ondrej.mihalyi_at_gmail.com>
Date: Sun, 30 Apr 2017 11:42:29 +0200

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.
>