Re: [PATCH 1/5] MSVC: Windows-native implementation for subset of Pthreads API

[Dmitry Potapov <dpotapov@gmail.com>]

On Fri, Jan 08, 2010 at 09:40:36PM +0100, Johannes Sixt wrote:
> On Freitag, 8. Januar 2010, Erik Faye-Lund wrote:
> > On Fri, Jan 8, 2010 at 4:32 AM, Dmitry Potapov <dpotapov@gmail.com> wrote:
> > > AFAIK, Win32 API assumes that reading LONG is always atomic, so
> > > the critical section is not really necesary here, but you need
> > > to declare 'waiters' as 'volatile':
> >
> > "Simple reads and writes to properly-aligned 32-bit variables are
> > atomic operations."
> > http://msdn.microsoft.com/en-us/library/ms684122(VS.85).aspx
> >
> > In other words: Yes, you are right.
> 
> Quite frankly, I do not want to stretch this statement to apply to the MinGW 
> compiler.

I am sure that MinGW compiler (i.e. gcc) will work fine as long as the
variable is marked as 'volatile'.

> The code in question is not performance critical anyway. I'd prefer 
> to leave it as is - it's undergone 2 months of testing already.

Well, it is a strong argument for not change anything, in general, but
the change is trivial -- instead of increment and decrementing some
varaible under a lock, it is increment/decrement using atomic operation.
There is no change to the logic, or anything that can have unexpected
side effects.

> Besides, 
> IMHO, it is much more readable the way it is written.

I _completely_ disagree with that. Using atomic operations is not only
more efficient, but it is more readable. Having an additional mutex just
to increment and decrement does not increase readability in any way but
only raises additional questions -- why do you need it here? Is it used
for something else besides incrementing and decrementing the variable,
which can be done atomically?


Dmitry