New GCD API change: DispatchWorkItems should be retained by their groups


(Karl) #1

Hi,

I’d like to propose a change to the new GCD API; that DispatchWorkItems be retained by their groups (and in turn, for them to be retained by their queues). This allows for whoever created the work item to create a weak reference to it, as an indicator for whether or not it has finished executing.

For example, let’s say I have some kind of class which manages a library of files. I want to have a refresh operation which runs on a background queue, enumerating the documents and perhaps even opening them and extracting some metadata:

class DocumentLibrary {

    weak var refreshOperation : DispatchWorkItem?

    func refresh(force:Bool = false) {

        If let ongoingOperation = refreshOperation {
      if force == true {
                ongoingOperation.cancel() // force an update
            }
            else {
                return // already refreshing
            }
        }

        refreshOperation = DispatchWorkItem(….) // processes the files, returns the results on the main queue
        DispatchQueue.global().async(refreshOperation)
    }
}

This relies on the fact that weak references are thread-safe, and avoids the need for an explicit completion handler which nils the variable.

Thoughts?

Karl


(Pierre Habouzit) #2

DispatchWorkItem.init() taking a group was a mistake and we are preparing an updated proposal to remove this and instead have a queue.async() overload that can take both a WorkItem and a group. The reason why is that the semantics of having a group attached to a DispatchWorkItem() is not useful, you most of the time want to enter() the group when the WorkItem is created, and have it consumed when it has run, but that not what the implementation does, it instead will enter the group at async() time.

However, if we changed the semantics to enter() at creation time, it would mean that the WorkItem could be asynced only once, which is a huge pitfall and design issue too.

As far as your proposal goes:
- dispatch groups are a single atomic counter and have no storage to own a reference on the WorkItem, changing this would dramatically change the performance profile of that existing API which is a non starter
- anything could still have references on the DispatchWorkItem and using weak references to poll for completion is both a bad design (IMO) and fraught with peril

DispatchWorkItem come with .notify() and .wait() which are the things you should use for this. You should have a .notify() block that sets a boolean, on your class to track this.

Also using the global() queues is also fraught with another peril: thread explosion, but that’s probably outside of the scope of your feature request.

-Pierre

···

On Jul 7, 2016, at 10:01 AM, Karl via swift-corelibs-dev <swift-corelibs-dev@swift.org> wrote:

Hi,

I’d like to propose a change to the new GCD API; that DispatchWorkItems be retained by their groups (and in turn, for them to be retained by their queues). This allows for whoever created the work item to create a weak reference to it, as an indicator for whether or not it has finished executing.

For example, let’s say I have some kind of class which manages a library of files. I want to have a refresh operation which runs on a background queue, enumerating the documents and perhaps even opening them and extracting some metadata:

class DocumentLibrary {

   weak var refreshOperation : DispatchWorkItem?

   func refresh(force:Bool = false) {

       If let ongoingOperation = refreshOperation {
      if force == true {
               ongoingOperation.cancel() // force an update
           }
           else {
               return // already refreshing
           }
       }

       refreshOperation = DispatchWorkItem(….) // processes the files, returns the results on the main queue
       DispatchQueue.global().async(refreshOperation)
   }
}

This relies on the fact that weak references are thread-safe, and avoids the need for an explicit completion handler which nils the variable.

Thoughts?


(Pierre Habouzit) #3

Hi,

I’d like to propose a change to the new GCD API; that DispatchWorkItems be retained by their groups (and in turn, for them to be retained by their queues). This allows for whoever created the work item to create a weak reference to it, as an indicator for whether or not it has finished executing.

Oh also, and that is a common misconception, groups have absolutely no ownership information so this “(… to be retained by their queues)” is not something that exists or is a thing.

What does exist is that a group is never destroyed while it has unbalanced enter/leaves, and the DispatchWorkItem is definitely retained by any queue it has been async()ed to (you can async a single WorkItem to several places, however that precludes you from using .notify() or .wait()).

For example, let’s say I have some kind of class which manages a library of files. I want to have a refresh operation which runs on a background queue, enumerating the documents and perhaps even opening them and extracting some metadata:

class DocumentLibrary {

   weak var refreshOperation : DispatchWorkItem?

   func refresh(force:Bool = false) {

       If let ongoingOperation = refreshOperation {
      if force == true {
               ongoingOperation.cancel() // force an update
           }
           else {
               return // already refreshing
           }
       }

       refreshOperation = DispatchWorkItem(….) // processes the files, returns the results on the main queue
       DispatchQueue.global().async(refreshOperation)
   }
}

This relies on the fact that weak references are thread-safe, and avoids the need for an explicit completion handler which nils the variable.

Thoughts?

DispatchWorkItem.init() taking a group was a mistake and we are preparing an updated proposal to remove this and instead have a queue.async() overload that can take both a WorkItem and a group. The reason why is that the semantics of having a group attached to a DispatchWorkItem() is not useful, you most of the time want to enter() the group when the WorkItem is created, and have it consumed when it has run, but that not what the implementation does, it instead will enter the group at async() time.

However, if we changed the semantics to enter() at creation time, it would mean that the WorkItem could be asynced only once, which is a huge pitfall and design issue too.

As far as your proposal goes:
- dispatch groups are a single atomic counter and have no storage to own a reference on the WorkItem, changing this would dramatically change the performance profile of that existing API which is a non starter
- anything could still have references on the DispatchWorkItem and using weak references to poll for completion is both a bad design (IMO) and fraught with peril

DispatchWorkItem come with .notify() and .wait() which are the things you should use for this. You should have a .notify() block that sets a boolean, on your class to track this.

Also using the global() queues is also fraught with another peril: thread explosion, but that’s probably outside of the scope of your feature request.

-Pierre

···

On Jul 7, 2016, at 11:03 AM, Pierre Habouzit <pierre@habouzit.net> wrote:

On Jul 7, 2016, at 10:01 AM, Karl via swift-corelibs-dev <swift-corelibs-dev@swift.org <mailto:swift-corelibs-dev@swift.org>> wrote: