Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parallelize ObjectStorageProvider downloads #1485

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

petamas
Copy link
Contributor

@petamas petamas commented Sep 2, 2024

After starting to use Boost from vcpkg in our project, the number of used ports jumped from 20 to ~100. Similarly, restoring all packages from an AWS S3 backend jumped from 2 minutes to almos 10 minutes, which is way too long for us.

Since AWS has a rate limit on individual file downloads, but there's no limit on downloading files in parallel, I've decided to parallelize the download process on the ObjectStorageProvider level.

I hope this doesn't break Google Cloud Storage and Tencent Cloud Object Storage backends, but I have no way of testing them. If needed, I can add a supports_parallel_downloads() method to IObjectStorageTool, and constrain the parallelization to AWS.

@autoantwort
Copy link
Contributor

I guess this is more or less the same as in #1392?

@petamas
Copy link
Contributor Author

petamas commented Sep 2, 2024

You're right, it's basically the same. I've also experienced about 5x speedup using this parallelization, similar to @Crzyrndm.

However, if this comment is right, i.e. printing is not thread-safe, then my solution has the same issue. Unfortunately, the fix is not trivial, because extracting out_sink.println_warning() is not enough: the underlying cmd_execute_and_capture_output may also print in case --debug is passed. :-/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants