Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show in-order processing patterns, equivalent to queue.async {} #69

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ktoso
Copy link
Member

@ktoso ktoso commented Jun 14, 2024

And that's not even half the story to be honest... there's even more advanced patterns which_do_ handle structured concurrency better... I'll leave at this example for now.

Future work here:

Overall I'd definitely like to include this somewhere here because it comes up all the time, but unsure about where exactly to put it. Open to opinions @mattmassicotte @hborla @xedin

resolves #22

@ktoso ktoso force-pushed the wip-show-in-order-processing-patterns branch from 7ef91aa to 68cab91 Compare June 14, 2024 04:19
Comment on lines +8 to +12
Swift's concurrency model with a strong focus on async/await, actors and tasks,
means that some patterns from other libraries or concurrency runtimes don't
translate directly into this new model. In this section, we'll explore common
patterns and differences in runtime behavior to be aware of, and how to address
them while you migrate your code to Swift concurrency.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think runtime behaviour is a great article name but IMO we should talk about executors and executor jobs here first and how they relate to Tasks before we dive into implementing a specific pattern of ordered processing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'm not writing those in "final document order";

we'll do #59 which will explain executors, and it will be here; no worries.

```

This example **does not** guarantee anything about the order of the printed values,
because Tasks are enqueued on a global (concurrent) threadpool which uses multiple
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would help if we spoke about executors here first IMO.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking a lot about this. Many readers are going to be seeing these concepts for the first time. I find it easier to see concepts I work with in day-to-day code first, and then have access to lower-level details should I need them. Structuring these kinds of things is tricky. But, I think starting high-level and then moving lower could help offer more progressive disclosure. But, it also could make for a less organized document. So, I'm not sure what the correct trade-off is here, especally given that this is specifically about runtime behavior.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's sections to be written about executors, that'd address this concern imho; it would not be in this section anyway

Comment on lines +119 to +122
The safest and correct way to enqueue a number of items to be processed by an actor,
in a specific order is to use an `AsyncStream` to form a single, well-ordered
sequence of items, which can be emitted to even from synchronous code.
And then consume it using a _single_ task running on the actor, like this:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to call out here that this pattern is good to get order but it is potentially dangerous since it can queue up an unbounded amount of work. We should call this out here that AsyncStream has no external backpressure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I can do a small note and direct to docs; there's a risk in too much information here, so I'll do just a short note on it

// Start consuming immediately,
// or better have a caller of Printer call `startStreamConsumer()`
// which would make it run on the caller's task, allowing for better use of structured concurrency.
self.streamConsumer = Task { await self.consumeStream() }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please not put spawning an unstructured task in this example here. Instead let's add a func run() async that consumes the stream. The users of this actor can then decide on what task to call that run method on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's explained below. I can inverse the order I guess

// between the producing and consuming task
actor Printer {
let stream: AsyncStream<Int>
let streamContinuation: AsyncStream<Int>.Continuation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this have to be nonisolated for the example to work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I compiled this with Swift 6 mode, seems fine; I can double check again

so we might need to resort to callbacks if we needed to report completion of an item
getting processed.

Notice that we kick off an unstructured task in the actor's initializer, to handle the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah we mentioned this down here. Can we just not show the unstructured task to begin with?

Guide.docc/RuntimeBehavior.md Outdated Show resolved Hide resolved
Guide.docc/RuntimeBehavior.md Outdated Show resolved Hide resolved
Swift concurrency naturally enforces program order for asynchronous code as long
as the execution remains in a single Task - this is equivalent to using "a single
thread" to execute some work, but is more resource efficient because the task may
suspend while waiting for some work, for example:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence is quite long, can it be broken down?

Guide.docc/RuntimeBehavior.md Outdated Show resolved Hide resolved
Guide.docc/RuntimeBehavior.md Outdated Show resolved Hide resolved
Guide.docc/RuntimeBehavior.md Show resolved Hide resolved
```

This example **does not** guarantee anything about the order of the printed values,
because Tasks are enqueued on a global (concurrent) threadpool which uses multiple
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking a lot about this. Many readers are going to be seeing these concepts for the first time. I find it easier to see concepts I work with in day-to-day code first, and then have access to lower-level details should I need them. Structuring these kinds of things is tricky. But, I think starting high-level and then moving lower could help offer more progressive disclosure. But, it also could make for a less organized document. So, I'm not sure what the correct trade-off is here, especally given that this is specifically about runtime behavior.

printer.enqueue(2)
printer.enqueue(3)
```

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think including a real init to make this syntatically valid, and then adding a swift will make this stand out less than it currently does.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok will do

printer.enqueue(2)
printer.enqueue(3)
```

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing here - the lack of syntax highlighting really makes these stand out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will fix

Guide.docc/RuntimeBehavior.md Outdated Show resolved Hide resolved
ktoso and others added 3 commits June 14, 2024 20:09
@ktoso
Copy link
Member Author

ktoso commented Jul 3, 2024

Huh seems I didn't follow up here, will do :)

Copy link

@ankushkushwaha ankushkushwaha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this documentation work. I appreciate it. Some typos and corrections I found.

printer.enqueue(3)
```

In this case, you'd should make sure to only have at-most one task consuming the stream,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In this case, you'd should make sure to only have at-most one task consuming the stream,
In this case, you should make sure to only have at-most one task consuming the stream,

We're assuming that the caller has to be in synchronous code, and this is why we make the `enqueue`
method `nonisolated` but we use it to funnel work items into the actor's stream.

The actor uses a single task to consume the async sequence of items, and this way guarantees

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The actor uses a single task to consume the async sequence of items, and this way guarantees
The actor uses a single task to consume the async sequence of items, and this guarantees

@ankushkushwaha
Copy link

Can we resolve conflicts for this branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document FIFO (or lack thereof) guarantees when coming from Dispatch
5 participants