CODEWITHSUNDEEP
c#performancesynchronizationparallel-processingtask-parallel-library
gjvdkamp
gjvdkamp

Reputation: 10516

Why does parallel this code work sometimes?

I wanted to parallelize a piece of code, but the code actually got slower probably because of overhead of Barrier and BlockCollection. There would be 2 threads, where the first would find pieces of work wich the second one would operate on. Both operations are not much work so the overhead of switching safely would quickly outweigh the two threads.

So I thought I would try to write some code myself to be as lean as possible, without using Barrier etc. It does not behave consistent however. Sometimes it works, sometimes it does not and I can't figure out why.

This code is just the mechanism I use to try to synchronize the two threads. It doesn't do anything useful, just the minimum amount of code you need to reproduce the bug.

So here's the code: 

    // node in linkedlist of work elements
        class WorkItem {
            public int Value;
            public WorkItem Next;
        }

        static void Test() {

            WorkItem fst = null; // first element

            Action create = () => {
                WorkItem cur=null;
                for (int i = 0; i < 1000; i++) {                    

                    WorkItem tmp = new WorkItem { Value = i }; // create new comm class

                    if (fst == null) fst = tmp; // if it's the first add it there
                    else cur.Next = tmp;        // else add to back of list

                    cur = tmp; // this is the current one
                }
                cur.Next = new WorkItem { Value = -1 }; // -1 means stop element
#if VERBOSE
                Console.WriteLine("Create is done");
#endif
            };

            Action consume = () => {
                //Thread.Sleep(1); // this also seems to cure it
#if VERBOSE
                Console.WriteLine("Consume starts"); // especially this one seems to matter
#endif

                WorkItem cur = null;
                int tot = 0;

                while (fst == null) { } // busy wait for first one
                cur = fst;
#if VERBOSE
                Console.WriteLine("Consume found first");
#endif
                while (true) {
                    if (cur.Value == -1) break; // if stop element break;
                    tot += cur.Value;

                    while (cur.Next == null) { } // busy wait for next to be set
                    cur = cur.Next; // move to next
                } 
                Console.WriteLine(tot);
            };

            try { Parallel.Invoke(create, consume); }
            catch (AggregateException e) {
                Console.WriteLine(e.Message);
                foreach (var ie in e.InnerExceptions) Console.WriteLine(ie.Message);
            }

            Console.WriteLine("Consume done..");
            Console.ReadKey();
        }

The idea is to have a Linkedlist of workitems. One thread adds items to the back of that list, and another thread reads them, does something, and polls the Next field to see if it is set. As soon as it is set it will move to the new one and process it. It polls the Next field in a tight busy loop because it should be set very quickly. Going to sleep, context switching etc would kill the benefit of parallizing the code. The time it takes to create a workitem would be quite comparable to executing it, so the cycles wasted should be quite small.

When I run the code in release mode, sometimes it works, sometimes it does nothing. The problem seems to be in the 'Consumer' thread, the 'Create' thread always seems to finish. (You can check by fiddling with the Console.WriteLines). It has always worked in debug mode. In release it about 50% hit and miss. Adding a few Console.Writelines helps the succes ratio, but even then it's not 100%. (the #define VERBOSE stuff).

When I add the Thread.Sleep(1) in the 'Consumer' thread it also seems to fix it. But not being able to reproduce a bug is not the same thing as knowing for sure it's fixed.

Does anyone here have a clue as to what goes wrong here? Is it some optimization that creates a local copy or something that does not get updated? Something like that?

There's no such thing as a partial update right? like a datarace, but then that one thread is half doen writing and the other thread reads the partially written memory? Just checking..

Looking at it I think it should just work.. I guess once every few times the threads arrive in different order and that makes it fail, but I don't get how. And how I could fix this without adding slowing it down?

Thanks in advance for any tips,

Gert-Jan

Upvotes: 1

Views: 326

Answers (2)

DanielOfTaebl
DanielOfTaebl

Reputation: 743

I do my damn best to avoid the utter minefield of closure/stack interaction at all costs. This is PROBABLY a (language-level) race condition, but without reflecting Parallel.Invoke i can't be sure. Basically, sometimes fst is being changed by create() and sometimes not. Ideally, it should NEVER be changed (if c# had good closure behaviour). It could be due to which thread Parallel.Invoke chooses to run create() and consume() on. If create() runs on the main thread, it might change fst before consume() takes a copy of it. Or create() might be running on a separate thread and taking a copy of fst. Basically, as much as i love c#, it is an utter pain in this regard, so just work around it and treat all variables involved in a closure as immutable.

To get it working:

//Replace 
WorkItem fst = null
    //with
WorkItem fst = WorkItem.GetSpecialBlankFirstItem();

//And 
if (fst == null) fst = tmp;
   //with
if (fst.Next == null) fst.Next = tmp;

Upvotes: 2

Jimmy
Jimmy

Reputation: 91482

A thread is allowed by the spec to cache a value indefinitely.

see Can a C# thread really cache a value and ignore changes to that value on other threads? and also http://www.yoda.arachsys.com/csharp/threads/volatility.shtml

Upvotes: 1

Related Questions