Introduce CoroutineDispatcher.limitedParallelism #2918
Conversation
qwwdfsad
force-pushed
the
sliceable-dispatcher
branch
from
1a3a29c to
778bfbc
Compare
qwwdfsad
requested review from
dkhalanskyjb
and removed request for
dkhalanskyjb
|
You may notice that we have a similar class I will take care of it in a separate PR along with general cleanup of "public" classes such as |
kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt
Outdated
Show resolved
Hide resolved
kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt
Outdated
Show resolved
Hide resolved
kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt
Outdated
Show resolved
Hide resolved
… is passed as an argument
* Support dispatchYield
* Fix doc
* Short-circuit limitedParallelism(x).limitedParallelism(y) for y >= x
qwwdfsad
force-pushed
the
sliceable-dispatcher
branch
from
44e2c4a to
ecd36dd
Compare
* Extract Ktor-obsolete API to a separate file for backwards compatibility
* Make Dispatchers.IO being a slice of unlimited blocking scheduler
* Make Dispatchers.IO.limitParallelism take slices from the same internal scheduler
Fixes #2943
kotlinx-coroutines-core/jvm/test/scheduling/CoroutineSchedulerCloseStressTest.kt
Show resolved
Hide resolved
kotlinx-coroutines-core/jvm/test/scheduling/CoroutineSchedulerCloseStressTest.kt
Show resolved
Hide resolved
kotlinx-coroutines-core/jvm/test/scheduling/CoroutineSchedulerCloseStressTest.kt
Show resolved
Hide resolved
|
|
||
| class DefaultDispatchersTest : TestBase() { | ||
|
|
||
| private /*const*/ val EXPECTED_PARALLELISM = 64 |
There was a problem hiding this comment.
What's wrong with const?
There was a problem hiding this comment.
It cannot be declared in the class
There was a problem hiding this comment.
It's private and so can't leave this file anyway.
There was a problem hiding this comment.
Right, but it obfuscates IDE search a bit by introducing two classes with the same name for IDE.
That's our default convention in tests -- declare "effectively constant" in screaming snake case until the language is fixed
| next = queue.poll() ?: return | ||
| dispatch(next, true) | ||
| } | ||
| } |
There was a problem hiding this comment.
(Historical note to save a future-someone some double-checking) This class is simply moved to the Deprecated file with no changes.
| } | ||
| } | ||
|
|
||
| private fun tryAdd(block: Runnable): Boolean { |
There was a problem hiding this comment.
Maybe rename to addAndTryDispatching, or just inline this? Adding the task to the queue always succeeds, so the name is somewhat misleading.
There was a problem hiding this comment.
Sure. I've extracted the method to reduce the bytecode size due to dispatchInternal being inlined
kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt
Outdated
Show resolved
Hide resolved
| override fun dispatch(context: CoroutineContext, block: Runnable) { | ||
| dispatchInternal(block) { | ||
| if (dispatcher.isDispatchNeeded(EmptyCoroutineContext)) { | ||
| dispatcher.dispatch(EmptyCoroutineContext, this) |
There was a problem hiding this comment.
Why is it ok not to pass this as the coroutine context here?
There was a problem hiding this comment.
Do you mean context argument or this as CoroutineDispatcher context element?
There was a problem hiding this comment.
In general, we do not have any reasonable contracts or use-cases to context parameter, it's mostly legacy from the early versions of coroutines.
I've changed it to this for at least some consistency, but in general, we do not ensure/check/have established/test/ contract of this parameter
|
I've also tweaked the contract for direct dispatchers and prohibited limited parallelism for |
…t it for Dispatchers.Unconfined
…se of default dispatacher for tests
kotlinx-coroutines-core/jvm/test/scheduling/CoroutineSchedulerCloseStressTest.kt
Show resolved
Hide resolved
|
Thanks to @1zaman and @denis-bezrukov for an extra review and valuable comments! |
….IO unbounded for limited parallelism (Kotlin#2918) * Introduce CoroutineDispatcher.limitedParallelism for granular concurrency control * Elastic Dispatchers.IO: * Extract Ktor-obsolete API to a separate file for backwards compatibility * Make Dispatchers.IO being a slice of unlimited blocking scheduler * Make Dispatchers.IO.limitParallelism take slices from the same internal scheduler Fixes Kotlin#2943 Fixes Kotlin#2919
….IO unbounded for limited parallelism (Kotlin#2918) * Introduce CoroutineDispatcher.limitedParallelism for granular concurrency control * Elastic Dispatchers.IO: * Extract Ktor-obsolete API to a separate file for backwards compatibility * Make Dispatchers.IO being a slice of unlimited blocking scheduler * Make Dispatchers.IO.limitParallelism take slices from the same internal scheduler Fixes Kotlin#2943 Fixes Kotlin#2919
Fixes #2919
Fixes #2943