-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DefaultDispatcher #136
Comments
Commited to develop branch |
Cool! Code became much cleaner, no longer need to pull |
I've added the corresponding bridges manually to maintain binary compatiliby with previously compiled code. |
Released in version 0.19 |
Any chance of having the default dispatcher be configurable by the user? (I mean globally/once, not really in any runtime sense) My use case is running on Vert.X with basically all builder calls using Unconfined (things are already taken care of by the platform).. With the new default, it's quite easy to accidentally omit the context.. |
Hi @mslenc, see example: |
I find CommonPool as default dispatcher quite dangerous. IMHO a thread should only be used when really needed but not silently introduced by the framework. I just run into a problem where I thought I'm writing a single threaded application and suddenly I got random crashes because of concurrency issues... Another issues I see with CommonPool as a default is that it results in inconsistent behaviour in common code (JVM/JS). For example, code that uses launch {} might run fine in JS but crash in JVM because of the introduced concurrency. The default launch implementation should have the same semantics in JVM/JS, e.g. that they use the current CoroutineContext... |
@czeidler The |
@elizarov That is all good, yet there are several JVM libraries and frameworks that provide "an event loop" anyway (and they're not in any way obscure either), so please provide that configuration option asked for above, regardless of what the default DefaultDispatcher ends up being.. |
If there is already a CoroutineContext, can't you just use this CoroutineContext for the new coroutine? This is a bit more conservative and I think also more intuitive. But yes if there is no event loop/ no CoroutineContext it becomes a bit tricky. A drastic measure would be to throw an Exception if there is no CoroutineContext, i.e. on the JVM you are forced to specify where you want to run your coroutine. Giving it some thought I would actually prefer this solution since not running code is usually better to debug than code that has threading problems. Good to hear that you are planning some debug tools though. |
This is write-up on why
kotlinx.coroutines
needs aDefaultDispatcher
and what it is.The plan is to have a default value of
context
parameter for all coroutine builders that are defined inkotlinx.coroutines
, so that instead oflaunch(someContext) { ... }
we can just writelaunch { ... }
unless we really care what context our coroutine runs in. Moreover, sometimes we only care about non-dispatch aspects of our coroutines and writelaunch(CoroutineName("foo")) { ... }
which does not really work as of now (see #133). We need some clearly defined default dispatcher that would be used in this case.This is especially important in light of our effort to bring
kotlinx.coroutines
into Kotlin/JS and Kotlin/Native code in such a way as writing common code with coroutines becomes possible. In common code we cannot writelaunch (CommonPool) { ... }
since theCommonPool
is defined only in Kotlin/JVM and is meaningless on other platforms due to different threading model. However, each platform can provide its own sensible default dispatcher.We are going to define a top-level val in
kotlinx.coroutines.experimental
package:There are a few questions to be answered here :
The type. We can use both
CoroutineContext
andCoroutineDispatcher
as types of that variable. The latter is preferable, since it would clearly highlight that it provides default dispatching logic and would prevent some obvious mistakes likeDefaultDispatcher + UI
(it will get flagged as an error during compilation, because adding two dispatchers does not make sense, as the later overwrites the former).The name.
DefaultDispatcher
looks good, since it clearly explains the purpose and fits the type. TheCaptilazationOfTheName is choosen for consistency with other dispatcher names likeUI
andCommonPool
, which are singletons.DefaultDispatcher
is also a singleton, so it is named as such. Of course, there is some bike-shedding to be held on how it all shall be named (see also CoroutineDispatcher's names #41), but we'll skip that bike-shedding for now, since the name ofDefaultDispatcher
is not going to actually appear in the source of end-user applications.The value. For Kotlin/JVM we'll start with
CommonPool
as the value ofDefaultDispatcher
. It is absolutely non-ideal value, since CommonPool is ill suited for the tasks that do not have recursive fork-join structure and most typical coroutine benchmarks run approximately 2 times faster in a single-threaded dispatcher than in CommonPool. Moreover, with CommonPool we cannot efficiently support blocking operations without having to switch to a different thread (see IO thread pool for blocking calls #79). Ultimately, a totally new, engineered from group-up implementation will be needed, but we'll leave this implementation for later.The text was updated successfully, but these errors were encountered: