Using Task.Run() for Linq - optimization or bottleneck

Question

I'm in the process of trying to optimize a few older projects, making sure they're "async all the way" and won't crash under heavier loads.

I've used variations of the snippet below and feel unsure if the Task.Run is an optimization or a possible bottleneck. This method gets quite heavy use in some of the larger forms.

public static Task<List<SelectListItem>> ToMultipleSelectListItems<T>(this List<T> items, Func<T, string> nameSelector, Func<T, Guid> valueSelector, List<Guid> selected, Func<T, string> orderBy)
{
    Task<List<SelectListItem>> selectList = Task.Run(() => items.OrderBy(orderBy).Select(item =>
                    new SelectListItem
                    {
                        Text = nameSelector(item),
                        Value = valueSelector(item).ToString(),
                        Selected = selected.Contains(valueSelector(item))
                    }).ToList());
    return selectList;
}

Example call..

model.Abilities = await (await Api.GetAbilities()).ToMultipleSelectListItems(
    x => x.Name, 
    x => x.Id, 
    model.Task.SelectedAbilitiesList, 
    x => x.OrderBy.ToString()
);

As I see it, the current thread would still need to wait for the new threads response before returning. So, under some load this would possibly create a strain on the CPU and perhaps even max out threads. I fail to see an upside.

Any feedback on best practice in this scenario would be appreciated.

`I fail to see an upside` Then why are you doing it in first place? Unless you're in a winform or WPF application and you want to offload the work from the UI thread, using a task here has no point whatsoever — Kevin Gosse, Oct 15 '16 at 22:42
@KooKiz Reason I'm asking is because I'm unsure if leaving that out would somehow hinder the "async all the way", and possibly create some other problem. Someone saw an upside and wrote this in the first place, I want to make sure removing it is the right thing. — Mackan, Oct 15 '16 at 22:48
If you are considered about performance you should replace the `List selected` with a `HashSet selected`, the `selected.Contains(valueSelector(item)` will get much better performance. — Scott Chamberlain, Oct 15 '16 at 23:27

score 4 · Accepted Answer · edited Oct 19 '22 at 03:38

There's no upside. This code will negatively impact scalability and provide no benefit at all.

Someone saw an upside and wrote this in the first place,

Nope, sorry.

It's just a less-efficient way of doing this:

public static List<SelectListItem> ToMultipleSelectListItems<T>(this List<T> items, Func<T, string> nameSelector, Func<T, Guid> valueSelector, List<Guid> selected, Func<T, string> orderBy)
{
  return items.OrderBy(orderBy).Select(item =>
      new SelectListItem
      {
          Text = nameSelector(item),
          Value = valueSelector(item).ToString(),
          Selected = selected.Contains(valueSelector(item))
      }).ToList());
}

model.Abilities = (await Api.GetAbilities()).ToMultipleSelectListItems(
    x => x.Name, 
    x => x.Id, 
    model.Task.SelectedAbilitiesList, 
    x => x.OrderBy.ToString()
);

Any feedback on best practice in this scenario would be appreciated.

Here's the relevant best practice: "avoid Task.Run on ASP.NET". To quote my Intro to Async ASP.NET article:

You can kick off some background work by awaiting Task.Run, but there’s no point in doing so. In fact, that will actually hurt your scalability by interfering with the ASP.NET thread pool heuristics. If you have CPU-bound work to do on ASP.NET, your best bet is to just execute it directly on the request thread. As a general rule, don’t queue work to the thread pool on ASP.NET.

Alright, thanks! The reason I was unsure to begin with is because I saw a few comments about "async linq", but I guess that's nothing to worry about in my case as the API-call is awaited and executed first. — Mackan, Oct 15 '16 at 23:17
@Mackan: Exactly. The `GetAbilities` I assume is a truly asynchronous (EF6-based) query, which should be asynchronous. — Stephen Cleary, Oct 15 '16 at 23:18

Using Task.Run() for Linq - optimization or bottleneck

1 Answers1

Linked