-
Notifications
You must be signed in to change notification settings - Fork 874
optimize task creation #4219
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
optimize task creation #4219
Conversation
aeec2e8 to
ef08cbf
Compare
610a889 to
91abd13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR optimizes task creation in multipart downloads by limiting concurrent task creation to match the configured HTTP concurrency limit. Previously, all download tasks were created simultaneously and competed for HTTP slots, which could create thousands of tasks for large files. The optimization moves the semaphore acquisition outside of task creation into the main loop, ensuring only ConcurrentServiceRequests tasks exist at any time.
Key Changes:
- Moved
_httpConcurrencySlots.WaitAsyncfrom insideCreateDownloadTaskAsyncto the task creation loop inStartDownloadsAsync - Added error handling to release HTTP slots if task creation fails
- Updated comments to reflect the new control flow
sdk/src/Services/S3/Custom/Transfer/Internal/MultipartDownloadManager.cs
Outdated
Show resolved
Hide resolved
sdk/src/Services/S3/Custom/Transfer/Internal/MultipartDownloadManager.cs
Outdated
Show resolved
Hide resolved
ef08cbf to
6574852
Compare
6574852 to
99f0783
Compare
99f0783 to
07354cd
Compare
Stacked PRs:
Description
Previously we would n number of tasks, where n is the number of parts all at the same time, and each task would then fight for an http slot. This was fine, but in cases where there are lots of parts (e.g. 10,000) this would mean the .net task scheduler would have to create 10,000 tasks and manage them all. Instead, we only create up to
concurrentservicerequeststasks at a time. This is done by moving the_httpConcurrencySlots.WaitAsyncoutside of the task creation and into the for loop that creates the task itself.I also updated the cancellation token logic to use the linked token when applicable. This would apply to scenarios where if one part fails to download we want it to cancel the other linked tokens
Motivation and Context
#3806
Testing
Types of changes
Checklist
License