SE-0304 (2nd review): Structured Concurrency

DevAndArtist · April 14, 2021, 12:37pm

I'm not entirely following your message on the mentioned types, but as I read it and looked up the current implementation for Task.Priority I got the following impression.

The type as currently implemented and proposed specifies the importance of a task on an integer based level. However, the rawValue isn't just 0, 1, 3, etc. it has a hardcoded incremental pattern, but with some strange gaps between those. This might be a 1 to 1 map with some Priority type from Dispatch. To me it looks like there might be some 'private' priorities which would fit right in between the one implemented right now. My point is not that this isn't public knowledge and I don't want to speculate any further on that. My point is that we had a similar problem in the language itself with "operator precedence groups".

Based on the mentioned problem, wouldn't it be better if Task.Priority was user-extensible and the priority sorting would be done at runtime? The priority on predefined integer levels simply won't do it.

That and the stuff I wrote in my previous message.

John_McCall · April 15, 2021, 7:03am

Allowing a user-extensible lattice of priorities seems a bit over-engineered for the task. It's also not implementable. The core of the priority design is to have a fairly small number of semantically-defined priority categories which can be mapped onto the needs of the system scheduler. Among other things, that means they need to be possible to turn into system-level thread priorities in some well-defined way. We can't ask the kernel to sort threads according to some lattice, and we have to share the process with threads that don't have anything to do with Swift.

Ultimately, we are going to support a small range of integer priorities; that's a design constraint. It's completely fair to point out that the names and categories in the current design are very Darwin-focused, and moreover that they aren't really described very carefully. Feedback on how you think they should look that fits within the constraints would be welcome.

John_McCall · April 15, 2021, 7:44am

I should add that Swift Evolution doesn't have the power to change the set of Dispatch QoS values supported by Apple platforms. So we can consider TaskPriority and Dispatch's QoSClass to be different types and translate between them, but if so we do still have to consider what that translation actually is. We might be able to make some of the categories only available on Darwin.

We also intentionally don't want to offer fine-grained values that map to arbitrary thread priorities.

Karl · April 15, 2021, 9:54am

My understanding is that the priority is intended to be consumed by the executor, and the integer priority is just a built-in kind of metadata which is understood by the default executor (and since we use libdispatch, naturally it is modelled on dispatch's QoS system).

But I also understand that the core team feel it is important to enable custom executors in order to apply custom scheduling logic. If that's the case, I think we do need some way for jobs to pass along some kind of application-specific metadata for that to be effective. As I mentioned in the executor thread, that could be Task-local storage, but it would also be nice if there was something that could be accessed more efficiently.

It might be possible, for instance, to allow the priority to be a pointer to an arbitrary object (marked with a tag bit). When the default executor sees these, it would execute them with default priority, but custom executors could interpret them for their own purposes.

Looking at the implementation, it seems that the priority is compressed with a bunch of other flags, so this would come at size cost. I suppose it depends on how much we expect people to use task-locals, and how much gain each application can realise by using custom scheduling.

hassila · April 15, 2021, 12:22pm

I'll just add one slightly different view, even if its perhaps a bit late.

I think one of the reason why people argue about priority classes, is that it is not the optimal way of thinking about the problem domain IMHO.

I think that instead of priority, it would be more natural to describe the classification of job.

E.g. We could have a priority classification of something like:

Mission critical
Blocks progress
Normal/standard/default
Not blocking progress
Backburner

Where the definitions would be:

Mission critical

Highest possible importance to the task, both urgent and important. This can be e.g. interactive user interface activity or a server needing to perform time critical control of external systems.

Blocks progress

This task is blocking progress for others, either end users (waiting for a task to finish) or responding to external system requests for a server. This is a reasonable choice when the latency of processing the request will be perceptible and important to the outside of the task.

Normal/standard/default

Standard default classification.

Not blocking progress

This is a lower priority task that may take some time to finish, but it is not blocking progress for anyone. External users/systems may be affected / give feedback when its done.

Backburner

Work that will be performed at the lowest priority. We still want to get it down, but no external users/systems particularly care exactly when its done.

Just my 0.02c

ktoso · April 15, 2021, 12:33pm

For what it’s worth @johannesweiss also brought up the point that Priority is weirdly married to Apple platforms with the names - ignoring what actual numeric values we’d use - a long time ago in early reviews.

It’s totally true and they’re pretty unapplicable to non-Apple platforms...

It seems to me like one of those battles that we don’t really see worth fighting hard, given that the primary other platform we care about is server-side so mostly including Linux where... Using priorities to influence task progress just isn’t a thing really... Servers are usually far more overcomitted with resources than e.g. „your iPhone” and there isn’t really much „complete THIS ONE NOW” as requests are more-or-less created equal...

I’m more concerned with thread-starvation than some fiddling with priorities in server-side systems as the grand scale of things — all users being served equally well, wins over some micro optimizations of specific tasks. At the same time, if there are any „super critical task” e.g. some heartbeating or crucial part of the clustered server application, it’s never going to be enough just to use priority to solve these — you’ll want to dedicate entire thread-pools to the important work. So these actually important tasks, which keep the cluster together and healthy, never have to compete at all with any „user tasks”.

I could be completely wrong, given some specific applications — but in my years of server side development, including large scale distributed actor systems; one never resorted to priority use. Neither have I ever seen them used in Netty, Aeron or similar applications. It also isn’t very „the actor way” to have entire mailboxes be based on priority, we had this in Akka as an optional thing and they were pretty useless in practice.

—

Having that said... it would be nice™ to not hardcode the apple naming of priorities; but it seems no-one so far really came up with a workable alternative design. We, so far, at least didn’t have it as high priority (pun intended) task enough to really see this as a blocker. (And by „we” I also mean the Linux support for Swift concurrency).

// Caveat for readers: my perspective is very much that of a server ecosystem developer; so it’s not that I like the Apple priorities or anything; it just seems to be a non-topic on servers really. I don’t know what these would mean for windows; but I assume we’d be able to add specific new prioritiy values if the need arises..

// Caveat: I felt pretty sad writing this to be honest; It would be very nice to have the Swift model be pure and not so tainted by Dispatch.

It’d be great if we could find a workable model — as John said, looking for workable ideas, but also worried it’d be one of those never-ending naming bike-shed topics...

hassila · April 15, 2021, 12:49pm

I'd just like to give a slightly dissenting opinion (with a pure server perspective), for systems that provide plugin-capability for end-customer code which shared thread resources, its been quite useful to provide priorities for that code... This was also with medium-scale distributed systems. In swift, each customer plug-in would likely be realized as an actor in the future, so a per-mailbox priority could/would actually be useful.

We ended up having to roll our own priorities at that time, but it'd definitely be nice to use the task priorities instead for a swift based solution in the future. (previous work was in the pthread + GCD world on Solaris and Linux, so we abstracted away the thread management from the customer code). I guess it's not a big thing at the end, we can always do our own again.

Anyway, I just never liked the hardcoding it to end-user interactions as its been and would have preferred something along the lines mentioned above, but if its a fight not worth doing, then lets not do it - it was just that the question was asked "Feedback on how you think they should look that fits within the constraints would be welcome."...

ktoso · April 15, 2021, 12:55pm

Yeah I can see that, though in today’s world I don’t think that level of isolation — especially between different customer code (!) — is viable. We ought to protect one customer’s code from other customer’s code — what if it had „bad actors” in it, attempting to either starve (by blocking) or actively attach the shared memory space. (pun intended )

Such multi tenant things really feel like in the future might rather be solved by WASM on the server, just like Cloudflare have been pushing for with their Workers: https://blog.cloudflare.com/webassembly-on-cloudflare-workers/

But I do see that there can be different kinds of applications and use cases though.

By the way: In no way am I trying to defend the apple specific namings - I’d love them to be generic names...

hassila · April 15, 2021, 1:00pm

Yeah, in our case, it was the same customers code, they would have a bunch of different plugins running concurrently but needed some control over scheduling as resources as always were limited and some things were business critical. (definitely agree in a shared multi tenancy solution its not viable).

JJJ · April 15, 2021, 1:52pm

Bikeshedding QoS / priority naming to get the discussion going

/// Describes the priority of a task.
enum Priority: Int, Comparable {
    /// For urgent and important work.
    /// In user interfaces this could be user interactions, such as animations, event handling, or updating the user interface.
    case critical

    /// For high priority work blocking other parts of the system (or the end user).
    /// In user interfaces this could be a task initiated by the user, which the user is actively waiting on.
    case highPriority

    /// Default priority for tasks. 
    case normal

    /// Priority for a utility function that e.g. could have a progress bar.
    case longRunning

    /// Priority for maintenance or cleanup tasks.
    case background
}

ktoso · April 15, 2021, 1:53pm

Naming is the easy part that can be bikeshed to no end... please consider an implementation strategy and how we'd tie it with existing Dispatch priorities when proposing things. Thank you.

JJJ · April 15, 2021, 1:54pm

I would actually just suggest renaming the priorities in the proposal and keep the proposed semantics.

Are people generally against that?

Karl · April 15, 2021, 2:25pm

I think the names are the least interesting aspect of priorities. IMO, we should be focusing on how priorities are described in the proposal, and whether the proposed APIs are adequate to fulfil those goals:

Task priority may inform decisions an executor makes about how and when to schedule tasks submitted to it. An executor may utilize priority information to attempt to run higher priority tasks first, and then continuing to serve lower priority tasks. It may also use priority information to affect the platform thread priority.

The exact semantics of how priority is treated are left up to each platform and specific executor implementation.

The priority of a task is used by the executor to help make scheduling decisions.

Are 5 arbitrary levels of priority with essentially no application-specific information attached enough to inform executors? Can you trust libraries to have good judgement about how important their tasks are as you compose them in to an application?

Or do we generally expect people to separate components in to separate executors, each of which interpret the 5 arbitrary levels in their own way and map them to the priority levels of a wrapped executor?

In which case, we'll have a bunch of tasks floating around whose priority levels are only really comparable with other tasks on the same executor. So we'd need to consider priority escalation:

In some situations the priority of a task must be escalated in order to avoid a priority inversion:

If a task is running on behalf of an actor, and a higher-priority task is enqueued on the actor, the task may temporarily run at the priority of the higher-priority task. This does not affect child tasks or the reported priority; it is a property of the thread running the task, not the task itself.

If a task is created with a task handle, and a higher-priority task waits for that task to complete, the priority of the task will be permanently increased to match the higher-priority task. This does affect child tasks and the reported task priority.

Firstly, I don't think this passage is very clear; I'd appreciate some elaboration about how it works. It also only talks about actors - but aren't tasks "enqueued on an actor" really just enqueued on the actor's executor? Isn't the binding of an actor to a specific executor the thing that gives it its isolation? How do custom executors (and possibly custom priority information via task-local storage) fit in to this mechanism, or is it only possible at the actor level?

John_McCall · April 15, 2021, 6:57pm

Priority needs to be a more global property of the system, as tasks can move between executors. I agree that we should allow some way for tasks to record other information that can be queried by interested executors. I am not convinced that priority is the right tool for that job.

John_McCall · April 15, 2021, 7:08pm

For example, this is not a good idea:

Execution priority is to a significant degree a global concept. Some executors might want to take advantage of additional information when it's present, but a system which causes arbitrary executors to fall back on default execution priority just because they don't specifically recognize some new custom priority is a system that probably isn't really honoring the basics of priority very well. Baseline task execution priority is something that every custom executor will be expected to honor in some way.

JJJ · April 15, 2021, 8:19pm

I would assume these proposed "priorities" were really meant to be used as Quality of Service hints in the libdispatch implementation.

Would it make sense to not talk about priorities in the proposal, and instead talk about either "priority hints", "scheduler hints", or just plain "quality of service hints"?

Perhaps one day it could be possible to say something like: "This framework or module should be allocated priorities in this range, according to its internal use of Task.QoS", perhaps?

benrimmington · April 15, 2021, 9:53pm

Would a floating-point value between 0.0 (lowest) and 1.0 (highest) be a workable design?

On the other hand, Foundation.Thread has a similar threadPriority: Double property, but with different semantics.

Karl · April 15, 2021, 11:35pm

Sure, but the other side of this is that we tend to encourage developing applications by composing modular functionality (often from 3rd-party libraries), and breaking your code down in to small types and functions which can locally reason about how they should behave.

This is something that I've always felt was lacking in GCD's QoS system. A 3rd-party library or some modular code may be able to say what the priority of some detached Task is relative to it, but it doesn't know about its importance to my overall application.

Perhaps that is a situation where you would need to feed the library some custom executor which maps its 5 levels of priority to a range which I think is appropriate for it, although that may no longer be possible if actor inheritance is not allowed.

hisekaldma · April 16, 2021, 1:58pm

I’ve been meaning to give this a deeper dive, but haven’t found the time. Since the review period is up, I’ll just mention the one thing that sticks out to me: withTaskGroup(of:) reads pretty awkwardly and doesn’t really promote clarity at the point of use. I’m still not sure I grok this 100%, but it seems to me that something like the following would do a better job at showing where concurrency is introduced, and what the type argument is for:

  await withConcurrentSubtasks(returning: CookingStep.self) { subtasks in
    subtasks.spawn {
      try await .vegetables(chopVegetables())
    }
    subtasks.spawn {
      await .meat(marinateMeat())
    }
    subtasks.spawn {
      await .oven(preheatOven(temperature: 350))
    }

    for await finishedSubtask in subtasks {
      switch step {
        case .veggies(let v): veggies = v
        case .meat(let m): meat = m
        case .oven(let o): oven = o
      }
    }
  }

The group result type doesn’t seem like it needs to be an argument to the function: it can be inferred or specified using as. This would also be more in line with other with functions in the language, e.g. withUnsafeBufferPointer, which don’t take a return type argument.

ktoso · April 16, 2021, 2:35pm

Thanks, that’s an interesting naming idea. I’m not sure this makes it easier I talk about these things… maybe. It feels weird to not give this concept its own name though. Child tasks come in many shapes — including “spawn let” or perhaps other ways to spawn sub tasks. So it feels weird and somewhat wrong to claim the “with child tasks” name just for this single shape of it.

A bunch of spawn lets is also concurrent. As would be any other keywords we might want to introduce… (I’m long wishing for a “send” similar to “detach” but without awaiting and making a child task…). So in a way it really feels this needs a name, and not just some “here’s a bunch of child tasks.” Though your snippet does read well, that is true.

It has a default value, so you never have to set it, but you can if it is convenient to do so.