SE-0311: Task-local values

I see. Well, in that case, I'd say the previous approach was better :/

I guess you can argue that the locals will be used in code more often than they are declared, so maybe we should optimise for the use-site ...

2 Likes

I feel like I'm missing something important here, but if the property wrapper solution becomes that weird, wouldn't it be better to just have variables of type TaskLocal<T> instead? That example would then become something like

enum ExampleTrace {
    static let traceID = TaskLocal<TraceID?>()
}
ExampleTrace.traceID.get() // -> TraceID?
ExampleTrace.traceID.withValue(TraceID(...)) { ... }

Rolls off the tongue better, IMO. TaskLocal would have an init(defaultValue: T), and when T is Optional, it would also have an init() that sets nil as the default value.

1 Like

How about a property wrapper where wrappedValue: Value was made unavailable and all access thus basically forced to go through projectedValue (of type TaskLocal<Value>.Access)?

Then code using the task local would—for better or worse—all go through a $-prefixed variable, but on the other hand you could set the default value with a natural looking syntax via init(wrappedValue:) such as:

@TaskLocal var request = RequestID.empty
@TaskLocal var traceID: TraceID?  // implicit nil for ExpressibleByNilLiteral
4 Likes

That’s an interesting idea - thank you.

I don’t love the $ prefix, but at least it is consistent then, and error messages guide people to the right thing to do. It seems like a viable approach to me.

How do people feel about that?

2 Likes

Personally I feel like declaring a global variable (or inside a "namespace" enum that a dev makes up) seems too magical. I know there is a type/wrapper but still, is hard to see the connection with what's going on.

Instead I feel that the proposed solution was more clear since you extend an existing system provided namespace, so at least you see a connection. Maybe this is just because of the familiarity with SwiftUI's api.

4 Likes

I'm not very worried about this. We can always have the compiler "know" about specific property wrappers and have extra semantic checks for Swift.TaskLocal structs. The proper check is that we don't want values of Swift.TaskLocal to be declared as non-global properties or local variables. This can be enforced in the type checker with an adhoc check if needed. It isn't really associated with the property wrapper - that is just the sugar that provides the type declaration.

About the design, why do you need the redundancy with two declarations:

enum ExampleTrace { 
  @TaskLocal(default: TraceID.empty)
  static var traceID
}

struct TraceID {  
  static var empty: TraceID? = nil
}

Is it possible to condense this down to something like this?

enum ExampleTrace { 
  @TaskLocal
  static var traceID : Int
}

ExampleTrace.traceID.get() // -> Int?

This gets back to my question above about "why have a distinguished 'default' value" instead of using optionals? TLS is fundamentally a scoped key/value store like a dictionary. Dictionaries can have values of arbitrary type, but handle the missing case by returning a "optional none" instead of storing a default value. I don't understand why TLS would diverge from that widely used practice.

-Chris

1 Like

Yeah, it's pretty trivial to add this and I was about to do so right away.

I did get some some unhappy reactions about doing such one off check for these. It would be nice to get the core team chime in on this check explicitly, because various people on the team seem to have different opinions if this is a good idea or a terrible idea.

Here specifically it is to get the Int? inferred. We could not just default it to nil and get an Int? inferred.

This is the follow change mentioned here, so yes it's possible but it will look like this on use sites:

// @TaskLocal static var empty: TraceID? = nil
ExampleTrace.$traceID.get() // -> Int?
ExampleTrace.$traceID.withValue(1234) {
} 

If we do this style of API, it's at least consistent that we always have to use the $. I don't see any way around it, because how types must be used in a property wrapper and it's projectedValue / wrappedValue. As in, we can't have it work with traceID.functions as that must be the wrapped value, and the wrapped value must be the "Access".

All in all it's "okey..." at least it would be consistent to always have to access them via $ for both reading and writing, albeit is is somewhat ugly. It seems though people are used to such APIs already from existing property wrapper APIs which are very prominent in SwiftUI apps.

Replying to @filip-sakel on the PR; we try to keep the review comments in the review threads.


Would it be possible to utilize projected values to avoid the awkward Library.myTaskValue.get() call?

I don't think that's a good goal. It should be explicit in some way that you're accessing a local. They're not as cheap to access as just any other static property. The get() is a feature in that sense; giving the ability to notice that this one is a local and I better take care where and how I read it.

Instead, we could write:

enum MyLibrary {
  @TaskLocal
  static var spicePreference: SpicePreference?
}

Yeah so far I agree, the API it causes however is:

MyLibrary.$spicePreference.get() // -> Int?
MyLibrary.$spicePreference.withValue(1234) {
} 
func myFoodPrepFunction() {
  MyLibrary.spicePreference // .blackPepper

  // DESIGN #1
  // An alternative to bindTo(_:body:) is withValue(_:body:).
  MyLibrary.$spicePreference.bindTo(.redPepperFlakes) {
    MyLibrary.spicePreference // .redPepperFlakes
  }

I don't like this for two reasons:

  • inconsistency, sometimes using the $ and sometimes not... but I guess people are used to this annoyance from SwiftUI programming...?
  • the read looks "trival" but is actually relatively pretty heavy MyLibrary.spicePreference // .blackPepper this is not good IMHO as it looks just like any other static property, which suddenly has magical contextual values and also we don't notice that it's more expesive to access than just some static constant.
    • Conciseness is not helpful here for analysis of performance impact of the use of task-locals in a given piece of code. E.g. if I'd see a local used in every iteration of a loop, that's terrible and it should be moved out of it. I don't think we can rely on the compiler to always figure that optimization out, and it is helpful to make it easier to spot.
  // DESIGN #2 
  // Some alternatives to withTaskLocal(_:boundTo:body:) are: 
  // withTaskLocal(_:of:body:) and bindTaskLocal(_:to:body:)
  withTaskLocal(MyLibrary.$spicePreference, boundTo: .redPepperFlakes) { 
    MyLibrary.spicePreference // .redPepperFlakes
  }
}

This makes the discoverability of binding much harder and I'm not sure for what gain. We still need to use the $ to use the binding API.

I guess this is the proposal being torn in opposing directions by prior work and other APIs.

On one hand, I had teams / people reviewing previous "context type" work strongly request the ability to say x: Priority = .unspecified or metadata: [String:String] = [:] so they can avoid the get() ?? [:] on every single call-site of those. This is a request I received from some real world swift projects a long long time ago even as we were working the Baggage type for tracing. Those teams want to move away from passing around a "context object" and adopt these instead.

My, personal, opinion is that the more flexible API gives us both, we can x: Int? = nil or we can x: Int = 0 so either should be happy. The storage cost I'm not so concerned about since it is a single static instance of the value after all.

But that's about it -- usability of the read-sites, to not have to pollute them with ?? [:] constantly, esp. if the key author has a good idea what the default shall be: e.g. some .empty form of a type.

1 Like

Avoiding Optionals is likely the most common design challenge for Swift developers. They're a constant source of friction which, even if it's useful where properly applied, developers work around ever day. APIs which enable both optional and non-optional uses are always desired and welcome.

4 Likes

I don't speak for the core team, only for myself, but I think it would be fine to have an adhoc check for this. We aspire to have as much as possible in the library, but "perfection" shouldn't get in the way of "good".

I think it is really really important to figure out the global variable story here. This approach with property wrappers is directly gunning for the recommendation that I was making. However, if we go that far, we should just make it a language thing, and allow traceID = 1234 and print(traceID) with language support.

The other direction to go is to more directly embrace the fact that this is an indirect lookup and is inherently scoped-dictionary-like. You could enable something like Task.locals[&traceID] (safe because traceID could be guaranteed to have be a global per the check above) or potentially use keypaths.

Another related design question is: is the withValue scoping mechanism is actually important to achieving your goals? As far as I can tell, it provides a very simple approach to make the path compression stuff work nicely: if you have child tasks, they can be cranking along and the parent can go ahead and make more scoped things happen without disturbing them.

However, there are alternative implementation approaches that would make this work (e.g. use the refcount machinery like the COW implementation does). This would allow you to decouple the scoping from the value setting, allowing you to provide a properly unstructured get/set sort of interface (which would be very familiar and dictionary like) but then also provide an explicit "SaveAndRestore" style scoping operation for those that want to make a change that only effects the code below in a lexical region.

-Chris

6 Likes

Overall big +1 to this. Some variation on task-local values it desperately needed to make tracing possible server-side without needing to add yet more stuff to a request's context.

In terms of the API, I think Task.local(\.requestID) is nice and compact, though I have no strong feelings either way in how it should be accessed from a library's point of view. I do strongly feel however that we should avoid making everything optional and values should only be optional if explicitly declared as optional. Having to handle optional request IDs etc in downstream libraries will make things more complicated that they should be IMO

3 Likes

Thanks for pointing this out; I was under the impression that the revision wasn’t announced in the review thread — it’s hard to keep up with all the concurrency proposals :slightly_smiling_face:.

I am in favor of bringing back the TaskLocalValues namespace, as I think a common declaration site will help alleviate mental overhead. Perhaps it could also aid in differentiating between regular static properties and task-local values, and their execution times by extension.

Agreed. Nevertheless, I prefer avoiding property wrappers altogether, as we don't really wrap the task-local; we expose a proxy — that has the get() and withValue(_:body:) methods.

Thanks, support of this type-check would be quite welcome in the core team's review of the proposal. Thanks!

I don't think those semantics are a good idea. I've seen that proposal way back when, and still don't think it is a reasonable thing to do. Happy to talk about it in depth but that's a separate topic.

It is crucial and the key feature of the task-locals to to make the bindings scoped.

Yes it allows for this optimization, but that is not the primary point. The primary point is enforcing the scope and "popping" the value binding as the scope exits. It just so happens to allow us a very efficient implementation.

Without this, every single user, every single time, they need to operate on some value need to make the increadibly repetetive, painful, and error prone dance of: 1) set the value 2) call the thing 3) unset the thing. This gets very complex very fast and it a source of the majority of bugs in tracing systems. Some component somewhere messing up the "dance".

We have seen this already internally with people trying to make voucher requiring APIs work with async await and it was incredibly painful. I also have seen the same amount of pain in tracing systems for many years, across many communities -- including akka's own instrumentation (which had to resort to agents weaving bytecode), kotlin and attempts to use zipkin there across async await, etc.The rust ecosystem solves these with macros, which do the before/after setting.

While this is doable, it's not a good implementation strategy in general IMHO. We're doing everything we can to not have to ref-count spawn let (i.e. "very structured, known at compile time") tasks and efficiently allocate them. It would be a huge waste for all this work to be immediately rendered moot because most tasks will have some task-local and that'd cause both allocations and refcounting.

It also comes back to the point that offering an un-structured way to set/get is an anti-feature we do not want to provide. To repeat this again, the existence of an un-structured API to "set" undermine the real-world issues task-locals are designed to be solving: tracing and carrying metadata "with" computation.

2 Likes

Should all task local values must be either immutable or actor references? Is intention for this feature to make certain variables appear as global? How would it interact with reentrancy?

Not quite, this is explained in the proposal:

Values stored in task-local storage must conform to the Sendable marker protocol, which ensures that such values are safe to be used from different tasks. Please refer to the Sendable proposal for more details on the guarantees and checks it introduces.

This is sufficient to make them safe and understandable. An sendable struct as well as actor reference are sendable so they can be stored in a task-local of course, but we are not limited to those two specifically.

No. This proposal does not intend to make globals magically be task-local.

The key declarations need to be global or static but that is not the same as making things appear as global.

The feature is intended for e.g. Instruments, swift-distributed-tracing, swift-log and the OS and its voucher mechanisms to use these values behind the covers. It is very rare that normal applications will be reaching to task-locals explicitly in day-to-day code.

It doesn't interact with reentrancy at all.

The "thing" that "re-enters" is running in some task, and task-local values are bound to a specific task. As such, a task-local lookup always just uses the task identity you're looking it up from.

2 Likes

Ok, just in case. I imagined that programms will be structured as a tree, with something like func main () async { ... } with a bunch declaration in it. Then anything inside this function would be declared as local to this task (main func) such that it's possible to reference certain things as if they were 'global'. Then my question is what happens when two concurrent child tasks attempt to await some value from actors method. I still look at new documents to figure out how it all is going to work, don't be mad :)


So it's like implicits in scala?

I'm sorry if my reply came across mad(?). I don't mean to and am happy to explain and re-visit things in depth. I had hoped some quick and precise replies would be welcome / helpful.

I mean... not "anything"; Functions are free to declare any normal local variables etc. Task-locals are specifically "if is set it here, then any function I call, during the scope of a withValue { ... }"

Not really, no.

Implicit parameters are a compile time feature in Scala and they are propagated by passing a fitting (by type) value to any function that accepts such. They are not scoped like task-locals, nor are they accessible by a function that didn't explicitly request the value.

Implicit paramters in Scala (Implicit Parameters | Tour of Scala | Scala Documentation):

def main() { 
  implicit val s: String = "name"
  test() // implicitly passes `s` because the implicit parameter matches String == the type of s
  test2() // nothing is passed
}

def test()(implicit s: String) { 
  println(s) // "name"
}

def test() { 
  println(???) // no implicit String available
}

are quite different as you can see from that snippet. They are only a convenient way to pass (or "summon") parameters which exist in scopes. They are not a replacement for task or thread local storage, and instrumentation systems in Scala still use thread locals to achieve what we're proposing to solve here.

1 Like

Konrad has made some major revisions to this proposal, and the Core Team has decided to put them up for review in a new thread. Please take any further discussion there.

5 Likes
Terms of Service

Privacy Policy

Cookie Policy