views:

601

answers:

4

Hi

I need to design a thread-safe logger. My logger must have a Log() method that simply queues a text to be logged. Also a logger must be lock-free - so that other thread can log messages without locking the logger. I need to design a worker thread that must wait for some synchronization event and then log all messages from the queue using standard .NET logging (that is not thread-safe). So what i am interested in is synchronization of worker thread - and Log function. Below is a sketch of the class that i designed. I think I must use Monitor.Wait/Pulse here or any other means to suspend and resume worker thread. I don;t want to spend CPU cycles when there is no job for logger.

Let me put it another way - I want to design a logger that will not block a caller threads that use it. I have a high performance system - and that is a requirement.

class MyLogger
{
  // This is a lockfree queue - threads can directly enqueue and dequeue
  private LockFreeQueue<String> _logQueue;
  // worker thread
  Thread _workerThread;
  bool _IsRunning = true;

 // this function is used by other threads to queue log messages
  public void Log(String text)
{
  _logQueue.Enqueue(text);
}

// this is worker thread function
private void ThreadRoutine()
{
 while(IsRunning)
 {
   // do something here
 }
}    
}
+3  A: 

"lock-free"does not mean that threads won't block each other. It means that they block each other through very efficient but also very tricky mechanisms. Only needed for very high performance scenarios and even the experts get it wrong (a lot).

Best advice: forget "lock-free"and just use a "thread-safe" queue.

I would recommend the "Blocking Queue" from this page.

And it's a matter of choice to include the ThreadRoutine (the Consumer) in the class itself.

To the second part of your question, it depends on what "some synchronization event" exactly is. If you are going to use a Method call, then let that start a one-shot thread. If you want to wait on a Semaphore than don't use Monitor and Pulse. They are not reliable here. Use an AutoResetEvent/ManualResetEvent.
How to surface that depends on how you want to use it.

Your basic ingredients should look like this:

class Logger
{
    private AutoResetEvent _waitEvent = new AutoResetEvent(false);
    private object _locker = new object();
    private bool _isRunning = true;    

    public void Log(string msg)
    {
       lock(_locker) { _queue.Enqueue(msg); }
    }

    public void FlushQueue()
    {
        _waitEvent.Set();
    }

    private void WorkerProc(object state)
    {
        while (_isRunning)
        {
            _waitEvent.WaitOne();
            // process queue, 
            // ***
            while(true)
            {
                string s = null;
                lock(_locker)
                {
                   if (_queue.IsEmpty) 
                      break;
                   s = _queue.Dequeu();
                }
                if (s != null)
                  // process s
            }
        } 
    }
}

Part of the discussion seems to be what to do when processing the Queue (marked ***). You can lock the Queue and process all items, during which adding of new entries will be blocked (longer), or lock and retrieve entries one by one and only lock (very) shortly each time. I've adde that last scenario.

A summary: You don't want a Lock-Free solution but a Block-Free one. Block-Free doesn't exist, you will have to settle for something that blocks as little as possible. The last iteration of mys sample (incomplete) show how to only lock around the Enqueue and Dequeue calls. I think that will be fast enough.

Henk Holterman
Well my requirement here is to implement non-blocking logging since we have some sort of high performance Windows service.
Captain Comic
But how are you going to implement blocking for the logger thread? The overhead of blocking is much greater than that of using Monitor.
wj32
Captain, I think we only disagree on the naming of things.
Henk Holterman
Quite perplexing. You want your worker thread to block yet you want a "non-blocking" queue. And @Henk Holterman: lock-free does actually imply overall progress. Since you can't hang in the middle of an instruction (while you can while a mutex is acquired), lock-free code never deadlocks.
wj32
Err... I don't understand quite well Multihreading - can you be so kind to explain a bit - may be in separate answer - using "block" in some public (used by other threads) function - doesn't that mean that one thread can block execution of other thread? If yes - i thought that if I use a lockfreequeue (based on CompareExchange) function (that I assume is non-blocking because CompareExchange is some sort of tricky Windows function) then I just need to send some sort of signal to worker thread so it can start dequeueing? Sorry again for my litte knowlege of the matter.
Captain Comic
It is very rare to have a data structure that is safe to update from multiple threads with no synchronization or locking constructs. Lock-free simply means you're not using the OS kernel lock objects, instead using things like spinwaits to wait for things to become available, typically mostly used in kernel code where such locks aren't available at all. I would start with something simple, like a normal blocking queue, and see how that scales.
Lasse V. Karlsen
Blocking isn't bad or anything. It just means your thread won't execute until you explicitly wake it up. In this case you want to notify the logger thread each time something is logged. Why use a lock-free queue? There are more things you need to synchronize than just enqueuing/dequeuing. Using a lock-free queue won't give you a noticable performance improvement at all.
wj32
@Lasse: spinlocks are locks. They're not lock-free. And who said you can't use locks in kernel-mode? I think you meant compare-and-swap (CAS), a.k.a. CompareExchange.
wj32
I use the design in this answer often and it works well enough for embedded systems. Use a queue that you lock when enqueueing/dequeueing the items. The lock is very short and contention is minimal for the same reason. The worker thread can lock once and dequeue multiple items locally for processing for even less locking.
Ioan
+3  A: 

Has your profiler shown you that you are experiencing a large overhead by using a simple lock statement? Lock-free programming is very hard to get right, and if you really need it I would suggest taking something existing from a reliable source.

erikkallen
+1  A: 

It's not hard to make this lock-free if you have atomic operations. Take a singly linked list; you just need the head pointer.

Log function:
1. Locally prepare the log item (node with logging string).
2. Set the local node's next pointer to head.
3. ATOMIC: Compare head with local node's next, if equal, replace head with address of local node.
4. If the operation failed, repeat from step 2, otherwise, the item is in the "queue".

Worker:
1. Copy head locally.
2. ATOMIC: Compare head with local one, if equal, replace head with NULL.
3. If the operation failed, repeat from step 1.
4. If it succeeded, process the items; which are now local and out of the "queue".

Ioan
A: 

Take a look at this; seems it's what you are looking for.

Kaveh Shahbazian