ThreadPool giving amazing results, did I do this right? (no, I didn't)

c#, multithreading, threadpool

Solution

Interlocked.Increment is better than locking, but the polling loop at the end scares me. First, if you're going to loop, then do a Thread.Sleep(0) to release the processor each time. Second, if you're going to poll for a variable, then you need to make sure it's either marked volatile or you use MemoryBarrier, else the compiler may assume no outside thread will change it and therefore optimize away the check, leading to an infinite loop.

Even better would be for each thread to check for it hitting zero and set an event if it does. You can then wait on the event instead of polling. The only trick is that you want to increment once in the main thread before the dispatch loop, then decrement and check for zero before waiting on the event.

edit

If it's broken because it's reusing the trigger, then the closure is wrong. Trying copying the value of world into a variable local to the inside of the loop and using that variable for the lambda expression.

Problem

Not that I'm not appreciative of the powers of multithreading or `ThreadPool`, but I'm scared I broke something since I'm getting a roughly 20x speed increase (2-3s down from over a minute) with a relatively naive usage of `ThreadPool`. So I submit my code here to be torn apart by people far wiser than I. Am I doing something wrong here, or is this just a far better candidate for multithreading than I ever hoped? (Yes, this function is an entire thread: like I said, this used to take over a minute to run) EDIT: To answer my own question, no, this is broken: It seems to be running multiple times, but over the same trigger. Is this because of the way lambda are handled? ``` private static void CompileEverything() { try { // maintain the state of our systray icon object iconLock = new object(); bool iconIsOut = true; // keep a count of how many threads are still running object runCountLock = new object(); int threadRunning = 0; foreach (World w in Worlds) { foreach (Trigger t in w.Triggers) { lock (runCountLock) { threadRunning++; } ThreadPool.QueueUserWorkItem(o => { // [snip]: Do some work involving compiling code already in memory with CSharpCodeProvider // provide some pretty feedback lock (iconLock) { if (iconIsOut) notifyIcon.Icon = Properties.Resources.Icon16in; else notifyIcon.Icon = Properties.Resources.Icon16out; iconIsOut = !iconIsOut; } lock (runCountLock) { threadRunning--; } }); } } // wait for all the threads to finish up while (true) { lock (runCountLock) { if (threadRunning == 0) break; } } // set the notification icon to our default icon. notifyIcon.Icon = Properties.Resources.Icon16; } // we're going down before we finished starting... // oh well, be nice about it. catch (ThreadAbortException) { } } ```

Original source