1

I have a WCF Service that is responseible for taking in an offer and 'reaching' out and dynamically provide this offer to X amount of potential buyers (typically 15-20) which are essentially external APIs.

Each of the buyers currently has 35 seconds to return a response, or they lose the ability to buy the offer,

In order to accomplish this, I have the following code which has been in production for 8 months and has worked and scaled rather well.

As we have been spending a lot of time on improving recently so that we can scale further, I have been interested in whether I have a better option for how I accomplishing this task. I am hesitant in making changes because it is workign well right now,however I may be able to squeeze additional performance out of it right now while I am able to focus on it.

The following code is responsible for creating the tasks which make the outbound requests to the buyers.

IBuyer[] buyer = BuyerService.GetBuyers();  /*Obtain potential buyers for the offer*/
var tokenSource = new CancellationTokenSource();
var token = tokenSource.Token;
Tasks = new Task<IResponse>[Buyers.Count];

for(int i = 0; i < Buyers.Count;i++)
{

    IBuyer buyer = Buyers[i];
    Func<IResponse> makeOffer = () => buyer.MakeOffer()
    Tasks[i] = Task.Factory.StartNew<IResponse>((o) =>
        {

            try
            {
                var result = MakeOffer();

                if (!token.IsCancellationRequested)
                {
                    return result;
                }
            } 
            catch (Exception exception
            {
                /*Do Work For Handling Exception In Here*/
            }
            return null;
        }, token,TaskCreationOptions.LongRunning);
};

Task.WaitAll(Tasks, timeout, token);    /*Give buyers fair amount of time to respond to offer*/
tokenSource.Cancel();

List<IResponse> results = new List<IResponse>();    /*List of Responses From Buyers*/

for (int i = 0; i < Tasks.Length; i++)
{
    if (Tasks[i].IsCompleted)   /*Needed so it doesnt block on Result*/
    {
        if (Tasks[i].Result != null)
        {
            results.Add(Tasks[i].Result);
        }
        Tasks[i].Dispose();
    }
}

/*Continue Processing Buyers That Responded*/

On average, this service is called anywhere from 400K -900K per day, and sometimes up to 30-40 times per second.

We have made a lot of optimizations in an attempt to tune performance, but I want to make sure that this piece of code does not have any immediate glaring issues.

I read alot about the power of TaskScheduler and messing with the SynchronizationContext and working async, and I am not sure how I can make that fit and if it is worth an improvement or not.

TheJediCowboy
  • 8,924
  • 28
  • 136
  • 208

1 Answers1

2

Right now, you're using thread pool threads (each Task.Factory.StartNew call uses a TP thread or a full .NET thread, as in your case, due to the LongRunning hint) for work that is effectively IO bound. If you hadn't specified TaskCreationOptions.LongRunning, you'd have seen a problem very early on, and you'd be experiencing thread pool starvation. As is, you're likely using a very large number of threads, and creating and destroying them very quickly, which is a waste of resources.

If you were to make this fully asynchronous, and use the new async/await support, you could perform the same "work" asynchronously, without using threads. This would scale significantly better, as the amount of threads used for a given number of requests would be significantly reduced.

As a general rule of thumb, Task.Factory.StartNew (or Task.Run in .NET 4.5, as well as the Parallel class) should only be used for CPU bound work, and async/await should be used for IO bound work, especially for server side operations.

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • I have been trying to determine whether it is worth making this change, however I am still unsure as to how I would accomplish what I am currently using Task.WaitAll(...) with the timeout specifiec if I was using the async/await? – TheJediCowboy Jul 12 '13 at 22:02
  • I read a few places I might be able to use Task.Delay(), but not sure. – TheJediCowboy Jul 12 '13 at 22:03
  • @CitadelCSAlum Basically, with async/await, this could be done using no threading. RIght now, you're using a thread per request, which is a HUGE number of threads. It'd be significantly more efficient. – Reed Copsey Jul 12 '13 at 22:30
  • I am still confused how I would be able to accomplish the above code with a 35 second timeout, and have the returned async's all continue to execute in one line of execution after the 35 seconds? – TheJediCowboy Jul 12 '13 at 22:45
  • 1
    @CitadelCSAlum See http://stackoverflow.com/questions/9846615/async-task-whenall-with-timeout for how to use `Task.WhenAll` with a timeout – Reed Copsey Jul 12 '13 at 23:35
  • So Task.WhenAll is alright to use when using async/await , but Task.WaitAll is not? Why is that if that is true? – TheJediCowboy Jul 13 '13 at 17:37
  • @CitadelCSAlum WaitAll blocks - WhenAll returns a Task that you can use with await to compose your results. async/await is all about *composing* asynchronous operations and chaining them together, not about creating async operations. You can use WaitAll, but WhenAll keeps things asynchronous. – Reed Copsey Jul 15 '13 at 16:08