[Concurrency][Pitch] Task Local Values

No strong preference, was just "to look similar to swift ui", I'm a fan of "consistency is king :crown:". But you may be right, with Task we do a lot of nesting already in other proposals after all. If we're happy with the namespaces (awaiting core team feedback on the general style), I'd be happy to do Task.LocalValues as the namespace -- good idea :+1:

1 Like

I'd be open to that, not sure what the core team and style guide would say about this style of nesting. It may be harder to know what Key is without an IDE, but honestly that's a minor concern -- those are always defined in a small extension like you propose.

Yeah could be fine, pending on what style of key definitions the core team will be happy with.

1 Like

Is it possible to detect if the task-local storage is accessed in a non-async function at runtime?

If it is, maybe we can have read returns default value on non-async context, and have only bind function be async.

I'm sorry but I don't see what the gain of that would be?

Please refer to https://github.com/ktoso/swift-evolution/blob/wip-tasklocals/proposals/nnnn-task-locals.md#access-from-non-async-functions which outlines future direction how accessing those values will be implemented once we can get to a task from a non-async function. We need the actual values, not just returning the default.

Sorry, I was exploring what can be done on the non-async side since I don't exactly like UnsafeCurrentTask. Ideally, I'd like it if we can combine Scheme's environment (on the non-async side) with the task-local value (on the async side). Especially that, as is, task-local values don't work on non-task contexts at all.

It doesn't help that I don't quite understand the mechanism behind task-local storage. So I don't even know if we can combine the two. As far as my investigation goes, things pivot around Builtin.getCurrentAsyncTask().

Awesome. This is so great, I am thrilled to see this proposal. I'm sorry for the delay getting a chance to read this, but hopefully better late than never. If this is too late, then I can bring up similar points in the formal review threads.

Task local values set in a task cannot out-live the task, solving many of the pain points relating to un-structured primitives such as thread-locals, as well as aligning this feature closely with Swift's take on Structured Concurrency.

Nice.

Tasks already require the capability to "carry metadata with them," and that metadata used to implement both cancellation, deadlines as well as priority propagation for a Task and its child tasks.

Yep!

Task local values may only be accessed from contexts that are running in a task: asynchronous functions. As such all operations, except declaring the task-local value's handle are asynchronous operations.

Is this imprecision in the wording, or do you mean this literally? Synchronous functions can be called from async functions and would like to access task-local values just like async functions do. This is commonly seen in builder APIs (e.g. S4TF, SwiftUI, LLVM IR wrappers, etc) where you want to have an implicit task local context and don't want to pass it around explicitly all the time.

For example, it would be untenable in S4TF or other machine learning frameworks to have to pass context objects into calls like Tensor<Int>(42), but such calls need access to task local state (so you know the current device etc) and it would be impractical to force all builder style APIs to be in async functions.

I don't think it is acceptable to punt this to Future Directions. This is core to the proposal, and the proposal isn't very useful without solving this.

I don't understand this at all. The only reason for a function to be marked async is if it suspends. Access to a task local value never does that, so I don't see why it would be marked async.

I understand your design point of being for library authors (e.g. like the internals of S4TF) but this seems really awkward to use in practice. Why not have a more direct getter/setter style of API, possibly wrapped up with keypaths or other nice things to make them beautiful?

Task local values are retained until withLocal 's operation scope exits. Usually this means until all child tasks created in such scope exit.

This is a great goal, I agree that task-local values should be bounded by the lifetime of the task. This implies that tasks created by the structured concurrency affordances should be destroyed when their tasks are destroyed.

That said, I don't understand the need for this complexity with the withLocal stuff. Why can't I just have unstructured access to a task-local value with a get/set value style of API? You're already handling the case where you get a value that isn't already initialized.

Reading your "rejected alternatives" section, I agree with you that it isn't correct to just use thread local or dispatch local values naively. The solution is that the "Executor" runtime needs to be able to provide an opaque token from a sync function executing in its context for the task-local dictionary to hang off. If that executor is implemented in terms of pthreads then it can use pthreads TLS to do this. If it is implemented in dispatch, then it can use dispatch local values to implement this. If it is implementing its own mechanism, then that mechanism can handle this.

In any case, none of the executor implementation details should be exposed to the Swift code (either in syntax or in ABI) because the Swift code should be compatible with multiple different executors.

.... oh I see you address this in future directions, I agree that something like unsafeCurrent is the way to go. It isn't clear to me why it is unsafe though. You say:

The Unsafe part of the name originates from the fact that it is NOT safe to modify such task object from other tasks, and such developers must not pass such task object to other tasks.

This is also concerning to me. Why are you conflating mutability into this? Swift has well defined ways to handle mutability, why do we need another one? There should be nothing unsafe about getting an opaque task handle. Such a thing is the basis for a lot of functionality (e.g. debugging and reflection APIs etc).

While actors are out of scope for this proposal, it should also be possible to get a pointer to the current actor (or nil if not on one) from sync code as well for the same reason.

Task local variables are semantically inherited the same way by child tasks similar to some other properties of a task, such as priority, deadline etc. Note that when storing reference types in task-local storage, they can be read by child tasks of a task, meaning they should SHOULD be either immutable, or thread-safe by some other means.

Please refine the proposal to tie into the ConcurrentValue proposal here. We need one safe and consistent way to handle/reject transfers across concurrency domains.

It is important to keep in mind the intended use case of this API. Task local values are not indented to "avoid passing parameters because I'm lazy" because generally the implicit propagation of values makes the application harder to reason about.

Just a suggestion on the writing, but I don't think it is productive to imply laziness. There are perfectly legit reasons to use "global" or "task local" state. It is also (factually) a thing that would be abused in some cases, but if you're going to try to address this concern, then please do so in a way that is more based on technical concerns than a character insult.

-Chris

5 Likes

Thanks a lot for the review, Chris - great to have some in-depth discussion on this :slight_smile:

It's just on time, as this was first posted to highlight missing things in the Task proposal (which you also notice in this review :slight_smile:) and since this was posted the missing things we ironed out in Task APIs and the upcoming Structured Concurrency Pitch #3 will have all APIs which enable this proposal to be implemented properly (i.e. also working from within sync functions).

At the time of writing this in December it wasn't clear what will happen with the Task / unsafeCurrent APIs if they'll happen or not, so the linked complete implementation and pitch were not able to implement the full potential of this pitch yet -- thankfully now soon we'll be able to as the calling convention and ABI of tasks/executors is changing to manage a thread local for the Task so it can be used for Task.unsafeCurrent which enables great updates to these APIs).

So timing is good, thanks for chiming in, let's dive in...


Usage in synchronous functions (coming very soon)

Yeah, I absolutely agree!

Back in December when this was implemented and pitched I had my hands tied somewhat by being unable to implement this given the restrictive state of the Task APIs. Access to tasks is currently implemented as "magical extra parameter to async functions," making it impossible to implement accessing a task from a sync function. It is indeed a huge, feature-breaking even, limitation...

Thankfully, during Pitch #2 of Structure Concurrency in the last weeks (this design) we were able to hash out the missing and misaligned APIs which resulted in: https://github.com/DougGregor/swift-evolution/pull/48 which will be Pitch #3 of those APIs.

This implies changes in ABI/calling-convention how a Task is passed around, so it's still not implementable until that is changed, but at least the team seems to be agreeing on adding Task.unsafeCurrent so finally we can express task locals within synchronous functions and this proposal becomes wholly useful then :partying_face:

The writeup above I hope answers this: this was the only way to implement this. It sucks, and it will be fixed in Pitch #3 with introduction of Task.unsafeCurrent :+1:

Indeed, Task.local(\.x) becomes non-async then :+1:


withLocal { ... } spelling

Reads are trivial and solved, as you say.

It is the "binds" that must be structured. The reason it can't be "just a setter" you kind of said yourself -- it would not be structured.

As usual... with enough compiler magic, everything is doable, and we could implement it as:

func foo() {
  // yeah we could even do Task.local.foo = 222, 
  // I did exactly such API for swift-distributed-tracing's attributes;
  Task.local(\.foo) = 222 // "pushes" a binding to task-local bindings stack

  // every binding magically generates:
  // @synthesized defer { Task._popTaskLocalValue() }
  // which must "pop" the binding off the task-local bindings as we return

  assert(Task.local == 222)
}

Task.local(\.foo) = 111
foo() // NOT a child task; foo is 222 inside there.
assert(Task.local(\.foo) == 111) // this assertion MUST hold

So while this may be "less noise" it is definitely a lot more magic and surprising: we're writing to something that looks static, but it'll automagically isn't and unbinds at the end of the scope...

Whereas the with style has lots of prior art and feels like pretty normal day-to-day Swift. There is no need to explain any magical semantics when presenting the following code:

func foo() {
  let before = Task.local(\.foo)

  Task.with(\.foo, boundTo: 222) {
    assert(Task.local(\.foo) == 222)
  }

  assert(Task.local(\.foo) == before)
}

This both enforces the right semantics, and informs the programmer with the shape of the API what's going to happen here. Is pretty much "normal" swift code and no-one has to jump through learning the specialized magical semantics of scoped task local values to understand this. Therefore, I think, the body/scope of withLocal(...) {...} is not noise, but helpful information.

So I really believe the less magic version is much preferable here, because it is more true to what is actually happening.

"currentActor"

While actors are out of scope for this proposal, it should also be possible to get a pointer to the current actor (or nil if not on one) from sync code as well for the same reason.

Yeah that's absolutely useful and probably one of the main things we'd spend time on when instrumenting Akka :wink: It helps a lot in debugging/logging and just understanding what's going on in the system.

+1 overall, but let's keep this separate from this proposal. I do suspect though we'll end up having such thing. Probably through the Task.current APIs :thinking:


That's a specific request from @John_McCall -- this isn't really about the task local values but about the general existence of an "give me a handle with full access to everything, even APIs which should never be called from a different task". We can get into the details of that Unsafe... name and type in the Pitch #3 of Structured Concurrency which should be posted this week I hope.

The actual unsafety of it comes not really from the Task.unsafeCurrent but the fact someone could escape/store this value and then use it from a different task, that's the unsafeness we're concerned about. John wanted to eventually make a type that's "un-movable" which then could be safe, since we'd prevent people from doing this. ConcurrentValue things don't really solve this -- people could store it in some property and then touch the same property, get that UnsafeCurrentTask from a different task and poke at it unsafely.

Will do, by now the ConcurrentValue is fleshed out enough and looking good that this seems like a good time to refer to it. I agree that restricting values stored in task locals to ConcurrentValue is a good idea :+1:


Thanks for the catch here; my self-deprecating humor backfired here, will reword this -- thanks for pointing it out :+1:

Please let me know if this makes sense -- in the meantime I think it's time to prepare Pitch #2 here and base it on the existence of the upcoming Task.unsafeCurrent :slight_smile:

1 Like

Amended proposal to assume the upcoming Task.unsafeCurrent so those will be able to be used inside synchronous functions as well.

Diff here: https://github.com/apple/swift-evolution/pull/1245/commits/8733e61d9b0d02945570649a9d2f92a73b0f687b

I'll kick off a Pitch #2 thread after pitch #3 of the structured APIs is out, but we can also just continue in this thread for now.

I do wonder why it is a requirement that it be structured like this though. Global variables aren't modeled like this. It also occurs to me that it's relatively easy to implement the current scoped design on top of unstructured variables while the reverse is cumbersome and less efficient.

After looking at those examples since the first pitch, I don't find the forced scoped nature to be intuitive, neither as a concept nor as a syntax. It seems to me the learning curve would be much easier if they worked like global variables. And it's not hard to make something scoped if you need that for a particular variable.

Scoped task-local implemented on top of an unstructured task-local
@taskLocal private(set) var foo: Int = 0
func withFoo(_ value: Int, _ task: () -> ()) {
    let savedValue = foo
    foo = value
    defer { foo = savedValue }
    task()
}
func test() {
   assert(foo == 0)
   withFoo(1) {
      assert(foo == 1)
   }
   assert(foo == 0)
}

(That said, if they keep their current scoped nature, I think if they should be called "task environment values". "Task local value" sounds too close to thread-local, which has a lot of history for being a per-thread global-like unstructured variable.)

1 Like

No, there must not be any unstructured API to "set" such values with the hope that everyone actually only ever uses it in a structured way.

This is exactly the world of pain asynchronous runtimes are continuously facing when trying to apply instrumentation, yet a single user-call using the wrong pattern completely breaks all traces and instrumentation performed on the system.

I base this on both personal experience of building and instrumenting asynchronous actor and streaming runtimes, as well as specific prior art, most notably Project Loom's latest efforts which are a direct answer to the suffering of instrumenting async systems without such structure. Not mentioning the numerous threads of various tracers where the most common issue is accidentally dropping a value, forgetting to "reset it to the previous value on return" and all these things which are impossible to get wrong if we stick to scopes as the only API to bind such values.


Just one simple example to show how things break down with the unstructured access you are proposing:

// what-if unstructured writes allowed
// (semantics as proposed by @michelf in post #39)
@taskLocal private(set) var traceID: String = "none"

{
traceID = "parent"
  func inner() async { 
    assert(traceID == "parent")
    traceID = "inner"
    // isn't a child task; is not even suspending
  }
  await inner() 
    // assert traceID == parent, OK
    // ! traceID = inner, correctly, within this function we have a new "ID"
    // ! no "reset"
  await inner() 
    // assert traceID != parent, BAD (!)
}

And the only way to restore correctness here, is to force every single function that ever touches any task local to also store the previous value (causes copying, and retain/releases), and restore/reset it as it returns. This is exactly what current day, un-structured runtimes have to do. Most of them have to weave code into functions to actually get it right, which is error prone, goes wrong often and is just incredibly accidental complexity.

Even the best developers, who absolutely know about this rule get it wrong all the time and it tremendously undercuts the usefulness of instrumentations when such mistakes are made. Even worse, finding out where the mistake is is often a very difficult task, since it is the missing of a call in a very specific place that is the bug, not the presence of a wrong call.

And if "tracing" seems alien and not something you're using, consider that Instruments needs exactly the same semantics and infrastructure to weave through identifiers through asynchronous code. We must get this right, and not be swayed by it looking slightly unfamiliar.


Naming: Sure we can continue discussing names but I don't think it is reason enough to move away from "Task Local" because it sounds similar to "Thread Local", in fact that's good -- it's the "you should be using this instead of thread locals", so similarity is fine and welcome. If we had to change then "Scope Values" is a good name candidate, but I had feedback already that "Scope" is very generic, there's tons of scopes in Swift, which can lead to confusion i.e. "is any let in a scope a scope value?"

edit: typos
edit 2: added note on naming

9 Likes

That's why I made it private(set) in my example. I acknowledge that for some use cases you don't want to allow arbitrary writes, and the answer is to provide a scoped setter that guaranties the structure you want and make the storage private(set).

There's definitely a place for structured variables, I'm mostly wondering whether this should be the only option. For instance, consider a use case more like this:

// what-if unstructured writes allowed
@taskLocal private(set) var taskLog: [String] = []

try {
   taskLog.append("Beginning Task")
   func inner() async { 
     taskLog.append("Doing inner things")
   }
   await inner() 
   taskLog.append("Between two inner things")
   await inner() 
   taskLog.append("Task Done")
} catch {
   print("Something went wrong: \(error)")
   print(taskLog) // only print if something goes wrong
}

Is this something reasonable to want to implement that way?

I can also see someone wanting to refactor a single-threaded program or algorithm to make it parallelizable: add @taskLocal to all the global variables and most of the work is done. (Maybe not the best way, but it'd be quick and convenient.)

Small status update: I received some excellent feedback off-thread and will be preparing a Pitch #2 soon.

It will extend the semantics to also cover inheritance of values with detached tasks, although with very carefully specialized semantics to avoid infinitely growing task local bound value stacks. It'll help with end-to-end tracing even in presence of detached tasks.

It also means we pretty definitely do need the Key type, as we'll need to special mark some keys for storage in "always inherit" limited-space storage.

Thanks for the input and cya in Pitch #2 soon

2 Likes

Fantastic, I'm thrilled to hear this @ktoso!

:happydance: Thank you!!

Yep of course, it isn't related to this proposal.

Ok, I'd love to know more about this. It just seems odd and would be best to keep these things orthogonal. The word "unsafe" has a lot of precedent in Swift, and keeping that consistent would be good.

Thx! Also, no worries about self-deprecating humor. I didn't mean anything negative, just trying to help make the writing clear and productive for when it gets out to a wider audience in full review.


I don't understand though: why is this desirable? What does "structured" mean in this context?

I'm not arguing for more compiler magic :-) , I'm just trying to understand the model and use-case. People will think about these as "thread local" values or "task local" values, and such concepts have wide precedent in ptherads, GCD, etc. These are based on getter/setter styles of APIs. Why deviate? I think I'm just missing an important aspect of your design goals here.

Ok, I can see why you want scoped mutation so you can restore if this is a goal, but "why is this a goal of the design"? It isn't clear to me at all that this is a better design than a simple getter/setter design. Such a design makes it much more complicated and just seems odd - what is the precedent and rationale here?

Thank you for all the hard work on this!

-Chris

1 Like

Yeah ok, let’s revisit this in the Structured Concucrrency Pitch update (meh, pending, should have been out but slipped) as that’s where we propose that API. John can chime in a bit more then about his preference on the naming there... It is “concurrency unsafe” though I understand that normally in Swift “unsafe” most often meant “memory unsafe”, but then again... Swift never before talked about anything wrt. concurrency, so that’s a first.

The primary use-case of these APIs is tracing, there’s two primary use cases (which are actually the same):

  • “Instruments style” use cases which need to “for this call, attach some identifier, and carry it through all calls, async/sync and even detached tasks made from this function — such that profilers can “this call, caused all this (a)synchronous work to happen.”
    • subsequent calls (one line below that call) should not have the same identifier, so we need the “restore” / “reset” operation
  • distributed tracing — pick any specification you like, but they all need “reset” to implement correctly and efficiently. We (SSWG) have https://opentelemetry.io/ compatible tracer implementations already, but through painfully passing around the metadata around explicitly.
    • the concept is exactly the same here as the “local” tracing/profiling, but we can extend the traces through multiple machines. Imagine getting a trace from device through all services it is hitting in backend services — a tremendously useful performance analysis tool.

Both those are something we’re actively interested in solving awesomely. Though it will some more time to get the tracers, profilers to make use of this.

There’s a lot of prior art on this, I link to three in the proposal itself — how Go’s context’s form such “tree”, Kotlin’s scopes, and Loom’s Scope Variables.

The best one to check out is Loom’s Scope Variables mostly because they have the best “in single place” complete rationale writeup about the pains they’re addressing: State of Loom: Part 2 The ability to solve issues with thread locals is served really well by embracing structured concurrency, and as Swift wants to do this anyway — structured task locals are solving another easy to get wrong thing while we’re at “fixing” concurrency to make it simpler to work with.

I’ll add some more rationale on this to the Motivation section too.

1 Like

There's also UnsafeContinuation, which which I think is worth mentioning as it's already in review (and is already past the review period).

What is the status of this one? I presume, it relies on structured concurrency proposal (and we're waiting for core team conclusion on that one).

Correct, it’s waiting for Structured Concurrency to conclude. It will be up for review once that is cleared up. We’re trying to have a focused review of Structured Concurrency and Actors right now, and not adding another topic to focus on concurrently.

Not much has changed here since, the up-to-date wording remains in: [Concurrency] Task Local Values by ktoso · Pull Request #1245 · apple/swift-evolution · GitHub (fixed link)

If you want to give it a go it is available in recent toolchains under the experimental concurrency flag.

1 Like

Yep, I've been using it for some time already :) I'm just afraid the API might change (or the whole feature gets rejected, whereas it is actually critical — currently I have to pass event loop/context struct to every call, while I could just take it from the Task.local).

Very glad to hear you’re using it already!

Yeah it is fairly critical for tracing systems and other instrumentation. I had some mixed feelings about passing the event loop with it, but perhaps it’s fine... I would not recommend designing a library around that idea I think though :thinking: (specifically for the event loop passing around I mean).

I can’t say much before the review but I do think this is important enough that it’ll make it in some shape or form. The usual caveats remain — this is under an experimental flag and may change at any moment after all :slight_smile:

Please make sure to chime in the review to voice the feedback for it once it is going to be under review — we’ll ping here then of course as well too. Thanks!

I see what you did there :smirk:

1 Like
Terms of Service

Privacy Policy

Cookie Policy