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

Code quality: Await is being used inside foreach loop #10268

Closed
QuaintMako opened this issue Oct 21, 2022 · 7 comments
Closed

Code quality: Await is being used inside foreach loop #10268

QuaintMako opened this issue Oct 21, 2022 · 7 comments

Comments

@QuaintMako
Copy link
Contributor

Description

Through the code, we can find some pieces of code looking like :

foreach (obj o in objectCollection)
{
    await AsyncMethod(o);
}

It is not the most efficient way of performing the tasks, since it means we are waiting for each task to be done before performing the next one. It would be much faster to perform all the task using the Task.WhenAll process.

As an exemple, using this process inside the AddAllItemsToSidebar method allowed for a gain of 15 % in speed.

Concerned code

  • Whole solution

Gains

  • A gain in performances, depending on how often the code is used.

Requirements

Replacing

foreach (obj o in objectCollection)
{
    await AsyncMethod(o);
}

By

var tasksCollections = objectCollection.Select(o=> AsyncMethod(o));
await Task.WhenAll(tasksCollections );

Comments

No response

@yaira2 yaira2 changed the title Await inside foreach loop Code: Await inside foreach loop Oct 21, 2022
@yaira2 yaira2 moved this to 🆕 New in Files task board Oct 21, 2022
@yaira2 yaira2 moved this from 🆕 New to 🔖 Ready to build in Files task board Oct 21, 2022
@yaira2
Copy link
Member

yaira2 commented Oct 21, 2022

Marking this as ready to build

@yaira2 yaira2 changed the title Code: Await inside foreach loop Code quality: Await inside foreach loop Oct 21, 2022
@yaira2 yaira2 changed the title Code quality: Await inside foreach loop Code quality: Await is being used inside foreach loop Oct 21, 2022
@gave92
Copy link
Member

gave92 commented Oct 21, 2022

Consider that doing this in AddAllItemsToSidebar would change the order of the displayed items. Some more changes are required.

@yaira2
Copy link
Member

yaira2 commented Oct 21, 2022

I think every scenario needs to be evaluated.

@QuaintMako
Copy link
Contributor Author

I did the tests on AddAllItemsToSidebar since it was a pretty easy step to test. Each and every modification will be evaluated for sure, it works only in scenarios where the order of task execution does not matter.

@yaira2 yaira2 moved this from 🔖 Ready to build to 🏗 In progress in Files task board May 14, 2023
@yaira2
Copy link
Member

yaira2 commented Feb 6, 2024

Consider that doing this in AddAllItemsToSidebar would change the order of the displayed items. Some more changes are required.

@gave92 do you have anything specific in mind?

@gave92
Copy link
Member

gave92 commented Feb 6, 2024

It's been a while sorry I do not recall :)

@yaira2
Copy link
Member

yaira2 commented Feb 6, 2024

How about this?

await Task.WhenAll(FavoriteItems.AsParallel().AsOrdered().Select(path => AddItemToSidebarAsync(path)));

Edit
This is 90% faster on my device but it adds items in the wrong order.

@yaira2 yaira2 moved this from 🏗 In progress to 🔖 Ready to build in Files task board Feb 6, 2024
@yaira2 yaira2 moved this from 🔖 Ready to build to 🏗 In progress in Files task board Feb 6, 2024
@yaira2 yaira2 closed this as completed Apr 14, 2024
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Files task board Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

4 participants