-
Notifications
You must be signed in to change notification settings - Fork 873
Fix download directory concurrency issue and refactor #4180
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,9 +13,12 @@ | |
| * permissions and limitations under the License. | ||
| */ | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Amazon.Runtime.Internal.Util; | ||
|
|
||
| namespace Amazon.S3.Transfer.Internal | ||
| { | ||
|
|
@@ -24,6 +27,11 @@ namespace Amazon.S3.Transfer.Internal | |
| /// </summary> | ||
| internal static class TaskHelpers | ||
| { | ||
| private static Logger Logger | ||
| { | ||
| get { return Logger.GetLogger(typeof(TaskHelpers)); } | ||
| } | ||
|
Comment on lines
+30
to
+33
|
||
|
|
||
| /// <summary> | ||
| /// Waits for all tasks to complete or till any task fails or is canceled. | ||
| /// </summary> | ||
|
|
@@ -33,7 +41,10 @@ internal static class TaskHelpers | |
| internal static async Task WhenAllOrFirstExceptionAsync(List<Task> pendingTasks, CancellationToken cancellationToken) | ||
| { | ||
| int processed = 0; | ||
| int total = pendingTasks.Count; | ||
| int total = pendingTasks.Count; | ||
|
|
||
| Logger.DebugFormat("TaskHelpers.WhenAllOrFirstExceptionAsync: Starting with TotalTasks={0}", total); | ||
|
|
||
| while (processed < total) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
@@ -48,7 +59,12 @@ await completedTask | |
|
|
||
| pendingTasks.Remove(completedTask); | ||
| processed++; | ||
|
|
||
| Logger.DebugFormat("TaskHelpers.WhenAllOrFirstExceptionAsync: Task completed (Processed={0}/{1}, Remaining={2})", | ||
| processed, total, pendingTasks.Count); | ||
| } | ||
|
|
||
| Logger.DebugFormat("TaskHelpers.WhenAllOrFirstExceptionAsync: All tasks completed (Total={0})", total); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -64,6 +80,9 @@ internal static async Task<List<T>> WhenAllOrFirstExceptionAsync<T>(List<Task<T> | |
| int processed = 0; | ||
| int total = pendingTasks.Count; | ||
| var responses = new List<T>(); | ||
|
|
||
| Logger.DebugFormat("TaskHelpers.WhenAllOrFirstExceptionAsync<T>: Starting with TotalTasks={0}", total); | ||
|
|
||
| while (processed < total) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
@@ -79,9 +98,92 @@ internal static async Task<List<T>> WhenAllOrFirstExceptionAsync<T>(List<Task<T> | |
|
|
||
| pendingTasks.Remove(completedTask); | ||
| processed++; | ||
|
|
||
| Logger.DebugFormat("TaskHelpers.WhenAllOrFirstExceptionAsync<T>: Task completed (Processed={0}/{1}, Remaining={2})", | ||
| processed, total, pendingTasks.Count); | ||
| } | ||
|
|
||
| Logger.DebugFormat("TaskHelpers.WhenAllOrFirstExceptionAsync<T>: All tasks completed (Total={0})", total); | ||
|
|
||
| return responses; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Executes work items with limited concurrency using a task pool pattern. | ||
| /// Creates only as many tasks as the concurrency limit allows, rather than creating | ||
| /// all tasks upfront. This reduces memory overhead for large collections. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This method provides a clean way to limit concurrent operations without creating | ||
| /// all tasks upfront. It maintains a pool of active tasks up to the maxConcurrency limit, | ||
| /// replacing completed tasks with new ones until all items are processed. | ||
| /// The caller is responsible for implementing failure handling within the processAsync function. | ||
| /// </remarks> | ||
| /// <typeparam name="T">The type of items to process</typeparam> | ||
| /// <param name="items">The collection of items to process</param> | ||
| /// <param name="maxConcurrency">Maximum number of concurrent tasks</param> | ||
| /// <param name="processAsync">Async function to process each item</param> | ||
| /// <param name="cancellationToken">Cancellation token to observe</param> | ||
| /// <returns>A task that completes when all items are processed, or throws on first failure</returns> | ||
| internal static async Task ForEachWithConcurrencyAsync<T>( | ||
| IEnumerable<T> items, | ||
| int maxConcurrency, | ||
| Func<T, CancellationToken, Task> processAsync, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| var itemList = items as IList<T> ?? items.ToList(); | ||
| if (itemList.Count == 0) | ||
| { | ||
| Logger.DebugFormat("TaskHelpers.ForEachWithConcurrencyAsync: No items to process"); | ||
| return; | ||
| } | ||
|
|
||
| Logger.DebugFormat("TaskHelpers.ForEachWithConcurrencyAsync: Starting with TotalItems={0}, MaxConcurrency={1}", | ||
| itemList.Count, maxConcurrency); | ||
|
|
||
| int nextIndex = 0; | ||
| var activeTasks = new List<Task>(); | ||
|
|
||
| // Start initial batch up to concurrency limit | ||
| int initialBatchSize = Math.Min(maxConcurrency, itemList.Count); | ||
| Logger.DebugFormat("TaskHelpers.ForEachWithConcurrencyAsync: Starting initial batch of {0} tasks", initialBatchSize); | ||
|
|
||
| for (int i = 0; i < initialBatchSize; i++) | ||
| { | ||
| var task = processAsync(itemList[nextIndex++], cancellationToken); | ||
| activeTasks.Add(task); | ||
| } | ||
|
|
||
| // Process completions and start new tasks until all work is done | ||
| while (activeTasks.Count > 0) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| var completedTask = await Task.WhenAny(activeTasks) | ||
| .ConfigureAwait(continueOnCapturedContext: false); | ||
|
|
||
| // Propagate exceptions (fail-fast behavior by default) | ||
| // Caller's processAsync function should handle failure policy if needed | ||
| await completedTask | ||
| .ConfigureAwait(continueOnCapturedContext: false); | ||
|
|
||
| activeTasks.Remove(completedTask); | ||
|
|
||
| int itemsCompleted = nextIndex - activeTasks.Count; | ||
| Logger.DebugFormat("TaskHelpers.ForEachWithConcurrencyAsync: Task completed (Active={0}, Completed={1}/{2}, Remaining={3})", | ||
| activeTasks.Count, itemsCompleted, itemList.Count, itemList.Count - itemsCompleted); | ||
|
|
||
| // Start next task if more work remains | ||
| if (nextIndex < itemList.Count) | ||
| { | ||
| Logger.DebugFormat("TaskHelpers.ForEachWithConcurrencyAsync: Starting next task (Index={0}/{1}, Active={2})", | ||
| nextIndex + 1, itemList.Count, activeTasks.Count + 1); | ||
| var nextTask = processAsync(itemList[nextIndex++], cancellationToken); | ||
| activeTasks.Add(nextTask); | ||
| } | ||
| } | ||
|
|
||
| Logger.DebugFormat("TaskHelpers.ForEachWithConcurrencyAsync: All items processed (Total={0})", itemList.Count); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
removing unused constructors