Re: [boost] Fw: [atomic] review results

Subject: Re: [boost] Fw: [atomic] review results
From: Helge Bahmann (hcb_at_[hidden])
Date: 2011-11-08 04:53:09

Hi Tim,

On Tuesday 08 November 2011 10:17:20 Tim Blechmann wrote:
> hi helge,
>
> > > > this could be avoided by using boost::interprocess::atomic<>, which
> > > > will associate a spinlock with each instance ... or by using
> > > > std::atomic on c++11 compilers.
> >
> > While this problem is definitely an argument in favour of embedding the
> > spinlock into each atomic, I am still concerned as this introduces
> > incompatibility with std::atomic implementations which may eventually
> > bite someone hard (data structure size, expectations -- changing boost to
> > "using std::atomic" might have grave consequences, and it's a transition
> > path I would like to keep open)
>
> not sure:
>
> * compilers are not required to implement atomics so that sizeof(atomic<T>)
> == sizeof (T). so the size of a data structure may change when switching
> compilers.

Yes that's right, however I trust that ABI conventions will be worked out per 
platform eventually -- people will consider inability to mix 
icc/gcc/clang/whatever at least on a shared-library level as unacceptable, 
and currently it works rather well. Also note that the intent of C++11 is to 
make these low-level structures compatible with C1X so there really is not 
that much room.

I think that there is value in trying to maintain sizeof(boost::atomic<T>) == 
sizeof(std::atomic<T>) whatever this is going to mean per platform, and part 
of the thing I have in my mind is indeed interprocess-safety. I also don't 
think that Boost.Atomic it is in that state currently, but I am a bit 
reluctant to make a decision that might shut down this path forever.

> * compilers do change the size of a data structure depending on compiler 
> flags. some compilers have a notion of `packed' structs, that ensure the 
> memory layout. however gcc and icc seem to require that all struct members
> are  PODs, while clang++ seems to accept non-POD members ...

well but __attribute__((packed)) is something you have to attach to each 
individual data structure so it is not a global change -- and a global flag 
that affects data structure layout is problematic in any case because you 
will have a hard time linking to and interfacing with any library.

> * i could imagine that c++11 compilers may be smart enough to pad adjacent
> std::atomic<> to ensure that they are placed in separate cache lines.

uhh, I hope they *don't* do that as it has grave implications on the ABI 
(currently the compiler would not be allowed to "fill" the hole with other 
data members), and cacheline sizes vary by processor generation

> > Another option that I have considered would be "piggy-backing" the
> > spinlock
> > pool into Boost.Thread -- the idea is that an application is either
> > single-threaded, or if it is multi-threaded it is expected to link with
> > Boost.Thread (even if nothing from Boost.Thread is used indeed, yes makes
> > me feel uneasy as well).
>
> this would imply that boost.thread cannot be used as static library any
> more.

That would indeed be bad, but I don't understand why this would be in 
conflict, could you explain a bit further?

Best regards
Helge