Re: My -short- lock-free sequencer class, I want to see your comments
Recently I needed a sequencer for my IOCP based socket server and
developed one. I try to implement it in lock-free manner. Your comments
will bee appreciated.
The class is very small and all it do is to maintain two numbers in thread
safe manner. Before every WSARecv call I get next available sequence
number from sequencer and put that number into my PerIoContext object.
When a recv completion occured then I check the sequence number to decide
if the completion occured in order. So one of the two numbers
(m_lCurrentSequence) represents call sequence and the other
(m_lRunningOrder) represents completion sequence. Here is the class :
Sequencer (LONG lMaxSequence) : m_lCurrentSequence (0),
(0), m_lMaxSequence (lMaxSequence)
Single-argument constructor that is not declared 'explicit' allows implicit
conversions, which are usually bad. Further, your class is still
default-constructible, copyable and assignable.
LONG getNextSequence ()
LONG lCurrentSequence, lNextSequence;
m_lCurrentSequence); lNextSequence = (lCurrentSequence ==
m_lMaxSequence ? 0 :
lCurrentSequence + 1);
if (lCurrentSequence == InterlockedCompareExchange
(&m_lCurrentSequence, lNextSequence, lCurrentSequence))
Inconsistent formatting, making it harder to understand what's going on.
Further, the whole code is undocumented. Documenting the design of code
often leads to the discovery of logic errors and allows maintenance by
bool isInOrder (const LONG lSequence)
return (lSequence == m_lRunningOrder);
Should be const.
bool updateRunningOrder (const LONG lSequence)
if (isInOrder (lSequence))
// Safe region...
LONG lNewRunningOrder = (lSequence + 1) > m_lMaxSequence ? 0 :
(lSequence + 1);
InterlockedExchange (&m_lRunningOrder, lNewRunningOrder);
InterlockedExchange() takes a pointer to a volatile LONG. While this is
automatically added in a function call, it is still necessary in
isInOrder(). Why? Simple reason: the compiler implements read-accesses to a
volatile in a way similar to the Interlocked*() functions. In fact, a
simple InterlockedRead() function is missing from that API.
C++ FAQ: http://parashift.com/c++-faq-lite
Sator Laser GmbH
Gesch??ftsf??hrer: Thorsten F??cking, Amtsgericht Hamburg HR B62 932