[swift-evolution] [Review] SE-0088: Modernize libdispatch for Swift 3 naming conventions

[Matt Wright]

> On May 13, 2016, at 9:29 AM, plx via swift-evolution <swift-evolution at swift.org> wrote:
> 
> 
>> 	* What is your evaluation of the proposal?
> 
> +1 conceptually, some quibbles.
> 
> I agree with a few others that `synchronously` and `asynchronously` aren’t ideal; `dispatchSynchronously` or `dispatchSync` (or `performSync` or `performSynchronously`) all seem more-appropriate.
> 
> I understand the impetus behind having fewer core methods, but IMHO the `dispatch_barrier_sync` and `dispatch_barrier_async` calls ought to have direct equivalents here (even if they are just sodlib-supplied conveniences that call through to the unified method).

I don’t see having barrier as separate methods as a particularly good fit for Swift. Given that there are other options surrounding how a block operates when it is executed, it makes more sense as default parameters to the same methods. This avoids having to have variants of the block submission methods for both barrier, QoS, both, etc.

> 
> I also don’t see `dispatch_apply` here anywhere; intentional? Ideally it’d be @noescape, but handling `throw` / `rethrow` for that function in this case seems complicated.

This is here in the updated version, though I agree it’s not simple to handle throwing out of the block as multiple threads are executing the same code at once.

> 
> This next one is subjective, but I find the placement of the group-related methods somewhat backwards vis-a-vis how I think of them in terms of the C-API.

I suspect this can go either way but the block is still being executed on a given queue, you’re just associating the execution of that block with a given group. In my mind that lives on the queue, as the execution will still occur there. Contrasting this to notify, which requests that the group itself submits the notify block to the queue when the group itself is empty.

> 
> EG: I think of `dispatch_group_async` as a “method” on a `dispatch_group`, so would’ve expected this:
> 
> class DispatchGroup : DispatchObject {
> 
>  // (actual name should match chosen name convention)
>  func asynchronouslyDispatch(to queue: DispatchQueue, work: @convention(block) () -> Void)
> 
>  // (actual name should match chosen name convention)
>  func notify(on queue: DispatchQueue, using block: @convention(block) () -> Void)
> 
> }
> 
> …(and presumably the API would have manual enter/leave/wait methods and so on exposed).
> 
> I don’t feel strongly here but bring it up in case others feel similarly.
> 
> I’m a little confused about the `DispatchSpecificKey<T>` class; is it anything more than a way to "smuggle in” a generic type parameter for the associated value? 

I think it’s a little stronger than “smuggle in” here, it allows the compiler to enforce that you have the same type going into setSpecific that you get out of getSpecific (or DispatchQueue.getSpecific) for a given key. The C API here forces you to cast away from void* and I think this is a good example of how the Swift equivalent API is helping make things safer.

> 
> Also on queue-specifics, what is our expected story if we have custom destructors? Subclass `DispatchSpecificKey`? 

The value supplied to setSpecific is boxed inside a class that’s then retained. When the queue is deallocated, or the value is replaced then the box is released and normal Swift semantics for memory management will kick in on your boxed value. That way the API is still flexible enough to consume non-objects, like Ints, but also capable of taking references to classes too.

> 
> For things like `Int` specifics, I assume this API is storing auto-boxed values…? Is there any way to side-step if we use want to store an unsafe pointer? It’s not a big deal for me if we can’t under this API, TBH, but I’d at least like to see this API’s implementation and costs spelled-out more explicitly.
> 
> For `DispatchData`, is there a principled reason there isn’t something like this defined:
> 
> struct DispatchDataSegment {
>  let bytes: UnsafeBufferPointer<UInt8>
>  let byteIndex: Int
> }
> 
> extension DispatchData {
> 
>  /// Returns a sequence that enumerates the contiguous chunks,
>  /// e.g. a sequence with elements of type `DispatchDataSegment`.
>  ///
>  /// Sequence-based eplacement-for `enumerateBytes(_:)`
>  var segmentSequence: DispatchDataSegmentSequence { get }
> 
> }

This is a good path to investigate in future improvements here.

> 
> …or something analogous (instead of the proposed use dispatch_data_apply?)? 
> 
> I don’t see any API yet for setting target queues, or getting queue labels. I know the proposal isn’t documenting the APIs in full but it’s hard to evaluate in that absence.
> 
> I don’t see basic API on dispatch sources yet for things like setting event handlers, (etc.); again I know the APIs aren’t fully specified here but it’s hard to evaluate something that’s not fully specified.

These should be present in today’s updated module listing.

> 
> 
>> 	* 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
>> 
>> Thank you,
>> 
>> -Chris Lattner
>> Review Manager
>> 
>> 
>> 
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org
>> https://lists.swift.org/mailman/listinfo/swift-evolution
> 
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution