Skip to content

Conversation

@GarrettBeatty
Copy link
Contributor

@GarrettBeatty GarrettBeatty commented Dec 10, 2025

Stacked PRs:


Description

  1. 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 concurrentservicerequests tasks at a time. This is done by moving the _httpConcurrencySlots.WaitAsync outside of the task creation and into the for loop that creates the task itself.

  2. 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

  1. Reran all tests and they pass
  2. Reran performance tests and they have same performance

Run:1 Secs:1.866644 Gb/s:23.009033
Run:2 Secs:1.363005 Gb/s:31.511007
Run:3 Secs:1.776760 Gb/s:24.173027
Run:4 Secs:0.890440 Gb/s:48.234192
Run:5 Secs:0.946861 Gb/s:45.360061
Run:6 Secs:0.940810 Gb/s:45.651798
Run:7 Secs:0.871014 Gb/s:49.309991
Run:8 Secs:0.882859 Gb/s:48.648384
Run:9 Secs:0.885273 Gb/s:48.515754
Run:10 Secs:0.866490 Gb/s:49.567436
  1. reran all integ tests
  2. dry run 16f30d8d-ec62-40bb-ab69-5f50383ed393 pass

Types of changes

  • [] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/gcbeatty/taskoptimization/2 branch from aeec2e8 to ef08cbf Compare December 10, 2025 15:52
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/gcbeatty/taskoptimization/1 branch from 610a889 to 91abd13 Compare December 10, 2025 15:52
@GarrettBeatty GarrettBeatty changed the title optimize task creation Optimize task creation for multi part download Dec 10, 2025
Copy link
Contributor

Copilot AI left a 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.WaitAsync from inside CreateDownloadTaskAsync to the task creation loop in StartDownloadsAsync
  • Added error handling to release HTTP slots if task creation fails
  • Updated comments to reflect the new control flow

@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/gcbeatty/taskoptimization/1 to feature/transfermanager December 10, 2025 18:49
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/gcbeatty/taskoptimization/2 branch from ef08cbf to 6574852 Compare December 10, 2025 18:49
@GarrettBeatty GarrettBeatty changed the title Optimize task creation for multi part download optimize task creation Dec 10, 2025
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/gcbeatty/taskoptimization/1 December 10, 2025 18:50
@GarrettBeatty GarrettBeatty changed the title optimize task creation Multi part download - optimize task creation Dec 10, 2025
@GarrettBeatty GarrettBeatty marked this pull request as ready for review December 10, 2025 19:08
@GarrettBeatty GarrettBeatty changed the base branch from GarrettBeatty/gcbeatty/taskoptimization/1 to feature/transfermanager December 10, 2025 22:39
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/gcbeatty/taskoptimization/2 branch from 6574852 to 99f0783 Compare December 10, 2025 22:39
@GarrettBeatty GarrettBeatty changed the title Multi part download - optimize task creation optimize task creation Dec 10, 2025
@GarrettBeatty GarrettBeatty changed the base branch from feature/transfermanager to GarrettBeatty/gcbeatty/taskoptimization/1 December 10, 2025 22:39
@GarrettBeatty GarrettBeatty mentioned this pull request Dec 10, 2025
7 tasks
Base automatically changed from GarrettBeatty/gcbeatty/taskoptimization/1 to feature/transfermanager December 11, 2025 14:26
stack-info: PR: #4219, branch: GarrettBeatty/gcbeatty/taskoptimization/2

add cancellation

stack-info: PR: #4221, branch: GarrettBeatty/gcbeatty/taskoptimization/4
@GarrettBeatty GarrettBeatty force-pushed the GarrettBeatty/gcbeatty/taskoptimization/2 branch from 99f0783 to 07354cd Compare December 11, 2025 14:27
@GarrettBeatty GarrettBeatty merged commit 2643108 into feature/transfermanager Dec 11, 2025
1 check passed
@GarrettBeatty GarrettBeatty deleted the GarrettBeatty/gcbeatty/taskoptimization/2 branch December 11, 2025 14:28
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.

3 participants