* What is your evaluation of the proposal?
I'm generally in favor of a modernized API overlay like this (and I've written something like it myself, albeit much simpler), but I'm hoping this proposal can go through another round or two of discussion/bikeshedding/revision before approval.
(Small note: I'm really happy about the strong-typed-ness of the Source subclasses, e.g. how mergeData is only available for Add/Or.)
In no particular order, here are some things on which I'm unclear, or not-so-+1:
- synchronously()'s block parameter should be @noescape. Perhaps more arguably, it should have a generic return type and rethrows, like autoreleasepool now does.
Both of these are present in the changes I have for this proposal. The former point is a mistake in my proposal text, the latter is an unfortunate oversight on my part in putting together the proposal document.
- The names asynchronously(execute:) and synchronously(execute:) don't seem to fit with any API guidelines I'm aware of. Did you consider including the verb in the method name?
We did. Of the number of names that we discussed, none of them were perfect. sync/async are common in other languages but don’t fit the general direction of the Swift 3 naming conventions. Using `dispatchAsynchronously` is an extremely long method name, even more so than `asynchronously`. `perform` does not capture the sync/async nature of the calls particularly well, compared to DispatchWorkItem where `perform` immediately executes the block.
(And I'm guessing that "func synchronously(work:...)" is meant to be "func synchronously(execute work:...)”?)
Right.
As another bikeshed-item, I'd vote for "Data.init(withoutCopying:...)" rather than "(bytesNoCopy:...)", and perhaps whenDone() instead of notify().
Here the init() functions closely mirror Data from Foundation, the Objective-C class is toll-free bridged to NSData and we desired a close match to the Foundation Swift API. `notify` is Dispatch-only API though, I’ll go think over that one.
- Are DispatchWorkItemFlags meant to overlay dispatch_block_flags? It would be nice to explicitly list these in the proposal.
The dispatch_block_* API is completely superseded by DispatchWorkItem in the proposal. DispatchWorkItemFlags is the equivalent to dispatch_block_flags.
- Are functions like dispatch_barrier_sync totally gone in favor of passing a .barrier flag? It would be nice to explicitly state this in the proposal.
Yes, you can supply .barrier to either `synchronously` or `asynchronously`, or create a DispatchWorkItem as a barrier item. Where possible the multiple variants of a class (dispatch_async, dispatch_barrier_async, etc) are collapsed into a single method with default arguments.
- I echo Austin's concerns about subclassability. I think it would be dangerously misleading if the classes were subclassable from user code, even if it didn't work properly.
Building at compile time will fail. So you wouldn’t get very far trying to use them, I plan to investigate adding `final` here (it’s only absent for technical reasons, as the classes originate from Objective-C).
- What of the APIs provided on Semaphore and Group objects? I'd like to see these before I vote for the proposal.
These would be transformed similarly, I will include them when updating the proposal.
class DispatchSemaphore : DispatchObject {
init(value: Int)
func wait(timeout: DispatchTime = default) -> Int
func wait(walltime timeout: DispatchWalltime) -> Int
func signal() -> Int
}
class DispatchGroup : DispatchObject {
init()
func wait(timeout: DispatchTime = default) -> Int
func wait(walltime timeout: DispatchWalltime) -> Int
func notify(queue: DispatchQueue, block: () -> Void)
func enter()
func leave()
}
- What will dispatch_set_target_queue's replacement look like look like?
extension DispatchObject {
func setTargetQueue(queue: DispatchQueue?)
}
- What about dispatch_once?
Removed. Swift already has lazy initialisation at the language level, dispatch_once is neither needed nor safe in Swift.
- Why use class funcs for the Source initializers, rather than an init on each individual subclass?
Implementation limitations. The “subclasses" are protocols would be introduced in order to attain a level of type safety around DispatchSource. We felt this was a significant enough improvement that it was worth including even with this implementation wart.
- Since DispatchSpecificKey is an object now, is there any concern with the object being allocated at the same address as an old, since-deallocated, object? (This might cause user confusion if both were used as "different" keys.)
The intention, though something that cannot be enforced, is that DispatchSpecificKey would be a global static object. I’d be interested if anyone has a pattern that more throughly enforces the allocation pattern required by the underlying API.
···
On May 10, 2016, at 11:52 PM, Jacob Bandes-Storch via swift-evolution <swift-evolution@swift.org> wrote:
* Is the problem being addressed significant enough to warrant a change to Swift?
Yes.
* Does this proposal fit well with the feel and direction of Swift?
Getting there.
* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
Medium-quick reading of this proposal. I've thought about this issue a good deal in the past, though.
Jacob
On Tue, May 10, 2016 at 9:39 PM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:
Hello Swift community,
The review of "SE-0088: Modernize libdispatch for Swift 3 naming conventions" begins now and runs through May 17. The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/proposals/0088-libdispatch-for-swift3.md
Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at
https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to the review manager.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
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@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution