New implementation of K/N multi-threaded dispatchers #3421
Conversation
qwwdfsad
force-pushed
the
native-new-dispatchers
branch
from
39a63a2 to
049f0aa
Compare
| // TODO handle rejections | ||
| tasksQueue.trySend(block) | ||
| if (activeWorkers.value != workersCount) { | ||
| tryAddWorker() |
There was a problem hiding this comment.
I would really like to provide a proper keep-alive here, but it doesn't seem adequately possible: we must terminate workers and we must await termination in a blocking way, otherwise native resources leak.
The preliminary decision is to get rid of workers and design something better (i.e. Thread) in K/N
|
Long story short, I was trying to rewrite the dispatcher with proper locks (i.e. c414dce) 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. 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 |
|
On a side note, the documentation to the native declaration of |
qwwdfsad
force-pushed
the
native-new-dispatchers
branch
from
b91335b to
6ade8f1
Compare
|
The fix looks good to me |
|
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
force-pushed
the
native-new-dispatchers
branch
from
9ab3fa7 to
1931831
Compare
| * A thread-safe resource pool. | ||
| * | ||
| * [maxCapacity] is the maximum amount of elements. | ||
| * [create] is the function that creates a new element. |
There was a problem hiding this comment.
| * [create] is the function that creates a new element. | |
| * [create] is the function that creates a new element. | |
| * It is part of 'common' source-set in order to be testable on JVM with Lincheck |
There was a problem hiding this comment.
Also, probably can be moved to 'concurrent'?
| private val create: (Int) -> T | ||
| ) { | ||
| /** Number of existing elements + isClosed flag in the highest bit. | ||
| * Once the flag is set, the value is guaranteed not to change anymore. */ |
There was a problem hiding this comment.
I would stick with the default Java convention, that autoformats this comment into the following form:
/*
* Comment
*/
I've stressed our (Kotlin) auto-formatter for a while, and it seems like it just doesn't work at all, I'll file all the bugs separately
* Create workers lazily * Do properly handle rejection * Await termination in parallel
Co-authored-by: dkhalanskyjb <[email protected]>
Co-authored-by: dkhalanskyjb <[email protected]>
Also, test it properly.
dkhalanskyjb
force-pushed
the
native-new-dispatchers
branch
from
1931831 to
ac7bc3c
Compare
No description provided.