Merge pull request #708 from ianks/update-memory-barriers

Prefer platform specific memory barriers
ruby-concurrency · Jul 6, 2018 · 0e1d60e · 0e1d60e
2 parents 956fc91 + e43ad71
commit 0e1d60e
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 21 deletions.
diff --git a/ext/concurrent/atomic_reference.c b/ext/concurrent/atomic_reference.c
@@ -7,8 +7,38 @@
 /*#include <libkern/OSAtomic.h>*/
 /*#endif*/
 
+/*
+Following the wisdom of postgres, we opt to use platform specific memory barriers when available.
+These are generally more performant. In this PR, we add specific cases for i386, x86_64.
+
+In the future, we could look at using pg's atomics library directly:
+https://github.com/postgres/postgres/tree/9d4649ca49416111aee2c84b7e4441a0b7aa2fac/src/include/port/atomics
+
+Point of contact @ianks
+*/
+
 #include "atomic_reference.h"
 
+#if (defined(__i386__) || defined(__i386)) && (defined(__GNUC__) || defined(__INTEL_COMPILER))
+#define memory_barrier() \
+  __asm__ __volatile__ ("lock; addl $0,0(%%esp)" : : : "memory", "cc")
+#elif defined(__x86_64__) && (defined(__GNUC__) || defined(__INTEL_COMPILER))
+#define memory_barrier() \
+  __asm__ __volatile__ ("lock; addl $0,0(%%rsp)" : : : "memory", "cc")
+#elif defined(HAVE_GCC__ATOMIC_INT32_CAS)
+#define memory_barrier() \
+  __atomic_thread_fence(__ATOMIC_SEQ_CST)
+#elif (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 1))
+#define memory_barrier() \
+  __sync_synchronize();
+#elif defined _MSC_VER
+#define memory_barrier() \
+  MemoryBarrier();
+#elif __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050
+#define memory_barrier() \
+  OSMemoryBarrier();
+#endif
+
 void ir_mark(void *value) {
   rb_gc_mark_maybe((VALUE) value);
 }
@@ -27,25 +57,13 @@ VALUE ir_initialize(int argc, VALUE* argv, VALUE self) {
 }
 
 VALUE ir_get(VALUE self) {
-#if HAVE_GCC_SYNC
-  __sync_synchronize();
-#elif defined _MSC_VER
-  MemoryBarrier();
-#elif __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050
-  OSMemoryBarrier();
-#endif
+  memory_barrier();
   return (VALUE) DATA_PTR(self);
 }
 
 VALUE ir_set(VALUE self, VALUE new_value) {
   DATA_PTR(self) = (void *) new_value;
-#if HAVE_GCC_SYNC
-  __sync_synchronize();
-#elif defined _MSC_VER
-  MemoryBarrier();
-#elif __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050
-  OSMemoryBarrier();
-#endif
+  memory_barrier();
   return new_value;
 }
 

diff --git a/ext/concurrent/extconf.rb b/ext/concurrent/extconf.rb
@@ -43,13 +43,6 @@ def compiler_is_gcc
       end
     end
 
-    try_run(<<CODE,$CFLAGS) && ($defs << '-DHAVE_GCC_SYNC')
-      int main() {
-        __sync_synchronize();
-        return 0;
-      }
-CODE
-
     create_makefile('concurrent/' + EXTENSION_NAME)
   rescue
     create_dummy_makefile