whenAll.then issue in then execution about then

[s4cha]

Hello @cliapis

First of all thank you for taking the time to write about your issue :)

I've tried rewriting your example on my side 👍

func testThen() -> Promise<[String]> {
    return Promise { resolve, reject  in
        var promises = [Promise<String>]()
        for matcher in 0...10 {
            promises.append(
                Promise<String> { resolve, _ in
                    DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
                        resolve("Hello")
                    }
                }
            )
        }
        Promises.whenAll(promises).then { results in
            resolve(results)
        }
    }
}

and then calling it like so:

let promise = testThen()
promise.then { results in
   print(results)
}

then block is called as expected.

Maybe one of the matcher.matchTermsAsync(from: phraseComponents) promise is failing?

Have you tried adding a onError block to check that possibility ?

By the way the example above can be simplified even further :

func testThen() -> Promise<[String]> {
    var promises = [Promise<String>]()
    for i in 0...10 {
        promises.append(
            Promise<String> { resolve, _ in
                DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
                    resolve("\(i)")
                }
            }
        )
    }
    return Promises.whenAll(promises)
}

Could you test this to see if it works on your side?

Concerning the concurrency aspect you can check WhenAll.swift implementation which is pretty straightforward.
To be honest I don't remember if DispatchGroup is serial or concurrent, its possible that you might need to wrap your promises in .concurrent dispatch queues (to be confirmed)

Cheers !

[cliapis]

Hello @s4cha , thank you for your quick reply.

I think that the problem is that the DispatchGroup.notify uses the Main DispatchQueue to send invoke the notify block.

My app is actually a web server (Server Side Swift) and the main queue's execution is busy "listening" to for new connections.

So I believe that changing the notify queue to a QOS user interactive should solve the problem.
I will check tomorrow and let you know.

If this is the case, I think the .whenAll() function should get as an optional parameter the notify queue.

Also an other minor, in some files there are missing imports for the Dispatch framework and this produces errors in Linux (Ubuntu).

Regarding concurrency, DispatchGroup is actually a counter, counting the number of started/finished jobs. So the actual code inside handles all the concurrency, so you are right! ;)

Ty.

[cliapis]

Hello again, indeed changing the notify queue solved the problem.
By the way, amazing work!! ;)

[s4cha]

Eagle eye!

I guess what we can do is call back on the calling queue if it finds one like so with a fallback on mainQueue :

let callingQueue = OperationQueue.current?.underlyingQueue
let queue = callingQueue ?? DispatchQueue.main

public static func whenAll<T>(_ promises: [Promise<T>]) -> Promise<[T]> {
    let p = Promise<[T]>()
    var ts = [T]()
    var error: Error?
    let group = DispatchGroup()
    for p in promises {
        group.enter()
        p.then { ts.append($0) }
            .onError { error = $0 }
            .finally { group.leave() }
    }
    let callingQueue = OperationQueue.current?.underlyingQueue
    let queue = callingQueue ?? DispatchQueue.main
    group.notify(queue: queue) {
        if let e = error {
            p.reject(e)
        } else {
            p.fulfill(ts)
        }
    }
    return p
}

Could you try with this alternative on your side and tell me if it fixes your issue?
If so, I'll put it in the next version :)

Thanks a ton for your time and effort !!!

[cliapis]

hmm I thought that it would work, but it didn't

The OperationQueue.current is nil so it falls back to the main queue.
I don't think that there is way to get the current queue. In the past we had the dispatch_get_current_queue but now it's deprecated.

The main queue is a good choice for iOS or macOS Apps. On the server or in command line Apps the main thread needs to be busy in order to keep your app running.

[s4cha]

Oh shoot that would have saved us :/
Ok so I guess we have to introduce an optional queue parameter as you said :)

[cliapis]

I can't think of any other solution. It's not that elegant in terms of API design and syntax, but it's an optional, so in the majority of cases it won't be used.

;)

[s4cha]

Yep, would something like this work for you ?

//
//  WhenAll.swift
//  then
//
//  Created by Sacha Durand Saint Omer on 08/04/16.
//  Copyright © 2016 s4cha. All rights reserved.
//

import Foundation
import Dispatch

public class Promises {}

extension Promises {
    
    public static func whenAll<T>(_ promises: [Promise<T>], callbackQueue: DispatchQueue? = nil) -> Promise<[T]> {
        let p = Promise<[T]>()
        var ts = [T]()
        var error: Error?
        let group = DispatchGroup()
        for p in promises {
            group.enter()
            p.then { ts.append($0) }
                .onError { error = $0 }
                .finally { group.leave() }
        }
        let callingQueue = OperationQueue.current?.underlyingQueue
        let queue = callbackQueue ?? callingQueue ??  DispatchQueue.main
        group.notify(queue: queue) {
            if let e = error {
                p.reject(e)
            } else {
                p.fulfill(ts)
            }
        }
        return p
    }
    
    public static func whenAll<T>(_ promises: Promise<T>..., callbackQueue: DispatchQueue? = nil) -> Promise<[T]> {
        return whenAll(promises, callbackQueue: callbackQueue)
    }
    
    // Array version
    
    public static func whenAll<T>(_ promises: [Promise<[T]>], callbackQueue: DispatchQueue? = nil) -> Promise<[T]> {
        let p = Promise<[T]>()
        var ts = [T]()
        var error: Error?
        let group = DispatchGroup()
        for p in promises {
            group.enter()
            p.then { ts.append(contentsOf: $0) }
                .onError { error = $0 }
                .finally { group.leave() }
        }
        let callingQueue = OperationQueue.current?.underlyingQueue
        let queue = callbackQueue ?? callingQueue ??  DispatchQueue.main
        group.notify(queue: queue) {
            if let e = error {
                p.reject(e)
            } else {
                p.fulfill(ts)
            }
        }
        return p
    }
    
    public static func whenAll<T>(_ promises: Promise<[T]>..., callbackQueue: DispatchQueue? = nil) -> Promise<[T]> {
        return whenAll(promises, callbackQueue: callbackQueue)
    }
}

PS: callingQueue is not always nil on my side so I left it there to be safe. The priority would be 1 the explicit queue, 2 the calling if found, 3 main (what it was before)

[cliapis]

Looks great!!

[s4cha]

@cliapis, This is now live in 3.0.2.

https://github.com/freshOS/then/releases/tag/3.0.2

Please excuse the delay.

Thanks a ton for your time and effort @cliapis, this is truely appreciated :)

Have a fantastic 2018 !