Explore our sites

Stack Exchange
tour help careers 2.0
Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 1 down vote favorite

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.

share|improve this question
 
Is .NET 4.5 an option? –  Reed Copsey Jul 12 '13 at 20:20
 
It is an option in the near future, not sure how near though! :) –  CitadelCSAlum Jul 12 '13 at 20:25
 
You can always use the Bcl async targeting pack for .NET 4 - async/await is really the right approach here, not the TPL via new threads/tasks. –  Reed Copsey Jul 12 '13 at 20:28
add comment

1 Answer

up vote 2 down vote accepted

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.

share|improve this answer
 
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? –  CitadelCSAlum Jul 12 '13 at 22:02
 
I read a few places I might be able to use Task.Delay(), but not sure. –  CitadelCSAlum 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? –  CitadelCSAlum Jul 12 '13 at 22:45
1  
@CitadelCSAlum See stackoverflow.com/questions/9846615/… for how to use Task.WhenAll with a timeout –  Reed Copsey Jul 12 '13 at 23:35
show 2 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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