Swift Concurrency: Feedback Wanted!

Evolution Discussion
76

I agree with most of @Karl's concerns, but I also think the warts are probably acceptable in a base-level API that most people will never use. The basic issue is that the TaskGroup closure has a throwingness and return type that are independent of those run by the actual subtasks. It's hard to see how type inference could be used to simplify this.

However, I'm puzzled as to why the obvious Foundation-level wrappers don't seem to be in evidence. I hope they're coming!

For example, here's the code originally cited by @Karl again:

What I'd like to write for this instead is:

func fetchThumbnails(for ids:[String]) async throws -> [String: UIImage] {
    let pairs = ids.asyncMap { try await fetchOneThumbnail(withID: $0) }
    return try await Dictionary(uniqueKeysWithValues: pairs)
}

Here, pairs is presumably some kind of AsyncSequence with element type (String, UIImage).

This code does exactly the same thing, but without all the clutter. It takes advantage of Swift's normal type inference and of rethrows. It presumes an async-aware asyncMap on Sequence and AsyncSequence-aware inits on standard types. The implementation is, conceptually, no more complicated than the original code. (At least to the extent I can envision it, but quite possibly, I'm overlooking something. :slightly_smiling_face:)

This version has one additional advantage over the original in that asyncMap can presumably avoid forking a separate async tasklet for each of the original ids at once. I doubt that you'd want to call the original code with thousands of ids. Even if there are no extra threads involved, it's still a lot of overhead, is it not?

I understand the desire to get the fundamentals nailed down first. But it's disconcerting to see robust async/await support already present in higher-level APIs and none yet in the basic types.

Yes, yes, the people, organizations, and procedures involved in implementing these APIs are entirely separate. Nevertheless, I would hate to see the formal release ship with the original code above as the recommended interface to TaskGroups for general use.

1 Like
77

I am sure the convenience APIs will come in time. However, while it's easy to criticize the task group based map-reduce here on its own, I think it's also worth considering how it would look using unstructured tasks only; not only would you have to run a loop to launch all the child tasks, and another to await and collect the results, but you would also have to manually propagate cancellation if the task doing the map is cancelled, or if an error occurs in one of the child tasks. Forgetting to do either of those things, or doing them wrong, could lead to lots of wastage from orphaned tasks and other common problems people run into working with plain futures. So while task groups may be a primitive interface, I think they are still a better primitive interface than many of the alternatives.

10 Likes
78

@Joe_Groff wrote: ...while it's easy to criticize the task group based map-reduce here on its own, I think it's also worth considering how it would look using unstructured tasks only...

I hope I did not give offense with my comments; that was not my intent. I should have said more explicitly that despite the reservations above, I can see why the TaskGroup API works the way it does and that I don't see any obvious way to improve it.

I would like to see the base-type wrappers not "because the raw TaskGroup API is so horrible," but because the sequence/basic-type interface seems like the "real" API to me, the one that people will actually be using 90% of the time, once it becomes available.

Really, this is a marketing issue. I can write my own wrapper implementations, so it's no skin off my nose. But I'd like people seeing Swift concurrency for the first time to say, "Wow, this is so clear and elegant from top to bottom!" These folks have no real need to learn the details of task groups until they actually confront a situation that the simple wrappers wouldn't address.

4 Likes
79

Actually, it doesn’t, without additional language features to “lift” the task group into the enclosing scope.

In order to meaningfully return an AsyncSequence, the actual work would have to give up on structured concurrency and used detached tasks. If using structured concurrency, the reduce step has to happen inside withThrowingTaskGroup, or you have to gather all the results in a concrete collection.

Random ideas about structured mapreduce

The “obvious” way to do this would be a single mapreduce function taking two closures:

asyncMapReduce<IntermediateElement, Result>(
    _ initalResult: Result,
    map: (Element) async -> IntermediateElement,
    reduce: (Result, IntermediateElement) async -> Result
) async -> Result

A slightly less obvious approach could allow .asyncMap { ... }.reduce(...) { ... } syntax:

// This just stores the iterator and map block in the return value
asyncMap<T>(
    _ transform: (Element) async -> T
) async -> _PendingAsyncMap<Element, T>

struct _PendingAsyncMap<Element, T> {
    // This executes the whole map-reduce in a task group
    func reduce<Result>(_
        initialResult: Result,
        _ nextPartialResult: (Result, T) async -> Result
    ) async -> Result
}

Neither of these strikes me as very elegant.

2 Likes
80

It would definitely be useful to have convenience functions for common tasks which require TaskGroups. I could imagine a withAsyncMap(_:body:) function implemented like so:

extension Sequence {
    @inlinable
    public func withAsyncMap<T, R>(
      _ transform: @escaping @Sendable (Element) async throws -> T,
      body: (ThrowingTaskGroup<T, Error>) async throws -> R
    ) async throws -> R {
        try await withThrowingTaskGroup(of: T.self) { group in
            for element in self {
                group.async {
                    try await transform(element)
                }
            }

            return try await body(group)
        }
    }
}

It would make the fetchThumbnails(for:) function at least a bit simpler, although I still wouldn't really like it:

func fetchThumbnails(for ids: [String]) async throws -> [String: UIImage] {
    var thumbnails: [String: UIImage] = [:]
    try await ids.withAsyncMap { id in
        (id, try await fetchOneThumbnail(withID: id))
    } body: { group in
        for try await (id, thumbnail) in group {
            thumbnails[id] = thumbnail
        }
    }
    return thumbnails
}

This example shows however, that TaskGroups (and their throwing counterparts) don't play well together with rethrows. AFAIK, it's not possible to implement withAsyncMap(_:body:) with rethrows, as it would either need to use TaskGroup or ThrowingTaskGroup. Another argument in favour of typed throws (which IMHO really should have come before concurrency, as it would have made all these duplicated types and functions like TaskGroup and ThrowingTaskGroup, withUnsafeContinuation(_:) and withUnsafeThrowingContinuation(_:), etc. obsolete).

81

The group return type is inferred, but I can’t figure out how. Can the compiler not figure it out if the function signature had a default like of childTaskResultType: ChildTaskResult.Type = ChildTaskResult.self like returning returnType does?

func withThrowingTaskGroup<ChildTaskResult, GroupResult>(of childTaskResultType: ChildTaskResult.Type, returning returnType: GroupResult.Type = GroupResult.self, body: (inout ThrowingTaskGroup<ChildTaskResult, Error>) async throws -> GroupResult) async rethrows-> GroupResult

82

It’s important to note that although we don’t have yet those convenience APIs on top of task group, the task group itself is an async sequence and you can use all the sequence methods on it. I’ve found myself writing very little for loops to gather the results so many ugly examples are nicer in reality.

2 Likes
83

Do you think those ugly examples will be enhanced? And where can we look at this alternate reality?

You don't have to rush an answer of course. I just want to say it will be cool when such information will be gathered and prominently shared. It's not easy to crawl the forums or Twitter in order to find the latest good practice, but I guess this is what we have to do know.

So... How does it look like when one uses a task group as an async sequence??? :stuck_out_tongue:

84

Overall, I'm really pleased with Swift Concurrency. I'd been following the async/await discussions, but only got familiar with structured tasks and actors once the Swift 5.5 beta came out. Actors in particular provide a lot of opportunities to re-organize some of my code and hopefully improve responsiveness.

Also, the compile-time checking of MainActor is great:

image
image2140×426 39.1 KB

Compiler errors like that one will save me countless hours of debugging.

85

Have result builders been discussed for alleviating syntactic friction in task groups? I haven’t fully thought out a design yet, but a builder could basically collect async function types with @autoclosure-accepting buildBlock methods. Intermediate values would also be allowed, since let declarations are left untouched by the transformation.

86

I wish I had the time to share in a nice way all the notes and examples I've gathered during all the time I've been following the concurrency work ^^ But one doesn't have as much time as one wished! I'm sure others in the community are in the same situation and I'm sure soon somebody will have a good compilation ready :)

So... How does it look like when one uses a task group as an async sequence??? :stuck_out_tongue:

To be clear I was explicitly referring to the gathering of the results of the group. So from inside the closure, not from the outside. For example when implementing a "concurrentMap" you can see how gathering is very nicely done with some sequence operators. Of course it would be nice to have those things out of the box already but it's early days ^^ My point was just that a TaskGroup is an async sequence, and that allows you to "join" very easily.

87

What you could maybe do instead is have an asyncMap(_:reduce:) API that implements the for loop over the input to spawn tasks, then the for await step to process results:

func asyncMap<S: Sequence, T, U>(_ sequence: S, initialValue: R, _ mapBody: (S.Element) async -> T, reduce reduceBody: (inout R, T) async -> Void) -> R {
  return withTaskGroup(of: T.self) { g in
    var result = initialValue
    for element in sequence {
      g.async { await mapBody(element) }
    }
    for await childResult in g {
      reduceBody(&result, childResult)
    }
    return result
  }
}
2 Likes
88

SQLite requires all it's calls be made on the same thread, that's why I want my class to be a subclass of Thread and run the command serially through the main method. I already have this working pre-async/await. Just thinking about how I can convert it.

3 Likes
89

You may be interested in the proposal for custom executors. Requiring all work done on an actor's state to happen on a specific thread is very similar to the MainActor global actor.

5 Likes
90

While trying out the concurrency features by way of playing around with a simple iOS application, I think I have found a bug in the URLSession async additions (specifically the data(from:delegate:) function, but I have not done comprehensive testing). Is it meaningful to post about it here, or should I just file a bug report at Apple?

91

Is there any update on the interaction between async/await and result builders? I remember this point being raised in some proposal thread, but I don’t know if it was addressed and, if so, how.

92

Probably best to report using the usual channels: feedback for apple frameworks incl. foundation, and bugs.swift.org for swift issues. Thanks in advance.

93

I have a question about cancellation here. Grateful for any help. Thanks!

94

Concurrency works really great, amazing work thank you!!

My feedback so far is regarding error handling. Still feels a little archaic and hoping strongly-typed errors be revisited in light of Concurrency. This was discussed in various threads over the years, but a throw to a general purpose Error is really difficult for the callers to handle errors without knowing the implementation details and feels like a gap. This could finally rid the need of the well served Result type once and for all.

Going from this:

do {
    try await performWork()
} catch let error as MyError {
    switch error { // Exhaust all error cases
        case .invalidUser: ...
        case .expiredToken: ...
        case .etc: ...
    }
} catch {
    // Will never reach here but need to satisfy compiler
}

To this:

do {
    try await performWork()
} catch .invalidUser {
    ...
} catch .expiredToken {
    ...
} catch .etc {
    ...
}

Also another completely different feedback I have is I noticed crashes while mixing GCD. I understand this is not the intended use case, but I think this will be unavoidable when async tasks performs work using dependencies and SDKs where we have no control or knowledge of the underlying implementation. I'm wondering if changing the GCD under the hood to switch between legacy and async/await implementation based on the caller would prevent these crashes?

4 Likes
95

catch .invalidUser

Just to clarify, one can catch MyError.invalidUser already. They will just need to specify the full type, and have a blanket catch statement if they wish to be exhaustive.