0

I have a requirement to download large number of files - previously only one file could be downloaded at a time. The current design is such that when the user downloads a single file, a URLSession task is created and the progress/completion/fail is recorded using the delegate methods for urlsession. My question is, how can I leave a dispatch group in this delegate method? I need to download 10 files at a time, start the next 10 when the previous ten finishes. Right now, if I leave the dispatch group in the delegate method, the dispatch group wait waits forever. Here's what I've implemented so far:

self.downloadAllDispatchQueue.async(execute: {
    self.downloadAllDispatchGroup = DispatchGroup()
    let maximumConcurrentDownloads: Int = 10
    var concurrentDownloads = 0
    for i in 0..<files.count
    {
        if self.cancelDownloadAll {
            return
        }
            if concurrentDownloads >= maximumConcurrentDownloads{
                self.downloadAllDispatchGroup.wait()
                concurrentDownloads = 0
            }
            if let workVariantPart = libraryWorkVariantParts[i].workVariantPart {
                concurrentDownloads += 1
                self.downloadAllDispatchGroup.enter()
                //call method for download
            }
    }
    self.downloadAllDispatchGroup!.notify(queue: self.downloadAllDispatchQueue, execute: {
        DispatchQueue.main.async {
            
        }
    })
})

In the delegates:

func downloadDidFinish(_ notification: Notification){
        if let dispatchGroup = self.downloadAllDispatchGroup {
            self.downloadAllDispatchQueue.async(execute: {
                dispatchGroup.leave()
            })
        }
}

Is this even possible? If not, how can I achieve this?

Koushik Ravikumar
  • 664
  • 11
  • 26
  • If I were to use recursively: II would chuck the array of download into groups of 10 max. Then I would create a `DispatchGroup`, with `enter()`, `leave()`, `notify()` on the first chunk. On the `notify()`, I would call the method itself, on the rest of the chunk. Else, (NS)Operation could be a solution too. – Larme Oct 09 '20 at 14:34

1 Answers1

0

If downloadAllDispatchQueue is a serial queue, the code in your question will deadlock. When you call wait, it blocks that current thread until it receives the leave call(s) from another thread. If you try to dispatch the leave to a serial queue that is already blocked with a wait call, it will deadlock.

The solution is to not dispatch the leave to the queue at all. There is no need for that. Just call it directly from the current thread:

func downloadDidFinish(_ notification: Notification) {
    downloadAllDispatchGroup?.leave()
}

When downloading a large number of files, we often use a background session. See Downloading Files in the Background. We do this so downloads continue even after the user leaves the app.

When you start using background session, there is no need to introduce this “batches of ten” logic. The background session manages all of these requests for you. Layering on a “batches of ten” logic only introduces unnecessary complexities and inefficiencies.

Instead, we just instantiate a single background session and submit all of the requests, and let the background session manage the requests from there. It is simple, efficient, and offers the ability to continue downloads even after the user leaves the app. If you are downloading so many files that you feel like you need to manage them like this, it is just as likely that the end user will get tired of this process and may want to leave the app to do other things while the requests finish.


Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • This doesn't work either. I initially tried this and then added the leave () in the same queue wondering if that would work. – Koushik Ravikumar Oct 11 '20 at 05:18
  • Dispatch groups are a robust and incredibly reliable mechanism, so there must be something else going on. Perhaps your dispatch group is getting replaced by the next request? It's impossible to say on the basis of what's been provided. But the dispatch group, itself, is not the problem. That having been said, dispatch groups would not be the first tool that I'd reach for if I was trying to control the degree of concurrency for download tasks, but rather I'd wrap them in operations or, if iOS 13 and later, only, use Combine publishers (e.g. see https://stackoverflow.com/a/32322851/1271826). – Rob Oct 11 '20 at 06:26
  • Or, as I suggested, use background sessions and get out of the business of trying to manually manage batches of dispatch groups at all. But certainly dispatching the `leave` as you did in your original question is incorrect. – Rob Oct 11 '20 at 06:26
  • Yes, I see what you mean, But even if I call the leave() method from the current thread, the wait() waits indefinitely. However, the download calls are done using background session, the only reason I want to limit the calls to 10 at a time is because there is an intermediate call which fetches the download url from the server; which I do not want to overload with a lot of calls at a time. – Koushik Ravikumar Oct 12 '20 at 07:22
  • The `enter`/`leave`/`wait` are robust, reliable methods, so if `wait` is waiting indefinitely, that just means that all your `enter` calls for a particular dispatch group have not yet been offset by associated `leave` calls for the same group. I'd suggest adding debugging print statements at `enter` and `leave` calls (e.g. `print("enter", group)` and `print("leave", group)` and make sure the `enter`/`leave` calls are offset correctly. – Rob Oct 12 '20 at 20:53
  • Re not overloading your server, are you aware that `URLSession` has [`httpMaximumConnectionsPerHost`](https://developer.apple.com/documentation/foundation/urlsessionconfiguration/1407597-httpmaximumconnectionsperhost)? So, make sure you're using a single `URLSession` instance, and set the max connections property. That's how we generally ensure we don't overtax the host. It really feels like you've over-complicated this if that's the problem you're trying to solve... – Rob Oct 12 '20 at 20:56