Code Quality: Reduced usage of Await inside foreach loops

[gumbarros]

Resolved / Related Issues

• Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
  Related to Code quality: Await is being used inside foreach loop #10268

Validation
How did you test these changes?

• Did you build the app and test your changes?
• Did you check for accessibility? You can use Accessibility Insights for this.
• Did you remove any strings from the en-us resource file?
  • Did you search the solution to see if the string is still being used?
• Did you implement any design changes to an existing feature?
  • Was this change approved?
• Are there any other steps that were used to validate these changes?
  1. Open app ...
  2. Click settings button ...

Fixed every .ForEach async void, except two edge cases at ItemViewModel.cs,in the method UpdateDateDisplay and the property EnabledGitProperties.

[gumbarros]

Good morning @yaira2! fixed the places that you asked, the other places can run concurrently?

[yaira2]

Good morning @yaira2! fixed the places that you asked, the other places can run concurrently?

The code for clearing the jumplist can, but I'm pretty sure the other ones need to run in a specific order. It might be best to revert those changes and just add a comment for future reference.

[gumbarros]

@yaira2 I think now it's ready, the only places that have Task.WhenAll now are for clearing the jumplist can, where the order does not matter. Just clarifying the other changes, places with .ForEach with async are changed to C# foreach so the Task results can be correctly handled.

[yaira2]

Thank you for staying on top of this! I'll try to finish my review in the next few days.

Just a quick tip regarding "conversations", instead of marking them as resolved, it's best to add a comment so the reviewer can see it and complete the conversation if they are happy with the solution.

[hishitetsu]

Why are you changing LINQ ForEach to foreach loop? Since lambda expressions in ForEach are not awaited, awaiting them in foreach loop is not good for performance.

[gumbarros]

Why are you changing LINQ ForEach to foreach loop? Since lambda expressions in ForEach are not awaited, awaiting them in foreach loop is not good for performance.

https://stackoverflow.com//questions/59518243/what-is-the-proper-way-to-await-for-async-inside-of-a-foreach#answer-59518587

This answer explains well the difference.

Then we can migrate the changed .ForEach with async void to Task.WhenAll, as they were already out of order. Also async void inside .ForEach will swallow any exceptions.

[hishitetsu]

Does this mean that this PR will not only improve performance, but will also include modifications to guarantee order where order is important, even at the expense of performance?

Then we can migrate the changed .ForEach with async void to Task.WhenAll

Awaiting Task.WhenAll also affects performance compared to .ForEach with async void. Is that also factored in?

[gumbarros]

Does this mean that this PR will not only improve performance, but will also include modifications to guarantee order where order is important, even at the expense of performance?

Then we can migrate the changed .ForEach with async void to Task.WhenAll

Awaiting Task.WhenAll also affects performance compared to .ForEach with async void. Is that also factored in?

What about Parallel.ForEachAsync instead of ForEach with async void?

[hishitetsu]

The issue is whether or not to await Task.WhenAll. If not awaiting, the performance is the same as .ForEach with async void.
If there is no performance problem with await, then that's fine.

[hishitetsu]

I'm not sure if we should await this. This is deleting items from the jump list, which is different from deleting the items itself, so it may be performed asynchronously.

[gumbarros]

Agree

[hishitetsu]

Probably my last comments.

[hishitetsu]

LGTM 🎉