Closed StefanOltmann closed 1 year ago
How would you set that global so that it has effect before the companion object gets created? As a environment property? Wouldn't that effect every other app that uses CoroutineWorker?
I still think that using the number of cores is a better default than a fixed 4. The 4 is just random.
Mind that the Dispatchers.Default on JVM is also backed by a thread pool which is equal to the number of cores: https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-dispatchers/-default.html
Yes, threads are expensive, but I don't want hundreds, I want 10 in my case. Maybe you think of AMD Threadripper with 64 cores... Would still make sense to me to have 64 threads then, but we could set a upper limit of something like 12 to prevent diminishing returns if that is what you worry about.
You can make the executor lazy
, and then it won't choose a number of threads until it's first accessed.
Yep generally there are diminishing returns. My main concern is that CoroutineWorker is not likely to be the primary thread pool in most use cases, in which case it'd be preferable to have a way to "throttle" CoroutineWorker, in a sense.
I would be happy with these defaults with a way to override. Seems pretty reasonable. Like @benasher44 was saying, we could easily add in a way to override by doing something like:
// This somewhere global and with a comment about how it has to be set prior to using coroutine worker.
@ThreadLocal
public var coroutineWorkerCount: Int = getDefaultNumWorkers()
...
private val executor: BackgroundCoroutineWorkQueueExecutor<WorkItem> by lazy {
BackgroundCoroutineWorkQueueExecutor(coroutineWorkerCount)
}
And thanks for the PR!
I don't think you want @ThreadLocal
though. I think you'd want it to be a global marked as @SharedImmutable
and then have it be an AtomicInt
Newer iPhones and iPads have 6 cores, current MacBooks 10. This change will use as much workers as there are processors. See issue #96.