Parallel.ForEach Usage

Question

I've got the following:

[HttpPost]
public async Task<IEnumerable<PlotAutocompleteModel>> Get()
{
     IEnumerable<PlotDomain> plots = await plotService.RetrieveAllPlots();
     var concurrent = ConcurrentQueue<PlotAutoCompleteModel>();
     Parallel.ForEach(plots, (plot) =>
     {
          concurrent.Enqueue(new PlotAutocompleteModel(plot);
     });

     return concurrent;
}

With this usage, it takes about two seconds. Compared to: return plots.Select(plot => new PlotsAutocompleteModel(plot)).ToList(); which takes about four and a half seconds.

But I've always been told that for a simple transformation of a domain model into a view model, a Parallel.ForEach isn't ideal, mostly because it should be for more com-putative code. Which my usage clearly doesn't do.

Clarification: Where you would use significantly more resources, for instance you have a bitmap, a large quantity, which you have to rasterize and create new images from.

Is this the proper option for this code? I clearly see a performance gain due to the raw amount of records I'm iterating then transforming. Does a better approach and exist?

Update:

public class ProductAutocompleteModel
{
     private readonly PlotDomain plot;
     public ProductAutocompleteModel(PlotDomain plot)
     {
          this.plot = plot;
     }

     public string ProductName => plot.Project.Name;
     // Another fourteen exist.
}

Answer 1

With this usage, it takes about two seconds. Compared to... about four and a half seconds.

But I've always been told that for a simple transformation of a domain model into a view model, a Parallel.ForEach isn't ideal, mostly because it should be for more com-putative code.

Yeah, um... there's no way - absolutely no way - that a "simple transformation of a domain model into a view model" should take four and a half seconds. There is something seriously wrong there. It should take maybe half a millisecond or so. So, your PlotAutocompleteModel constructor is doing something like 10,000 times the amount of work that is normal.

Is this the proper option for this code? I clearly see a performance gain due to the raw amount of records I'm iterating then transforming.

Probably not, because you're hosting on ASP.NET. If you use parallelism on ASP.NET, you will see individual requests complete faster, but it will negatively impact the scalability of your web server as a whole. For this reason, I never recommend parallelism in ASP.NET handlers. (There are specific situations where it would be acceptable - such as a non-public server where you know you have a hard upper limit on the number of simultaneous users - but as a general rule, it's not a good idea).

Since your PlotAutocompleteModel constructor is taking several orders of magnitude longer than expected, I suspect that it's doing blocking I/O as part of its work. The best solution here is to change the blocking I/O to asynchronous I/O, and then use concurrent asynchrony, something like this:

class PlotAutocompleteModel
{
  public static async Task<PlotAutocompleteModel> CreateAsync(PlotDomain plot)
  {
    ... // do asynchronous I/O to create a PlotAutocompleteModel.
  }
}

[HttpPost]
public async Task<IEnumerable<PlotAutocompleteModel>> Get()
{
  IEnumerable<PlotDomain> plots = await plotService.RetrieveAllPlots();
  var tasks = plots.Select(plot => PlotAutocompleteModel.CreateAsync(plot));
  return await Task.WhenAll(tasks);
}