DefaultDispatcher - why is this the CommonPool?

[ZakTaccardi]

RxJava is synchronous by default. This behavior is awesome.

observable.subscribe { }

By default, coroutine builders like launch, async, produce, etc use the CommonPool. In RxJava, that's like automatically doing:

observable.subscribeOn(Schedulers.computation()).subscribe { }

Because it's a good pattern for testing and maintainability, I always inject my dispatchers.

launch(dispatchers.background) { }

Unfortunately, the DefaultDispatcher lets unfamiliar devs easily make the mistake of referring to the common pool. Even worse, they may not even understand what the CommonPool is and just throw coroutines around and not understand in which dispatcher they run in.

launch { } // surprise! runs in the common pool

Why not make the default dispatcher Unconfined? That way devs will know that if they want to confine the coroutine to a particular context, they can pass the appropriate dispatcher.

I think it's a bit confusing to have different coroutine builders refer to different dispatchers.

Note: I understand it's probably too late to change this behavior :(

[jcornaz]

This is quite opinionated I think. In my opinion, Unconfined would have been indeed a good default, for the reasons you mentioned. However, if it is better than CommonPool I am less sure.

if launch and async would be unconfined by default, then someone would have writen a similar post to yours with the following snippet:

async { } // surprise! It is not asynchronous!

Usually if you use async or launch, this is because you want to launch a concurrent coroutine (that's why it is named like this). So it does make sense to use CommonPool by default IMO.

And I'd like to precise that subscribe of RxJava should not be compared to launch or async, it has to be compared to its equivalent for coroutines: consumeEach:

observable.subscribe { } // sychronous? asynchronous? Who knows? You have to check if `subscribeOn` and/or `observeOn` is present in the upstream.
channel.consumeEach { } // This is a suspending function always executed synchronously in the current context.

[elizarov]

Let me add some more insight here. launch { ... } was supposed to start a new background asynchronous activity by design. It many ways it is similar to thread { ... }. Now, I admit that this may not be entirely safe practice to go left-and-right starting background activities, so we have some thoughts into how to make it slightly more explicit in the future.

I think you are looking for Rx-like cold-stream abstraction that does not start any background activity. We plan to provide that in the future. See #254.

[ZakTaccardi]

we have some thoughts into how to make it slightly more explicit in the future.

I would definitely be curious to hear more of this.

Personally, I think the Unconfined dispatcher should be the default. Though it is probably too late to change it now.

[elizarov]

@ZakTaccardi I assume that you are looking for Rx-like streams (see #254). They will be not have any dispatcher by default and should match your expectations. However, for asynchronous background activities like async { ... } the default should not be synchronous.

On our other ideas see #410

[voddan]

Although this issue is closed, I would like to leave a piece of feedback on this.

Our team builds a (production) web app with Spring a coroutines. Every time we bridge between Spring router and the rest of the code, we have to make sure to put Unconfined there. Effectively, Unconfined is the default in our team.

On the other hand, every time someone needs a background task executed, the best practice is to create a separate thread pool with the needed settings and use it instead of CommonPool. This rule is written in blood: one time we leaved the dispatcher to be the default for a long-lived task with a timer, the prod failed under stress tests because all threads in CommonPool we used and the timer task could not be resumed in time. Debugging this was no fun.

In short, the current choice for DefaultDispatcher is bizarre: it is not suited for most production use cases (Unconfined is much more common), and when you need a background execution, you can't use it because of side effects that come when using CommonPool

[jcornaz]

In short, the current choice for DefaultDispatcher is bizarre: it is not suited for most production use cases (Unconfined is much more common)

I do a agree. Unconfined is also the effective default in our team and we usually leave responsibility to suspending functions to use withContext if they need to execute something in a specific thread pool (most of the time IO or CommonPool).

But it doesn't change what I've said before: I think it would be a bit unnatural if launch and async would be synchronous by default.

one time we leaved the dispatcher to be the default for a long-lived task with a timer, the prod failed under stress tests because all threads in CommonPool we used and the timer task could not be resumed in time

Sounds like some tasks were blocking the threads. In this case one should indeed not use the CommonPool but IO.

However. for computational tasks, CommonPool is still the best choice. I don't see the point of using more threads than logical processors for computation intensive tasks.

[voddan]

one time we leaved the dispatcher to be the default for a long-lived task with a timer, the prod failed under stress tests because all threads in CommonPool we used and the timer task could not be resumed in time

Sounds like some tasks were blocking the threads. In this case one should indeed not use the CommonPool but IO.

@jcornaz nothing was really blocking, those task just happened to run when it was time for the timer coroutine to wake up. Also any task with higher priority could usurp the CPU and prevent coroutine switching.

However. for computational tasks, CommonPool is still the best choice. I don't see the point of using more threads than logical processors for computation intensive tasks.

My point is that when delegating to a thread pool, we always need to be explicit about that. Want to compute on CommonPool - be explicit about it and explain why potential blocking won't hurt. Want a separate thread pool - specify who can use it with you.

IMO any thread pool with fixed number of threads is unfit to be the default dispatcher.

[jcornaz]

My point is that when delegating to a thread pool, we always need to be explicit about that. Want to compute on CommonPool - be explicit about it and explain why potential blocking won't hurt. Want a separate thread pool - specify who can use it with you.

This is a good point. I agree with it.

Maybe the best would be to not have any default dispatcher at all?

[voddan]

Maybe the best would be to not have any default dispatcher at all?

Having Unconfined as default would be pretty useful. AFAIU in both yours and mine teams it is already the default.

[jcornaz]

Having Unconfined as default would be pretty useful

"useful" -> yes
"natural and easy to read/understand" -> I don't think so.

async { } // surprise! It is not asynchronous!

I think forcing the user to be explicit could be nice. At least I'd prefer that than having Dispatchers.Unconfined as a default.

[elizarov]

We've lived with "you must be explicit" for a while, but we had to abandon this policy when we started to work on common code. You cannot require to be explicit in your common code, since JVM has threads and pool, but JS does no, so you cannot just go a write code that runs both on JS and JVM. We needed some default.

What that default should be? It cannot be Unconfined for the reasons outlined above (asynchronous code ceases to be asynchronous). Moreover, in JS world it is natural to always dispatch your tasks on your event thread (unconfined is not idiomatic in JS). The best analogue of "the event thread" that we have for JVM is the pool of threads sized to the number of CPU cores, hence it is our default.

Now, with structured concurrency (#410) this should not be a big issue anymore. A default is used only when you do GlobalScope.launch, which should not be done often (and should never be done in larger projects), so we are kind of back to requiring explicit dispatcher. Now in you apps you need to define your own CoroutineScope where you have to define which dispatcher works best for you.

[jcornaz]

A default is used only when you do GlobalScope.launch, which should not be done often (and should never be done in larger projects)

@elizarov May I ask which CoroutineScope should be used when we are not in a coroutine yet and need to schedule a work. Here is an example:

// here this function cannot suspend as it will be called by the UI framework in a blocking manner.
fun handleUiAction(event: ActionEvent) {

  // what scope should I use here?
  GlobalScope.launch { doSomeBackgroundWork() }
}

[qwwdfsad]

@jcornaz handleUiAction is probably defined in some Activity or Fragment class.
Then this class should implement CoroutineScope as it is described in the new UI guide section

[voddan]

@elizarov I've just read the papers on structured concurrency. Very cool!

Did I get it right that now when I have a Spring class with request routing methods, I implement CoroutineScope for the whole class, and choose the strategy I need (Unconfined) there, after which every fun response() = async {} uses this strategy and is canceled when the class is canceled?