Use atomic_read32_relaxed (lighter) when spinning instead of a sequential consistent read.

Author: igaztanaga

include/boost/interprocess/detail/atomic.hpp

@@ -79,6 +79,9 @@ BOOST_INTERPROCESS_FORCEINLINE boost::uint32_t atomic_read32(volatile boost::uin
 //! Atomically read an boost::uint32_t from memory with acquire semantics
 BOOST_INTERPROCESS_FORCEINLINE boost::uint32_t atomic_read32_acquire(volatile boost::uint32_t *mem);
 
+//! Atomically read an boost::uint32_t from memory with relaxed semantics.
+BOOST_INTERPROCESS_FORCEINLINE boost::uint32_t atomic_read32_relaxed(volatile boost::uint32_t *mem);
+
 //! Atomically set an boost::uint32_t in memory
 //! "mem": pointer to the object
 //! "param": val value that the object will assume
@@ -294,6 +297,26 @@ BOOST_INTERPROCESS_FORCEINLINE boost::uint32_t atomic_read32_acquire(volatile bo
    #endif
 }
 
+//! Same as atomic_read32, but with relaxed semantics
+BOOST_INTERPROCESS_FORCEINLINE boost::uint32_t atomic_read32_relaxed(volatile boost::uint32_t *mem)
+{
+   #if defined(__ATOMIC_RELAXED)
+   //Clang (clang-cl included) and GCC: the same plain load instruction a
+   //volatile read would give, but one that participates in the memory model,
+   //so ThreadSanitizer does not flag it as racing other threads' writes
+   return __atomic_load_n(mem, __ATOMIC_RELAXED);
+   #elif defined(_M_ARM64EC) || defined(_M_ARM64) || defined(_M_ARM)
+   //A plain 32 bit load the compiler cannot tear or widen and, unlike a
+   //volatile read under /volatile:ms, one that carries no barrier
+   return (boost::uint32_t)__iso_volatile_load32(reinterpret_cast<const volatile __int32*>(mem));
+   #else
+   //x86/x64: an aligned volatile load is a single MOV, and a relaxed load
+   //needs no barrier, not even a compiler-only one: the value is a hint
+   //that only a later acquire operation may act on
+   return *mem;
+   #endif
+}
+
 //! Atomically set an boost::uint32_t in memory
 //! "mem": pointer to the object
 //! "param": val value that the object will assume
@@ -599,6 +622,22 @@ BOOST_INTERPROCESS_FORCEINLINE boost::uint32_t atomic_read32_acquire(volatile bo
    #endif
 }
 
+//! Same as atomic_read32, but with relaxed semantics
+BOOST_INTERPROCESS_FORCEINLINE boost::uint32_t atomic_read32_relaxed(volatile boost::uint32_t *mem)
+{
+   #if defined(__ATOMIC_RELAXED)
+   //GCC 4.7 and later, Clang: the same plain load instruction a volatile
+   //read would give, but one that participates in the memory model, so
+   //ThreadSanitizer does not flag it as racing other threads' writes
+   return __atomic_load_n(mem, __ATOMIC_RELAXED);
+   #else
+   //GCC 4.1 to 4.6: no __atomic builtins and no ThreadSanitizer either. An
+   //aligned volatile load is a single load instruction on every supported
+   //target, and a relaxed load needs no barrier around it
+   return *mem;
+   #endif
+}
+
 //! Atomically set an boost::uint32_t in memory
 //! "mem": pointer to the object
 //! "param": val value that the object will assume

include/boost/interprocess/sync/detail/common_algorithms.hpp

@@ -36,13 +36,16 @@ namespace ipcdetail {
 //////////////////////////////////////////////////////////////////////////////
 //
 // A mutex may offer "bool maybe_lockable()", a cheap, conservative answer to
-// "would try_lock() have a chance right now?". It is a hint, subject to two
+// "would try_lock() have a chance right now?". It is a hint, subject to three
 // rules:
 //    - returning true is always allowed: the caller then performs the real
 //      try_lock(), which is what actually decides,
 //    - it must not keep returning false while the mutex is in fact free, or a
 //      spinning caller would never take it. Re-reading the state each call is
-//      enough to satisfy this.
+//      enough to satisfy this,
+//    - and, less obviously, a mutex whose try_lock() has side effects the
+//      waiter depends on must NOT implement it. robust_spin_mutex is the
+//      example
 //
 // The point is that for a spin mutex try_lock() is a read-modify-write, and a
 // failed one still takes the cache line for writing, so a loop that retries it

include/boost/interprocess/sync/spin/mutex.hpp

@@ -96,11 +96,11 @@ inline void spin_mutex::lock(void)
    ipcdetail::timeout_when_locking_aware_lock(clw);
 }
 
-//A plain load, which only needs the cache line shared, where try_lock() needs
-//it exclusive. Used by the spin loops in common_algorithms.hpp to avoid
-//hammering the line while the mutex is held.
+//A relaxed load, which only needs the cache line shared, where try_lock()
+//needs it exclusive. Used by the spin loops to avoid
+//hammering the line while the mutex is held, as answer is only a hint.
 BOOST_INTERPROCESS_FORCEINLINE bool spin_mutex::maybe_lockable(void)
-{  return ipcdetail::atomic_read32(const_cast<boost::uint32_t*>(&m_s)) == 0u;  }
+{  return ipcdetail::atomic_read32_relaxed(&m_s) == 0u;  }
 
 BOOST_INTERPROCESS_FORCEINLINE bool spin_mutex::try_lock(void)
 {

test/atomic_test.cpp

@@ -20,6 +20,7 @@
 using namespace boost::interprocess;
 using boost::interprocess::ipcdetail::atomic_read32;
 using boost::interprocess::ipcdetail::atomic_read32_acquire;
+using boost::interprocess::ipcdetail::atomic_read32_relaxed;
 using boost::interprocess::ipcdetail::atomic_write32;
 using boost::interprocess::ipcdetail::atomic_write32_release;
 using boost::interprocess::ipcdetail::atomic_add32;
@@ -58,19 +59,23 @@ void test_read_write()
 
    BOOST_INTERPROCESS_CHECK(atomic_read32(&v) == 0u);
    BOOST_INTERPROCESS_CHECK(atomic_read32_acquire(&v) == 0u);
+   BOOST_INTERPROCESS_CHECK(atomic_read32_relaxed(&v) == 0u);
 
    atomic_write32(&v, 12345u);
    BOOST_INTERPROCESS_CHECK(atomic_read32(&v) == 12345u);
    BOOST_INTERPROCESS_CHECK(atomic_read32_acquire(&v) == 12345u);
+   BOOST_INTERPROCESS_CHECK(atomic_read32_relaxed(&v) == 12345u);
 
    atomic_write32_release(&v, 54321u);
    BOOST_INTERPROCESS_CHECK(atomic_read32(&v) == 54321u);
    BOOST_INTERPROCESS_CHECK(atomic_read32_acquire(&v) == 54321u);
+   BOOST_INTERPROCESS_CHECK(atomic_read32_relaxed(&v) == 54321u);
 
    //The whole 32 bit range must survive a write/read round trip
    const boost::uint32_t all_ones = ~boost::uint32_t(0);
    atomic_write32(&v, all_ones);
    BOOST_INTERPROCESS_CHECK(atomic_read32(&v) == all_ones);
+   BOOST_INTERPROCESS_CHECK(atomic_read32_relaxed(&v) == all_ones);
    atomic_write32_release(&v, all_ones);
    BOOST_INTERPROCESS_CHECK(atomic_read32_acquire(&v) == all_ones);
 
@@ -80,6 +85,7 @@ void test_read_write()
    for(int i = 0; i != 8; ++i){
       BOOST_INTERPROCESS_CHECK(atomic_read32(&v) == 7u);
       BOOST_INTERPROCESS_CHECK(atomic_read32_acquire(&v) == 7u);
+      BOOST_INTERPROCESS_CHECK(atomic_read32_relaxed(&v) == 7u);
    }
    BOOST_INTERPROCESS_CHECK(v == 7u);
 }
@@ -315,7 +321,13 @@ class cas_lock_thread
          //the previous owner is visible to this thread
          spin_wait swait;
          while(atomic_cas32_acquire(&m_data->m_lock, 1u, 0u) != 0u){
-            swait.yield();
+            //Poll with a relaxed load until the lock looks free before
+            //retrying the acquire CAS - spin_mutex's maybe_lockable()
+            //idiom: a read only needs the cache line shared. The value is
+            //just a hint, only the CAS above can actually take the lock
+            while(atomic_read32_relaxed(&m_data->m_lock) != 0u){
+               swait.yield();
+            }
          }
          ++m_data->m_guarded;
          //Unlock with release semantics, so that the write above is visible