kotlinx.coroutines icon indicating copy to clipboard operation
kotlinx.coroutines copied to clipboard

New implementation of K/N multi-threaded dispatchers

Open qwwdfsad opened this issue 3 years ago • 2 comments
trafficstars

qwwdfsad avatar Aug 23 '22 13:08 qwwdfsad

Long story short, I was trying to rewrite the dispatcher with proper locks (i.e. https://github.com/Kotlin/kotlinx.coroutines/commit/c414dce40d749436d1681f8fac8ad47fe2f2b17b) in order to make it faster.

Apparently, zero-contention cases indeed become significantly faster, but any contention will kill all the dispatcher performance (I mean it, orders of magnitude). In order to avoid it, we need something that barely resembles a scalable lock (i.e. AbstractQueuedSynchronizer). The closest thing to it is, surprise, a channel :)

So we invested around ~10 human hours (I've also included K/N folk to help me and avoid some stupid gotcha) and result is to keep the code the way it is :)

I do have ideas on how to get rid of channel overhead for uncontended cases, but it doesn't seem to be worth it

qwwdfsad avatar Aug 30 '22 14:08 qwwdfsad

On a side note, the documentation to the native declaration of newFixedThreadPoolContext will arrive later, first, we'll merge it, and I'll create Dispatchers.IO on top of that and only then KDoc will follow

qwwdfsad avatar Aug 30 '22 15:08 qwwdfsad

The fix looks good to me

qwwdfsad avatar Oct 10 '22 10:10 qwwdfsad

I am wary now that we found two races in the implementation, so I extracted the data structure and added a test for it. Sure, it's only used in one place, but the benefit of having that code tested (and also a bit more transparent, IMHO) outweighs this, I think.

dkhalanskyjb avatar Oct 10 '22 13:10 dkhalanskyjb