-1

I want to increase the performance of a procedure which invokes a web service multiple times sequentially and store the result in a list.

Due that a single call to the WS last 1second and I need to do something like 300 calls to the web service if I do the job sequentially it takes 300 seconds to accomplish the task, that's why I changed the procedure implementation to multithreading using the following piece of code:

List<WCFResult> resultList= new List<WCFResult>()
using (var ws = new WCFService(binding, endpoint))
{
    foreach (var singleElement in listOfelements)
    {
         Action action = () =>
                   {                            
                        var singleResult = ws.Call(singleElement);                                 
                        resultList.Add(singleResult);        
                   };

         tasks.Add(Task.Factory.StartNew(action, TaskCreationOptions.LongRunning));
    }
}

Task.WaitAll(tasks.ToArray());

//Do other stuff with the resultList...

Using this code I achieve to save 0.1 seconds per single element which is less than I thought, do you know any further optimization I can do? Or can you share an alternative?

Paolo
  • 631
  • 2
  • 10
  • 23
  • Side note, unless you're using a concurrent collection, you may have potential issues adding to your list in parallel. Parallel.ForEach will let you aggregate the results of each partition, and would be better than launching three hundred concurrent tasks anyways – pinkfloydx33 Jul 12 '19 at 09:02
  • 1
    @pinkfloydx33 - It's not a potential issue - it's a certain one that just depends on luck if it manages to succeed. – Enigmativity Jul 13 '19 at 03:59
  • Well, in the practice that is not a problem. I assume the concurrency while accessing the list is managed automatically otherwise I had your same opinion. – Paolo Jul 13 '19 at 18:06
  • I add to my prevous comment that removing the command to add the result on the list does not change the times. Furthermore the Parallel.ForEach implementation was the first attempt I did and the result was poorest. – Paolo Jul 14 '19 at 07:33
  • 1
    Do you control the web service? If so, change it to take a collection of inputs and do just one round-trip. If you do not control the web service, you might want to verify with whomever provides the service that it can handle whatever load you are subjecting it to. It is possible that hammering the service would result in slower performance (through throttling or resource contention on the service side). – user469104 Jul 15 '19 at 15:42
  • This can be a good point for improvement, unfortunately the web service is not owned by my team so I need to push third party teams... can you suggest an IIS configuration that could improve the server response time? – Paolo Jul 16 '19 at 09:02
  • The option `LongRunning` is intended for exceptional cases where a dedicated thread makes more sense than a thread-pool thread. In your case it is certainly not the case. By starting a dedicated thread for each task you are wasting tons of system resources, for no real benefit. I bet that you are actually hurting the performance, with all the RAM that must be allocated (1MB per thread), and all the context switches between the hundreds of threads. Using thread-pool threads would also have the side-effect of throttling the parallel execution, which seems much desirable in your case. – Theodor Zoulias Jul 16 '19 at 12:58

1 Answers1

-1

Using the following code all the request are handled in half of the time

ParallelOptions ops = new ParallelOptions();
ops.MaxDegreeOfParallelism = 16;

ConcurrentBag<WCFResult> sapResultList = new ConcurrentBag<WCFResult>();

Parallel.ForEach(allItems, ops, item =>
{
        var ws = new WCFClient(binding, endpoint);
        result = ws.Call(item);
        svc.Close();
        resultList.Add(result);
});
//Do other stuff with the resultList...

Mission accomplished. I also modified the result list to be a ConcurrentBag instead of a List

Paolo
  • 631
  • 2
  • 10
  • 23
  • Yes it is, what is your suggestion instead? – Paolo Jul 16 '19 at 13:08
  • [`ConcurrentBag`](https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentbag-1) or [`ConcurrentQueue`](https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentqueue-1), or keep the `List` but protect it using a [`lock`](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement). Or even better express your workload as a LINQ query, so that you can parallelize it simply with `AsParallel`, without having to deal with thread-safe collections and fixing the messed up order of the results. – Theodor Zoulias Jul 16 '19 at 13:32
  • Thanks for the suggestion I also modified the result list type gaining some seconds. It is not clear why using a List did not result in any error 100% of run I did, anyway your suggestion is pretty clear and I did the change – Paolo Jul 16 '19 at 15:26
  • Threading problems are inherently inconsistent. In the case of a `List` you won't get an exception unless two threads are trying concurrently to resize the internal array. It is more common that you'll miss updates when two threads are adding records concurrently, but this doesn't raise an exception and you may not even notice it. – Theodor Zoulias Jul 16 '19 at 15:41
  • This piece of code done in C++ would have cause an exception instantly that is why I believe that .NET does something under the wood to prevent the error. Anyway just for completeness I tried also locking the List and the ConcurrentQueue the running time does not change thou – Paolo Jul 16 '19 at 15:45
  • Appreciated your help, don't understand why down voting... but ... thanks anyway I think that others can take adavantage of this optimization and of the thread in general – Paolo Jul 16 '19 at 15:57
  • The performance of your code is highly dependent to the performance of the external web service, and the most promising way of helping it performing at its best is by throttling your requests. The difference between making 300 requests in batches of 5 and making 300 requests all at once may be the difference between a remote server that works at its peak and a remote server that has been thoroughly depleted from resources and struggles to do anything done. – Theodor Zoulias Jul 16 '19 at 16:00
  • Here are two helpful external links about throttling : [Approaches for throttling asynchronous methods in C#](https://blog.briandrupieski.com/throttling-asynchronous-methods-in-csharp), [Implementing a simple ForEachAsync, part 2](https://devblogs.microsoft.com/pfxteam/implementing-a-simple-foreachasync-part-2/). And one from StackOverflow: [How to limit the amount of concurrent async I/O operations?](https://stackoverflow.com/questions/10806951/how-to-limit-the-amount-of-concurrent-async-i-o-operations). – Theodor Zoulias Jul 16 '19 at 16:06