3

I was trying following code in playground, but it seems that they are not working as I expected.

Two group_async operations cause about 5~6 seconds in total on my mac.

  • When I set the timeout time to DispatchTime.now() + 10, "test returns" and "done" are both printed.
  • When I set the timeout time to DispatchTime.now() + 1 (some value make the group timed out), nothing is printed except the printing codes in two group_async operations.

What I want is to suspend the group and do some clean-up when timed out, and do some other further operations when group successfully finished. Any advice is appreciated. Thanks.

import Dispatch
import PlaygroundSupport

PlaygroundPage.current.needsIndefiniteExecution = true
let queue = DispatchQueue.global(qos: .utility)

func test() {
    let group = DispatchGroup()
    __dispatch_group_async(group, queue) {
        var a = [String]()
        for i in 1...999 {
            a.append(String(i))
            print("appending array a...")
        }
        print("a finished")
    }

    __dispatch_group_async(group, queue) {
        var b = [String]()
        for i in 1...999 {
            b.append(String(i))
            print("appending array b...")
        }
        print("b finished")
    }

    let result = group.wait(timeout: DispatchTime.now() + 10)
    if result == .timedOut {
        group.suspend()
        print("timed out")
    }
    print("test returns")
}


queue.async {
    test()
    print("done")
}
Evan
  • 430
  • 6
  • 16
  • @Rob I only tried this in playground. Do you mean the behavior would be different in both situations? – Evan Oct 11 '16 at 04:54
  • Thank you for you advice. I will try this later in my app. – Evan Oct 11 '16 at 05:15
  • Hey, @Rob. I've tested this in a new app. And the time out codes got executed. Now, I will be careful using playground to test GCD behaviors due to the inconsistency. – Evan Oct 11 '16 at 05:43

3 Answers3

3

This code snippet raises a variety of different questions:

  1. I notice that the behavior differs a bit between the playground and when you run it in an app. I suspect it's some idiosyncrasy of needsIndefiniteExecution of PlaygroundPage and GCD. I'd suggest testing this in an app. With the caveat of the points I raise below, it works as expected when I ran this from an app.

  2. I notice that you've employed this pattern:

    __dispatch_group_async(group, queue) {
        ...
    }
    

    I would suggest:

    queue.async(group: group) {
        ...
    }
    
  3. You are doing group.suspend(). A couple of caveats:

    • One suspends queues, not groups.

    • And if you ever call suspend(), make sure you have a corresponding call to resume() somewhere.

    • Also, remember that suspend() stops future blocks from starting, but it doesn't do anything with the blocks that may already be running. If you want to stop blocks that are already running, you may want to cancel them.

    • Finally, note that you can only suspend queues and sources that you create. You can't (and shouldn't) suspend a global queue.

  4. I also notice that you're using wait on the same queue that you dispatched the test() call. In this case, you're getting away with that because it is a concurrent queue, but this sort of pattern invites deadlocks. I'd suggest avoiding wait altogether if you can, and certainly don't do it on the same queue that you called it from. Again, it's not a problem here, but it's a pattern that might get you in trouble in the future.

    Personally, I might be inclined to use notify rather than wait to trigger the block of code to run when the two dispatched blocks are done. This eliminates any deadlock risk. And if I wanted to have a block of code to run after a certain amount of time (i.e. a timeout process), I might use a timer to trigger some cleanup process in case those two blocks were still running (perhaps canceling them; see How to stop a DispatchWorkItem in GCD?).

Community
  • 1
  • 1
Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • What about the requested timeout? I think that it is a good reason for using groups and wait (timeout) for this scenario. Otherwise using user defined concurrent queue and waiting on .barrier lock for all jobs to be done is a better and "solid" idea. – user3441734 May 16 '17 at 23:50
  • I respectfully disagree. My final paragraph outlines my opinion regarding a better way to do it: just create cancelable tasks/operations and have timer cancel any that are not done after certain amount of time. The "waiting w timeout" is weak pattern, IMHO (blocks an extra thread; overlooks essential cleanup tasks; introduces deadlock risks; etc.). Hey, if you want to "wait", feel free, but to suggest that it's better is debatable. – Rob May 17 '17 at 00:41
  • asyncAfter approach means "stop all jobs after timeout", but how don't run it if all other jobs are already done? That is exactly the scenario for what dispatch groups are part of libdispatch. Using wait or notify on group must be done properly, I agree. Worst what Evan did, was direct use of the global queue. Create own queue means to have the controls in your hands :-) If somebody need synchronize different task, using groups is the right way. – user3441734 May 17 '17 at 09:21
0

@Rob has really nice detail suggestions for each point. I have noticed when I run Evan's code with the tweak from Rob's notes, it seem to work in Playground. I have not tested this in an App. Notice how group is declared outside the test function so we can call the group.notify later on where we can call PlaygroundPage's finishExcution(). Also note that notify function of DispatchGroup is a great way to do any additional work after submitted task objects have completed. In PlayGround we call notify in:

import Foundation
import PlaygroundSupport
PlaygroundPage.current.needsIndefiniteExecution = true

let queue = DispatchQueue.global(qos: .utility)
let group = DispatchGroup()

func test() {
      queue.async(group: group) {
         var a = [String]()
         for i in 1...999 {
            a.append(String(i))
            print("appending array a...")
         }
         print("a finished")
      }
      queue.async(group: group){
         var b = [String]()
         for i in 1...999 {
            b.append(String(i))
            print("appending array b...")
         }
         print("b finished")
      }

   DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + 0.01) {
      print("doing clean up in timeout")
   }


}

test()
print("done")

group.notify(queue: DispatchQueue.global()) {
   print("work completed")
   PlaygroundPage.current.finishExecution()
}
Yohst
  • 1,671
  • 18
  • 37
Ohmy
  • 2,201
  • 21
  • 24
  • this code doesn't take into account the requested feature, which is to wait until all the job is finished, but no more than the maximum timeout – user3441734 May 16 '17 at 23:50
0

just to compare different approaches try this in your Playground

import Foundation

func test(timeout: Double) {
    let queue = DispatchQueue(label: "test", attributes: .concurrent)
    let group = DispatchGroup()
    var stop = false
    let delay = timeout

    queue.async(group: group) {
        var str = [String]()
        var i = 0
        while i < 1000 && !stop{
            str.append(String(i))
            i += 1
        }

        print(1, "did", i, "iterations")
    }
    queue.async(group: group) {
        var str = [String]()
        var i = 0
        while i < 2000 && !stop{
            str.append(String(i))
            i += 1
        }

        print(2, "did", i, "iterations")
    }
    queue.async(group: group) {
        var str = [String]()
        var i = 0
        while i < 100 && !stop{
            str.append(String(i))
            i += 1
        }

        print(3, "did", i, "iterations")
    }
    queue.async(group: group) {
        var str = [String]()
        var i = 0
        while i < 200 && !stop{
            str.append(String(i))
            i += 1
        }

        print(4, "did", i, "iterations")
    }
    group.wait(wallTimeout: .now() + delay)
    stop = true
    queue.sync(flags: .barrier) {} // to be sure there are no more jobs in my queue
}

var start = Date()
test(timeout: 25.0)
print("test done in", Date().timeIntervalSince(start), "from max 25.0 seconds")
print()
start = Date()
test(timeout: 5.0)
print("test done in", Date().timeIntervalSince(start), "from max 5.0 seconds")

it prints (in my environment)

3 did 100 iterations
4 did 200 iterations
1 did 1000 iterations
2 did 2000 iterations
test done in 17.7016019821167 from max 25.0 seconds

3 did 100 iterations
4 did 200 iterations
2 did 697 iterations
1 did 716 iterations
test done in 5.00799399614334 from max 5.0 seconds
user3441734
  • 16,722
  • 2
  • 40
  • 59