Commit 6279657
committed
Remove
# Motivation
Currently a lot of the operator implementations in here that consume other `AsyncSequence`s require the `AsyncIterator` to be `Sendable`. This is mostly due to the fact that we are calling `makeAsyncIterator` on the upstream `AsyncSequence` and then pass that iterator around to various newly spawned `Task`s. This has two downsides:
1. It only allows users to use operators like `merge` if their `AsyncSequence.AsyncIterator` is `Sendable`
2. In merge we are creating new `Task`s for every new demand. Creating `Task`s is not cheap.
My main goal of this PR was to remove the `Sendable` constraint from `merge`.
# Modification
This PR overhauls the complete inner workings of the `AsyncMerge2Sequence`. It does a couple of things:
1. The main change is that instead of creating new `Task`s for every demand, we are creating one `Task` when the `AsyncIterator` is created. This task has as child task for every upstream sequence.
2. When calling `next` we are signalling the child tasks to demand from the upstream
3. A new state machine that is synchronizing the various concurrent operations that can happen
4. Handling cancellation since we are creating a bunch of continuations.
# Result
In the end, this PR swaps the implementation of `AsyncMerge2Sequence` and drops the `Sendable` constraint and passes all tests. Furthermore, on my local performance testing I saw up 50% speed increase in throughput.
# Open points
1. I need to make this sequence re-throwing but before going down that rabbit whole I wanna get buy-in on the implementation.
2. We should discuss and document if `merge` and other operators are hot or cold, i.e. if they only request if they got downstream demand
3. I need to switch `AsyncMerge3Sequence` over to the same iplementationAsyncIterator: Sendable requirement from merge1 parent c0fdfdb commit 6279657
File tree
4 files changed
+1241
-183
lines changed- Sources/AsyncAlgorithms
4 files changed
+1241
-183
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
20 | 22 | | |
21 | | - | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
22 | 29 | | |
23 | 30 | | |
24 | 31 | | |
| |||
0 commit comments