SE-0311 (3rd review): Task-local values

Evolution Proposal Reviews
1

Hello, Swift community.

The third review of SE-0311: Task-local values begins now and runs through June 9th, 2021.

The second review ended several weeks ago; I apologize for the fact that the community has been left hanging.

The second review of SE-0311 was run to consider a significantly different language approach for binding and accessing task-local values, one based around property wrappers rather than an explicit key type. Shortly after the second review started, the community provided strong feedback that the proposal's new use of property wrappers didn't match their expectations because of the indirection through a wrapper value. The author agreed to re-revise the proposal, and the review was extended.

The Core Team has tried to evaluate the community's feedback as it applies to that revised second proposal, understanding that it's not always easy to pick apart. The major thrusts are as follows:

  • Overall, the community strongly supports the new use of property wrappers.

  • There's some discomfort over the verbosity and nesting of the with-style scoped API for installing a new task-local value. The author believes that scoping is an important part of the basic design. The Core Team agrees and believes that we can investigate ways of improving the syntax for this sort of scoped change to a value.

  • There's some discomfort over the fact that it's not possible to mutate the current value. If the new value needs to be derived from the current value, e.g. by appending to an array, the old value must be copied, modified, and then re-installed, which will generally defeat the standard library's copy-on-write optimizations. This is an inevitable trade-off of the scoped approach because the old value must be restored when the scope exits. The Core Team believes that it would be an interesting future direction to consider a complementary feature which maintains a stack of values for a key; reading a key would then produce a sequence of values rather than a single one. This would also allow some clients to be much lazier when installing values; for example, code could install and look up individual entries in a dictionary without ever having to construct a full Dictionary value for all the installed keys.

  • Some community members felt that it was odd that you could set a default value for a task-local, as opposed to the task-local being forced to be optional. The author believes that default values avoid a lot of practical inconveniences with optionals, such as needing to spell the desired default at every use site with a ?? operator. The Core Team discussed this and agrees that having a default value is the right design rather than forcing the use of optionals.

  • Some community members feel that it's odd that values can be read in any function but that new values can only be set in async functions. This kind of contextual value isn't solely useful in async code; many sorts of APIs can benefit from it. Moreover, the current set of task-local values are captured during the creation of certain kinds of non-structured task, and so it's meaningful to be able to install values that will be captured this way. The Core Team agrees, and the author has agreed to revise the proposal to allow this.

    However, please note that the feature is still driven by the use case of task-local values, which imposes certain constraints that might be unsuitable for other use cases of contextual values or thread-local storage. Most importantly, task-local value types must be Sendable, which means that the feature cannot be used to e.g. install a contextual reference to a mutable class instance (at least, not without bypassing Sendable checking).

The Core Team has elected to run this third review in order to consider this final change, where values can be installed even in synchronous code. The Core Team is otherwise prepared to accept SE-0311 as proposed. However, because of the rather complex schedule of revisions, we believe it is possible that some members of the community are confused about what exactly is proposed, and so we do still invite feedback on the overall proposal.

This review is part of the larger concurrency feature , which we are reviewing in several parts. While we've tried to make it independent of other concurrency proposals that have not yet been reviewed, it may have some dependencies that we've failed to eliminate. Please do your best to review it on its own merits, while still understanding its relationship to the larger feature. However, if you see any problematic interactions with proposals that have already been reviewed, that are currently in review, or that are currently being pitched, that is highly valuable feedback.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager. If you do email me directly, please put "SE-0311" somewhere in the subject line.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

As always, thank you for contributing to Swift.

John McCall
Review Manager

15 Likes
2

Thank you for saying this and to you and all others for all your hard work on concurrency. I’m sure you and your team will make up for this delay by this time next week :wink:

4 Likes
3

+1 on the overall proposal. I've been using task-locals for distributed tracing since the first proposal where they prove to be invaluable for implicit context propagation and enjoy the new syntax introduced in the second revision.

I agree that requiring the with-API for this use-case makes sense as it makes the scope of a task-local value very clear.

In Distributed Tracing, we hit a similar "issue" with indentation, where we also offer a withSpan API to ensure proper error handling and automatic ending of a span. At least for this use case, I think something like function wrappers could make it a bit easier and remove the extra level of indentation.

2 Likes
4

This all LGTM. Thank you to the proposal author for the detailed iteration on this, this is a great and important piece of the Swift Concurrency model!

One question since John raised the topic:

Most importantly, task-local value types must be Sendable , which means that the feature cannot be used to e.g. install a contextual reference to a mutable class instance (at least, not without bypassing Sendable checking).

Is there a potential future extension to allow this? Would this just be a TLV sort of thing that isn't inheritable across tasks, or something more?

-Chris

1 Like
5

Thanks for the reviews over the last few rounds, this has shaped up quite well.

That specific capability "don't inherit" was something that came up in a few contexts and is definitely something we can do in the future.

With the property wrapper approach it may be a bit weird how to express this, we may need a new wrapper? Because the goal with @TaskLocal would be to cause compile time Sendable-check failures once we're ready to enable them across the board, so it'd be hard to not do that check for @TaskLocal(inherit: .never), but it's something we could do in some way for sure.

6

Yeah, it seems to me that it would make more sense for that to be a different property wrapper, just one with (probably) very similar structure.

Similarly, I think a multi-valued feature would be a different wrapper, something like @TaskLocalStack ("stack" referring to the abstract data structure, not the language-implementation concept).

3 Likes
7

+1 for a different wrapper. I was just hoping it could leverage the same runtime mechanics etc under the hood. In any case, a discussion for another day, I agree it is a "similar but different" thing.

1 Like
8

Yeah, it could use the same storage and general runtime I believe :+1:

Agreed on delaying that discussion though :slight_smile:

9

Big +1 to this! I'm looking forward to libraries adopting TLVs to make tracing distributed systems much easier and cleaner.

One thing I'd be interested to see is how (if at all) we can migrate libraries and frameworks over to TLVs from their current worlds. Specifically in my case SwiftNIO's EventLoopFutures. Is it a case that it's not possible to combine the two or is it simple?

I know TLVs can be used outside of a defies Task but will that translate to ELFs if we guarantee the work stays on the same event loop? This would allow frameworks like Vapor to integrate TLVs for tracing and offer external async APIs, even if they're all still future based under the hood and just use NIO concurrencys .get() to provide an async API.

Or will this simply not work and we'll have to ensure that TLVs are used (or at least will only work) when calling through an entirely async stack.

1 Like
10

Hey Tim!

Marrying these features with NIO is slightly weird indeed. This is because NIO takes full control of its threads (in the form of EventLoops).

The synchronous storage part of this API may weirdly enough "just work" here to be honest. This is because NIO controls its own threads, until we have custom executors, we can't have a Task that is bound to a NIO controller EventLoop etc.

So the "structured" approach here can't work in NIO in that sense. Creating a new task off the side to call into the channel pipeline also is not right, because we'll hop back to the EL and lose the Task there.

However... is it the right thing to do? Actually probably not. Because many connections may be handled on the same EventLoop, and so setting something "in the thread" (event loop) is actually wrong because it'd be a per connection thing... So we'd have to maintain some dictionary of [Channel: <data>] as the TLV. I guess that may be viable?

On the JVM, Netty solves these things by having an Attributes (an AttributeMap) available on all channels. AttributeMap (Netty API Reference (4.1.82.Final))

This of course has no relation to structured concurrency since that's not a thing on the JVM (well, at least yet -- a form of task locals is coming along with loom). But it'd be a way to perhaps look at this...

Long story short: I don't think we have a great answer here yet, and task-locals work great for when the API is using swift concurency "style", and NIO (along with it's futures and pipelines) is a bit odd in that world yet. I think this is actually fine because NIO always was intended to be a low level thing and we can survive a difference in abstraction levels between the two -- at least until we figure out how to make the two collaborate more nicely. I think we'll have a chance to do this with custom executors - as then tasks would be ON the event loops, and things would just work.

2 Likes
11

Was something omitted here?

12

Reformatting broke the flow here.

The continuation is “ Because NIO controls its own threads…”

Will change; done, moved the sentence one line up.

1 Like