5
\$\begingroup\$

I have come across a situation where I feel running some code in parallel will greatly improve performance, but I am concerned about the implementation and am looking for some confirmation. Take the following controller resources

[HttpGet]
[Route("{id}")]
public IHttpActionResult Get(int id)
{
     return Ok(_context.Get(id));
}

[HttpGet]
[Route("async/{id}")]
public async Task<IHttpActionResult> Get(int id)
{
     return Ok(await _context.GetAsync(id));
}

The contents of Get(...) function are fairly straight forward. A WCF call is made using the passed id to retrieve document a object. That document object contains a list of associated material numbers for which I need to make a separate WCF call for each to gather additional data

public DocumentModel Get(int id)
{
     //Call to WCF service
     DocumentModel model = _documents.Get(id);

     //For each material, make a call to WCF service
     //Potential for bottle neck due to possibility of large number of associated materials
     foreach (Material material in model.Materials)
         model.MaterialModels.Add(_materials.Get(material.Number));

     return model;
}

It is worth noting that I am using a library developed and maintained by a colleague to call WCF services that are hosted on the same VM, so lets just say that I don't have access to modify this library for sake of the question. Retrieving a material looks like the following

public MaterialModel Get(string number)
{
     using(var client = new MaterialClient(out WcfResult result))
    {
         client.Get(number, out Material material, out Result result);
         return new MaterialModel(material);
    }
}

Everything works as expected, but as previously noted its possible for a significant number of materials to be returned (~20-60). Typically, I wouldn't encounter a problem like this since I would be utilizing HATEOAS and supplying the resource for each material in the DocumentModel object, however; this is an API for internal business use and the business requires all the data be in the response so I am left with little choice. So I made a separate implementation that will retrieve that material data in parallel

public async Task<DocumentModel> GetAsync(int id)
{
     //Call to WCF service
     DocumentModel model = _documents.Get(id);
     IEnumerable<Task<MaterialModel>> tasks = model.Materials.Select(material => 
          _materials.GetAsync(material.Number));

     MaterialModel[] result = await Task.WhenAll(tasks);
     model.MaterialModels.AddRange(result);
     return model;
}

Here is where my concern is. There are no Async implementations in the library, so my initial thought was to just do the following

public Task<MaterialModel> GetAsync(string number)
{
     //This concerns me
     return Task.Run(() =>
     {
         using(var client = new MaterialClient(out WcfResult result))
         {
              client.Get(number, out Material material, out Result result);
              return new MaterialModel(material);
         }
     });
}

Both implementations work, but when I run them both side by side the one making calls in parallel is significantly faster and diagnostics shows the flow is what I expected as well

Sequence


Parallel

Is this an acceptable implementation or are there some pitfalls to this I may not be aware about?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ The main problem with doing this in ASP.Net specifically is that you can kill scalability. That might not be a problem for your specific use case (i.e. if it's an internal API called infrequently). \$\endgroup\$ – RobH Oct 2 '20 at 10:30
3
\$\begingroup\$

I think instead of creating an unknown amount of task using PLinq would make it simpler and have control over the number of task inflight/created

something like

var result = model.Materials
                .AsParallel()
                .WithDegreeOfParallelism(8) // whatever you want or leave it out 
                .Select(material => _materials.Get(material.Number));

model.MaterialModels.AddRange(result);

This way it's not trying to make synchronous code async and can limit the amount of task inflight vs unlimited amount.

\$\endgroup\$
1
  • \$\begingroup\$ If you don't care about ordering then you might improve efficiency by calling ForAll: .Select(material => _materials.Get(material.Number)).ForAll(model.MaterialModels.Add) \$\endgroup\$ – Peter Csala Oct 7 '20 at 9:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.