hi all, unfortunately only few reviews of boost.atomic arrived in time, however i count 3 YES and no NO, so i am happy to say: Helge Bahmann's boost.atomic library is ACCEPTED. since the library is an implementation of a c++11 feature for c++03, design, API and semantics are mostly fixed. many comments are going into details of the implementation, so i won't sum them all up ... iac, helge already said he wants to address the raised issues, the main points are summarized below. full implementation of the c++11 interface: the library currently only implements a subset of the c++11 library. it lacks the functional interface for integral atomic types and the ATOMIC_VAR_INIT / ATOMIC_FLAG_INIT macro. quickly consulting the latest draft that i currently have at hand the following two functions are missing as well: template <class T> T kill_dependency(T y); void atomic_signal_fence(memory_order); i encourage helge to provide a full implementation of the interface. documentation: the documentation is currently lacking a good API reference. i would therefore encourage helge to provide a doxygen-generated reference section. there were some concerns that it assumes that readers will have to be familiar with concurrency. maybe it is a good idea to point to some literature/tutorials about concurrent programming and/or to c++11 atomics. the section of real-world examples could probably include a link to shavit/herlihy's book `the art of multiprocessor programming'. it would also be helpful to have a detailed specification, if a platform/compiler combination supports a specific type natively (with run-time dispatching or specific compiler flags) ... maybe in form of a table. shared memory support/multi-module support: in multi-module applications and when using shared memory, the blocking emulation of atomics can cause troubles. for multi-module applications this can be resolved by placing the spinlock pool inside a shared library. shared- memory support could be realized by associating a (spin)lock with each blocking atomic<> instead of using a pool of spinlocks. boost should definitely provide atomics which support shared memory, either by default or by an additional implementation which is placed in the boost::interprocess namespace. i would like to thank helge for implementing boost.atomic and submitting it to boost and everyone who participated in the review. cheers, tim review manager
Helge Bahmann's boost.atomic library is ACCEPTED.
Congratulations on the acceptance, and a few belated (sorry) comments: - Boost.Atomic should not use the smart_ptr spinlock pool; it should contain its own under boost/atomic. It could be a straight copy of the shared_ptr one. Boost.Atomic is lower level than shared_ptr and should not have an upward dependency, because a future shared_ptr may (and will) use Boost.Atomic in its spinlock implementation. - For the same reason, atomic_flag should be a proper fundamental component and not a wrapper over atomic_bool. atomic_flag is the building block for spinlocks; it should always be present and should always be lock-free. (And it should have ATOMIC_FLAG_INIT, because the spinlock pool needs it.) - The interlocked implementation seems suboptimal - it doesn't use InterlockedExchange and InterlockedExchangeAdd. (The spelling of atomic_signal_fence under MSVC version something and above is _ReadWriteBarrier - see boost/smart_ptr/detail/spinlock_w32.hpp). I apologize if these have already been discussed.
On Monday 07 November 2011 18:06:33 Peter Dimov wrote:
Helge Bahmann's boost.atomic library is ACCEPTED.
Congratulations on the acceptance, and a few belated (sorry) comments:
- Boost.Atomic should not use the smart_ptr spinlock pool; it should contain its own under boost/atomic. It could be a straight copy of the shared_ptr one. Boost.Atomic is lower level than shared_ptr and should not have an upward dependency, because a future shared_ptr may (and will) use Boost.Atomic in its spinlock implementation.
yes makes sense -- there was a concern raised by Andrey Semashev that the spinlock pool as implemented and used by shared_ptr presently may fail on Windows due to the pool being non-unique (not had a chance to test this yet), and I have found a way to produce a similar failure using dlopen, atomics private to shared libraries and RTLD_LOCAL -- currently I am therefore leaning on creating a shared library just for the spinlock pool, but since you wrote the initial implementation maybe you could comment as well?
- For the same reason, atomic_flag should be a proper fundamental component and not a wrapper over atomic_bool. atomic_flag is the building block for spinlocks; it should always be present and should always be lock-free. (And it should have ATOMIC_FLAG_INIT, because the spinlock pool needs it.)
okay agreed
- The interlocked implementation seems suboptimal - it doesn't use InterlockedExchange and InterlockedExchangeAdd. (The spelling of atomic_signal_fence under MSVC version something and above is _ReadWriteBarrier - see boost/smart_ptr/detail/spinlock_w32.hpp).
yes that's indeed an oversight and it is needed Best regards Helge
Hi all and especially Tim, On Monday 07 November 2011 16:41:44 Tim Blechmann wrote:
hi all,
unfortunately only few reviews of boost.atomic arrived in time, however i count 3 YES and no NO, so i am happy to say:
Helge Bahmann's boost.atomic library is ACCEPTED.
thanks I will update the implementation to address the issues raised and send an announcement [...]
shared memory support/multi-module support: in multi-module applications and when using shared memory, the blocking emulation of atomics can cause troubles. for multi-module applications this can be resolved by placing the spinlock pool inside a shared library. shared- memory support could be realized by associating a (spin)lock with each blocking atomic<> instead of using a pool of spinlocks. boost should definitely provide atomics which support shared memory, either by default or by an additional implementation which is placed in the boost::interprocess namespace.
I would suggest putting it into boost::interprocess, I will prepare an implementation and post it as well Thanks everyone again Helge
El 07/11/2011 21:11, Helge Bahmann escribió:
Hi all and especially Tim,
[...]
shared memory support/multi-module support: in multi-module applications and when using shared memory, the blocking emulation of atomics can cause troubles. for multi-module applications this can be resolved by placing the spinlock pool inside a shared library. shared- memory support could be realized by associating a (spin)lock with each blocking atomic<> instead of using a pool of spinlocks. boost should definitely provide atomics which support shared memory, either by default or by an additional implementation which is placed in the boost::interprocess namespace.
I would suggest putting it into boost::interprocess, I will prepare an implementation and post it as well
Thanks, until now, I was using my own (I guess wrongly written) atomics under boost/interprocess/detail/atomic.hpp, glad to see I can rely on Boost.Atomic now. Ion
participants (4)
-
Helge Bahmann -
Ion Gaztañaga -
Peter Dimov -
Tim Blechmann