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.

Answer 1

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.