Limit concurrent HTTP requests when downloading binary dependencies

[plu]

Limit the concurrent HTTP requests for downloading binary dependencies to 8 concurrent requests.

Motivation:

We have a fairly large Package.swift with 50 binary dependencies. In total the size of these 50 zip files is 400 MB. On a slower internet connection (16 MBit) the swift package resolve command errors out with some HTTP timeouts. It seems the main problem is that all 50 HTTP requests are executed at the same time.

Modifications:

Using some DispatchSemaphore to limit the concurrently downloaded binary dependencies to 8 concurrent requests.

Result:

In our project with the 50 binary dependencies it successfully completes the swift package resolve command. It's still open for discussion if the max 8 concurrent downloads is the right number. Also not sure if we should introduce another queue for this, reusing callbackQueue doesn't feel exactly right. However I found another TODO that's also just using callbackQueue and wondering if there should be a separate queue.

[plu]

See also: https://bugs.swift.org/browse/SR-15725

[plu]

@swift-ci Please smoke test

[tomerd]

similar to how we handle group, we should should also defer a signal on the semaphore and then another wait after L2289, otherwise the continue from L2284 may create problems

[plu]

Hmm, maybe we should do group.enter() and semaphore.wait() only once after the guard, when we're sure we're actually executing the asynchronous HTTP request? Probably I'm just missing some detail about the lines before the guard though, not understanding the full picture why they also should be part of the DispatchGroup. What do you think?

[plu]

I just pushed 97ccc8d - which is what I meant with my earlier comment. If this is wrong for some reason that I don't see yet, please let me know and I'll revert it and make the change you suggested in your first comment.

[tomerd]

yes this seems like a good change!

[tomerd]

thanks @plu, couple of follow up questions

[plu]

Thank you for the quick review @tomerd. I made one change already, and had another question related to the code around the DispatchGroup.

[tomerd]

@swift-ci please smoke test

[tomerd]

@plu would you please also create a cherry-pick into the release/5.6 branch

[plu]

Thank you for the review/approval, @tomerd. Here's the PR that cherry-picks the commit into release/5.6: #4024

[plu]

Is this something that Xcode is using directly, or should I file some radar for Xcode? I'm asking because we're also facing this issue when we do File -> Packages -> Resolve Package Versions in Xcode.

[tomerd]

@plu this will be included in Xcode in the next release cycle