libdispatch renaming feedback


(Guillaume Lessard) #1

Here’s some feedback after translating some Dispatch-heavy code for the new Dispatch module.

1. I like the result. Thanks for the effort!

2. Omissions

- Can't initialize a new queue or obtain a global queue using a DispatchQoS instance. [SR-1770]

One thing I have previously done was the following:

let anotherQueue: dispatch_queue_t = <somethingvalid>
let matchedQueue = dispatch_get_global_queue(dispatch_queue_get_qos_class(anotherQueue, nil), 0)

There is no longer a way to do this.

Given that all the methods that enqueue blocks for execution have DispatchQoS parameters, this must be an oversight (I didn’t notice it when reviewing the proposal).

The following should exist:
extension DispatchQueue {
  init(qos: DispatchQoS, attributes: DispatchQueueAttributes)
  class func global(qos: DispatchQoS, attributes: DispatchQueue.GlobalAttributes) -> DispatchQueue
}
extension DispatchQueueAttributes {
  // replacing dispatch_queue_attr_make_with_qos_class:
  init(qos: DispatchQoS, attributes: DispatchQueueAttributes)
}

- DispatchTime should be Comparable [SR-1771]

- qos_class_self() and qos_class_main() do not translate to DispatchQoS [SR-1769]

Since qos_class_t presumably isn’t disappearing (it’s used by Darwin pthreads), the following are needed:

extension DispatchQoS {
  static func current() -> DispatchQoS // equivalent to qos_class_self()
  static var main: DispatchQoS // equivalent to qos_class_main()
  init(qos: qos_class_t) // perhaps with a relativePriority?
}

3. Oddities

- DispatchQueueAttributes is top-level, while DispatchQueue.GlobalAttributes is not.

DispatchQueue.Attributes would look better to my eyes.

- Similarly, DispatchWorkItemFlags is top-level; it could perhaps be DispatchWorkItem.Flags

- QualityOfService and DispatchQoS.QoSClass seem redundant. One of these could probably go.
(QualityOfService is defined under Foundation.NSObjCRuntime and is an enum with rawValue)

4. Bad translations or fixits

- dispatch_get_global_queue(qos_class_self(), 0) gets an invalid suggestion (then again there is a void there, as noted above)

- dispatch_block_create does not have a fixit pointing to the new DispatchWorkItem type.

Cheers,
Guillaume Lessard


(Matt Wright) #2

Here’s some feedback after translating some Dispatch-heavy code for the new Dispatch module.

1. I like the result. Thanks for the effort!

2. Omissions

- Can't initialize a new queue or obtain a global queue using a DispatchQoS instance. [SR-1770]

One thing I have previously done was the following:

let anotherQueue: dispatch_queue_t = <somethingvalid>
let matchedQueue = dispatch_get_global_queue(dispatch_queue_get_qos_class(anotherQueue, nil), 0)

There is no longer a way to do this.

Thanks for the feedback. I’ll take a look at getting something in that can do this.

Given that all the methods that enqueue blocks for execution have DispatchQoS parameters, this must be an oversight (I didn’t notice it when reviewing the proposal).

The following should exist:
extension DispatchQueue {
init(qos: DispatchQoS, attributes: DispatchQueueAttributes)
class func global(qos: DispatchQoS, attributes: DispatchQueue.GlobalAttributes) -> DispatchQueue
}
extension DispatchQueueAttributes {
// replacing dispatch_queue_attr_make_with_qos_class:
init(qos: DispatchQoS, attributes: DispatchQueueAttributes)
}

- DispatchTime should be Comparable [SR-1771]

I’m curious why you need to compare DispatchTime values but if you can do this via rawValue it seems like it should be Comparable, even if I can’t bring to mind an immediately useful use-case.

- qos_class_self() and qos_class_main() do not translate to DispatchQoS [SR-1769]

SR-1769 has already been brought to my attention! I think that more or less covers all the comments you had about qos_class_t <-> DispatchQoS.

Since qos_class_t presumably isn’t disappearing (it’s used by Darwin pthreads), the following are needed:

extension DispatchQoS {
static func current() -> DispatchQoS // equivalent to qos_class_self()
static var main: DispatchQoS // equivalent to qos_class_main()
init(qos: qos_class_t) // perhaps with a relativePriority?
}

3. Oddities

- DispatchQueueAttributes is top-level, while DispatchQueue.GlobalAttributes is not.

I can’t immediately recall why I made this top-level but there was a technical problem with moving it into DispatchQueue. Perhaps that no longer exist, I’ll try again.

DispatchQueue.Attributes would look better to my eyes.

- Similarly, DispatchWorkItemFlags is top-level; it could perhaps be DispatchWorkItem.Flags

- QualityOfService and DispatchQoS.QoSClass seem redundant. One of these could probably go.
(QualityOfService is defined under Foundation.NSObjCRuntime and is an enum with rawValue)

There are backwards compatibility issues with combining Foundation.QualityOfService and DispatchQoS.QoSClass. For now they need to remain separate.

···

On Jun 15, 2016, at 11:14 AM, Guillaume Lessard via swift-evolution <swift-evolution@swift.org> wrote:

4. Bad translations or fixits

- dispatch_get_global_queue(qos_class_self(), 0) gets an invalid suggestion (then again there is a void there, as noted above)

- dispatch_block_create does not have a fixit pointing to the new DispatchWorkItem type.

Cheers,
Guillaume Lessard

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution