views:

80

answers:

2

I have an ASP.NET app, very basic, but right now too much code to post if we're lucky and I don't have to.

We have a class called ReportGenerator. On a button click, method GenerateReports is called. It makes an async call to InternalGenerateReports using ThreadPool.QueueUserWorkItem and returns, ending the ASP.NET response. It doesn't provide any completion callback or anything.

InternalGenerateReports creates and maintains five threads in the threadpool, one report per thread, also using QueueUserWorkItem, by 'creating' five threads, also with and waiting until calls on all of them complete, in a loop. Each thread uses an ASP.NET ReportViewer control to render a report to HTML. That is, for 200 reports, InternalGenerateReports should create 5 threads 40 times. As threads complete, report data is queued, and when all five have completed, report data is flushed to disk.

My biggest problems are that after running for just one report, the aspnet process is 'hung', and also that at around 200 reports, the app just hangs.

I just simplified this code to run in a single thread, and this works fine. Before we get into details like my code, is there anything obvious in the above scendario that might be wrong?

Here is an abreviated example of the code:

public class SscceReports
{
    Dictionary<Guid, AsyncReportCreator> runningWorkers = new Dictionary<Guid, AsyncReportCreator>();
    public void GenerateReports(Queue<int> reportKeys)
    {
        int goodPoolSize = System.Environment.ProcessorCount;
        System.Threading.ThreadPool.SetMaxThreads(goodPoolSize + 10, goodPoolSize * 10);
        System.Threading.ThreadPool.QueueUserWorkItem(new System.Threading.WaitCallback(InternalGenerateReports), reportKeys);
    }

    void InternalGenerateReports(object state)
    {
        Queue<int> reportKeys = state as Queue<int>;
        while (reportKeys.Count > 0)
        {
            for (int i = 0; i < 5 && reportKeys.Count > 0; i++)
            {
                Guid workerId = Guid.NewGuid();
                int rk = (int) reportKeys.Dequeue();
                AsyncReportCreator asrc = new AsyncReportCreator(rk);
                runningWorkers.Add(workerId, asrc);
                asrc.WorkComplete += CompleteCallBack;
                System.Threading.ThreadPool.QueueUserWorkItem(asrc.StartWork);
            }
            while (runningWorkers.Count > 0)
                System.Threading.Thread.Sleep(500);
        }
        while (runningWorkers.Count > 0)
            System.Threading.Thread.Sleep(5000);
    }

    void CompleteCallBack(object state)
    {
        // Write queued report content to disk.
        runningWorkers.Remove((Guid) state);
    }
}

public class AsyncReportCreator
{
    public event System.Threading.WaitCallback WorkComplete;
    private int key;
    public AsyncReportCreator(int reportKey)
    {
        key = reportKey;
    }

    public void StartWork(object state)
    {
        // Create report;
        WorkComplete(state);
    }
}
A: 

The .NET ThreadPool by default has 25 threads per processor, so if you're generating 200 reports when you call InternalGenerateReports it will queue 1,000 work items in the ThreadPool (200 reports * 5 work items for each InternalGenerateReports). Only 25 will be active at a time, but you can see that this model is probably not suitable for you.

The thread pool has a default limit of 25 threads per available processor, which could be changed using CorSetMaxThreads as defined in the mscoree.h file. Each thread uses the default stack size and runs at the default priority. Each process can have only one operating system thread pool.

Think about using a producer/consumer pattern and instead of queuing 25 work items (i.e 25 threads), just create a few consumer threads (matching the number of processors/cores you have) which process requests for report generation from a central blocking queue which is populated by the producer(s). That should make things a bit more reasonable...

There are some articles that say you shouldn't use a ThreadPool with ASP.NET:

You can use the ThreadPool in exactly the same way in ASP.NET and it works just as you would expect. The problem is not in the ThreadPool itself but in what else ASP.NET uses it for at the same time. ASP.NET is multi-threaded by design and it uses the ThreadPool to serve pages and content mapped to the ASP.NET ISAPI filter.

If you also use the ThreadPool, then ASP.NET has fewer threads to utilize and requests are put on hold until the pool returns a free thread. This might not be a problem for a low traffic site, but more popular sites can get into trouble. Low traffic sites can get into trouble if they use the ThreadPool a lot.

Update

I think I found your issue... you're queuing asrc.StartWork but you're not passing in a state so when CompleteCallBack is called it doesn't have anything to remove from the runningWorkers. Here is an improved version for you:

public class CountDownLatch {
    private volatile int m_remain;
    private EventWaitHandle m_event;

    public CountDownLatch(int count) {
        m_remain = count;
        m_event = new ManualResetEvent(false);
    }

    public void Signal() {
        // The last thread to signal also sets the event.
        if (Interlocked.Decrement(ref m_remain) == 0)
            m_event.Set();
    }

    public void Wait() {
        m_event.WaitOne();
    }
}

public class SscceReports
{
    public void GenerateReports(Queue<int> reportKeys)
    {
        int goodPoolSize = System.Environment.ProcessorCount;
        System.Threading.ThreadPool.SetMaxThreads(goodPoolSize + 10, goodPoolSize * 10);
        System.Threading.ThreadPool.QueueUserWorkItem(o=>
        {
            InternalGenerateReports(reportKeys);
        });
    }

    void InternalGenerateReports(Queue<int> reportKeys)
    {
        // assuming that the reportKeys.Count contains only 5 keys,
        // we create a countdown latch to hande the same number of keys
        CountDownLatch latch = new CountDownLatch(reportKeys.Count);
        foreach( int rk in reportKeys)
        {
            AsyncReportCreator asrc = new AsyncReportCreator(rk);
            asrc.WorkComplete += CompleteCallBack;
            System.Threading.ThreadPool.QueueUserWorkItem(o=>
            {
                asrc.StartWork(latch);
            });
        }

        // Wait for all the tasks to complete instead of sleeping
        latch.Wait(); // <- blocks untill all (5) tasks call latch.Signal()
    }

    void CompleteCallBack(CountDownLatch latch)
    {
        // Write queued report content to disk.
        latch.Signal(); 
    }
}

public class AsyncReportCreator
{
    public delegate void WorkComplete(CountDownLatch latch);
    public AsyncReportCreator(int reportKey)
    {
        key = reportKey;
    }

    public void StartWork(CountDownLatch latch)
    {
        // Create report;
        WorkComplete(latch);
    }
}
Lirik
Thanks, I love your suggestion of the consumer model, and will be using it in my redesign, but I think you misunderstood my scenario. For 200 reports, InternalGenerateReports will queue 5 work items 40 times. I have updated the question to make this more clear. As for the ThreadPool and ASP.NET, the ASP.NET application id dedicated to this task, so I doubt sharing the pool with requests will present noticeable problem.
ProfK
@ProfK, what do you mean when you say that "InternalGenerateReports creates and maintains five threads in the threadpool"? Specifically, what do you mean by "maintains"? How many threads are actually starting/queuing at the same time (at peak use)?
Lirik
@Lirik, by maintains I just mean it keeps a list of running and complete tasks (threads), and waits for the Complete callback on each task before transfering it from the running to the complete list. At peak use (all use in fact) there are only ever 5 threads queued/running.
ProfK
@ProfK, if I understand you correctly there are 5 threads in the ThreadPool which are running throughout the entire life of the application. This would not be optimal use of the ThreadPool, it's preferred that only short-running tasks are queued. So while it's not the best practice, it is probably not be the root cause of your problem, but without seeing some code it's hard to say where the problem really is. Can you replicate the issue on a smaller scale? Can you come up with an sscce (www.sscce.org) compliant example?
Lirik
@Lirik, it's not the same 5 threads for the whole app lifetime, but five at most given times. Example coming up soon.
ProfK
@ProfK, I've updated the example with your comments in mind. Let me know if it helps.
Lirik
A: 

If you're using, or can use Framework 4.0, the new Parallel extensions are very good, and very simple for parallel processing. I've combined that with the BackgroundWorker class to create really powerful but simple processing engines:

http://msdn.microsoft.com/en-us/library/dd460720.aspx

Much easier than handling all the threads yourself. This scales out wonderfully.

Amethi