Introduce WorkStealingDispatcher.

[zach-klippenstein]

No description provided.

[zach-klippenstein]

Tests in this PR and the runtime one (#1365) are green, so removed the logs, and this is fully ready to review (and hopefully merge).

[rjrjr]

LGTM but I don't think my review is worth much. Maybe we can get @jingibus to take a look?

[steve-the-edwards]

I wonder if we could get some of the folks from kotlinx.coroutines that were engaging in my issue - Kotlin/kotlinx.coroutines#4456 - and on the thread - https://kotlinlang.slack.com/archives/C1CFAFJSK/p1745528199117359 - to review this.

The suggestion was made that the behaviour we wanted was runBlocking but we did not have the mechanism to control the coroutines except via the dispatcher.

This is that mechanism to do that, but I wonder if they might know of some more gotchas/important test cases from work with runBlocking.

The advantage we have here is that we carefully control where advanceUntilIdle is called.

I'm still confused by calling advanceUntilIdle at the end of resumeWith() though?

[steve-the-edwards]

Are you talking about the event loop here? Might as well mention it.

[steve-the-edwards]

why would we ever need to set it back to true? haven't we just drained the queue?

[zach-klippenstein]

haven't we just drained the queue?

Yes, and our scheduled drain operation will indeed be a noop if nothing is dispatched in the meantime.

why would we ever need to set it back to true?

Because if something is dispatched between the end of this operation and when our scheduled drain runs, we don't want to schedule another drain.

I think what you're wondering is why we don't cancel the scheduled drain here instead. The reason is: we can't. The dispatcher API only allows you to schedule Runnables to run, not cancel them. I think the reason for that is that even a cancelled continuation needs to be resumed when cancelled to run any finally blocks.

[steve-the-edwards]

I don't think forEach guarantees ordering, does it? My frame of reference for contrast is the trampolining queue in Compose's AndroidUiDispatcher. Having the ordering behaviour enforced (FIFO) is helpful because it means that our actions will get queued up before we start handling them.

I think I see how we can get around not locking getting the next item off the queue, because we are using an isolated queue for each draining, and the rest of the tasks build up on a new queue. So that part makes sense.

How do you think this performs compared to using the locked nextTask() method of Compose's AndroidUiDispatcher?

[zach-klippenstein]

forEach guarantees ordering for the "batch" of tasks, but ordering of batches depends on the underlying dispatcher and how advanceUntilIdle is called. It's not guaranteed if the underlying dispatcher is not single-threaded and/or advanceUntilIdle is called from multiple threads. E.g. Dispatchers aren't required to guarantee FIFO ordering. Dispatchers.Default can't, for example, since it has no control over thread scheduling. The current impl of WSD doesn't provide any additional guarantees on top of whatever the underlying dispatcher does. It weakens ordering guarantees if advanceUntilIdle is called from other threads, but as mentioned above this isn't something we do, and would just add extra overhead for no reason.

Discussed performance considerations above. tl;dr: The trade-off is that when there's contention, we can process a whole batch without locking after every task, but might end up allocating a few extra queue list objects. But AndroidUiDispatcher locks on every task, so that's probably fine.

[steve-the-edwards]

No delay() tests here. But again, what do we want do with that? if we're going to ignore, then we need to make that clear.

[zach-klippenstein]

Without this we get an error about the internal expect actual class Lock. It doesn't affect public API, so we don't need to worry about transitive compatibility.

[zach-klippenstein]

forEach guarantees ordering for the "batch" of tasks, but ordering of batches depends on the underlying dispatcher and how advanceUntilIdle is called. It's not guaranteed if the underlying dispatcher is not single-threaded and/or advanceUntilIdle is called from multiple threads. E.g. Dispatchers aren't required to guarantee FIFO ordering. Dispatchers.Default can't, for example, since it has no control over thread scheduling. The current impl of WSD doesn't provide any additional guarantees on top of whatever the underlying dispatcher does. It weakens ordering guarantees if advanceUntilIdle is called from other threads, but as mentioned above this isn't something we do, and would just add extra overhead for no reason.

Discussed performance considerations above. tl;dr: The trade-off is that when there's contention, we can process a whole batch without locking after every task, but might end up allocating a few extra queue list objects. But AndroidUiDispatcher locks on every task, so that's probably fine.