Merge pull request #3083 from alexrp/gc-pointless-memory-barrier
authorRodrigo Kumpera <kumpera@gmail.com>
Mon, 6 Jun 2016 15:37:39 +0000 (08:37 -0700)
committerRodrigo Kumpera <kumpera@gmail.com>
Mon, 6 Jun 2016 15:37:39 +0000 (08:37 -0700)
[sgen] Remove a pointless memory barrier from the middle of the managed allocator.

1  2 
mono/metadata/sgen-mono.c
mono/sgen/sgen-alloc.c

index 339a4de7e4057eff833cd9db1a9de4b605a9ba35,97aa337954fdc9c0aacd38d1effc9b45a9077a63..adb1419600a72ec1b51bf697ed1d91633eaec690
@@@ -26,7 -26,6 +26,7 @@@
  #include "metadata/handle.h"
  #include "utils/mono-memory-model.h"
  #include "utils/mono-logger-internals.h"
 +#include "utils/mono-threads-coop.h"
  #include "sgen/sgen-thread-pool.h"
  
  #ifdef HEAVY_STATISTICS
@@@ -994,22 -993,13 +994,22 @@@ static gboolean use_managed_allocator 
  
  #ifdef HAVE_KW_THREAD
  
 -#define EMIT_TLS_ACCESS_NEXT_ADDR(mb) do {    \
 +#define EMIT_TLS_ACCESS_VAR(_mb, _var) /* nothing to do */
 +
 +#define EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR(mb, _var) \
 +      do { \
 +              mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX); \
 +              mono_mb_emit_byte ((mb), CEE_MONO_TLS); \
 +              mono_mb_emit_i4 ((mb), TLS_KEY_SGEN_IN_CRITICAL_REGION_ADDR); \
 +      } while (0)
 +
 +#define EMIT_TLS_ACCESS_NEXT_ADDR(mb, _var)   do {    \
        mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX);   \
        mono_mb_emit_byte ((mb), CEE_MONO_TLS);         \
        mono_mb_emit_i4 ((mb), TLS_KEY_SGEN_TLAB_NEXT_ADDR);            \
        } while (0)
  
 -#define EMIT_TLS_ACCESS_TEMP_END(mb)  do {    \
 +#define EMIT_TLS_ACCESS_TEMP_END(mb, _var)    do {    \
        mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX);   \
        mono_mb_emit_byte ((mb), CEE_MONO_TLS);         \
        mono_mb_emit_i4 ((mb), TLS_KEY_SGEN_TLAB_TEMP_END);             \
  #else
  
  #if defined(TARGET_OSX) || defined(TARGET_WIN32) || defined(TARGET_ANDROID) || defined(TARGET_IOS)
 -#define EMIT_TLS_ACCESS_NEXT_ADDR(mb) do {    \
 -      mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX);   \
 -      mono_mb_emit_byte ((mb), CEE_MONO_TLS);         \
 -      mono_mb_emit_i4 ((mb), TLS_KEY_SGEN_THREAD_INFO);       \
 +
 +// Cache the SgenThreadInfo pointer in a local 'var'.
 +#define EMIT_TLS_ACCESS_VAR(mb, var) \
 +      do { \
 +              var = mono_mb_add_local ((mb), &mono_defaults.int_class->byval_arg); \
 +              mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX); \
 +              mono_mb_emit_byte ((mb), CEE_MONO_TLS); \
 +              mono_mb_emit_i4 ((mb), TLS_KEY_SGEN_THREAD_INFO); \
 +              mono_mb_emit_stloc ((mb), (var)); \
 +      } while (0)
 +
 +#define EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR(mb, var) \
 +      do { \
 +              mono_mb_emit_ldloc ((mb), (var)); \
 +              mono_mb_emit_icon ((mb), MONO_STRUCT_OFFSET (SgenClientThreadInfo, in_critical_region)); \
 +              mono_mb_emit_byte ((mb), CEE_ADD); \
 +      } while (0)
 +
 +#define EMIT_TLS_ACCESS_NEXT_ADDR(mb, var)    do {    \
 +      mono_mb_emit_ldloc ((mb), (var));               \
        mono_mb_emit_icon ((mb), MONO_STRUCT_OFFSET (SgenThreadInfo, tlab_next_addr));  \
        mono_mb_emit_byte ((mb), CEE_ADD);              \
        mono_mb_emit_byte ((mb), CEE_LDIND_I);          \
        } while (0)
  
 -#define EMIT_TLS_ACCESS_TEMP_END(mb)  do {    \
 -      mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX);   \
 -      mono_mb_emit_byte ((mb), CEE_MONO_TLS);         \
 -      mono_mb_emit_i4 ((mb), TLS_KEY_SGEN_THREAD_INFO);       \
 +#define EMIT_TLS_ACCESS_TEMP_END(mb, var)     do {    \
 +      mono_mb_emit_ldloc ((mb), (var));               \
        mono_mb_emit_icon ((mb), MONO_STRUCT_OFFSET (SgenThreadInfo, tlab_temp_end));   \
        mono_mb_emit_byte ((mb), CEE_ADD);              \
        mono_mb_emit_byte ((mb), CEE_LDIND_I);          \
        } while (0)
  
  #else
 -#define EMIT_TLS_ACCESS_NEXT_ADDR(mb) do { g_error ("sgen is not supported when using --with-tls=pthread.\n"); } while (0)
 -#define EMIT_TLS_ACCESS_TEMP_END(mb)  do { g_error ("sgen is not supported when using --with-tls=pthread.\n"); } while (0)
 +#define EMIT_TLS_ACCESS_VAR(mb, _var) do { g_error ("sgen is not supported when using --with-tls=pthread.\n"); } while (0)
 +#define EMIT_TLS_ACCESS_NEXT_ADDR(mb, _var)   do { g_error ("sgen is not supported when using --with-tls=pthread.\n"); } while (0)
 +#define EMIT_TLS_ACCESS_TEMP_END(mb, _var)    do { g_error ("sgen is not supported when using --with-tls=pthread.\n"); } while (0)
 +#define EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR(mb, _var)     do { g_error ("sgen is not supported when using --with-tls=pthread.\n"); } while (0)
  #endif
  
  #endif
   * that they are executed atomically via the restart mechanism.
   */
  static MonoMethod*
 -create_allocator (int atype, gboolean slowpath)
 +create_allocator (int atype, ManagedAllocatorVariant variant)
  {
 -      int p_var, size_var;
 +      int p_var, size_var, thread_var G_GNUC_UNUSED;
 +      gboolean slowpath = variant == MANAGED_ALLOCATOR_SLOW_PATH;
        guint32 slowpath_branch, max_size_branch;
        MonoMethodBuilder *mb;
        MonoMethod *res;
                goto done;
        }
  
 +      EMIT_TLS_ACCESS_VAR (mb, thread_var);
 +
 +#ifdef MANAGED_ALLOCATOR_CAN_USE_CRITICAL_REGION
 +      EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR (mb, thread_var);
 +      mono_mb_emit_byte (mb, CEE_LDC_I4_1);
 +      mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
 +      mono_mb_emit_byte (mb, CEE_MONO_ATOMIC_STORE_I4);
 +      mono_mb_emit_i4 (mb, MONO_MEMORY_BARRIER_NONE);
 +#endif
 +
        size_var = mono_mb_add_local (mb, &mono_defaults.int_class->byval_arg);
        if (atype == ATYPE_SMALL) {
                /* size_var = size_arg */
  
        /* tlab_next_addr (local) = tlab_next_addr (TLS var) */
        tlab_next_addr_var = mono_mb_add_local (mb, &mono_defaults.int_class->byval_arg);
 -      EMIT_TLS_ACCESS_NEXT_ADDR (mb);
 +      EMIT_TLS_ACCESS_NEXT_ADDR (mb, thread_var);
        mono_mb_emit_stloc (mb, tlab_next_addr_var);
  
        /* p = (void**)tlab_next; */
  
        /* if (G_LIKELY (new_next < tlab_temp_end)) */
        mono_mb_emit_ldloc (mb, new_next_var);
 -      EMIT_TLS_ACCESS_TEMP_END (mb);
 +      EMIT_TLS_ACCESS_TEMP_END (mb, thread_var);
        slowpath_branch = mono_mb_emit_short_branch (mb, MONO_CEE_BLT_UN_S);
  
        /* Slowpath */
        mono_mb_emit_ldloc (mb, new_next_var);
        mono_mb_emit_byte (mb, CEE_STIND_I);
  
-       /*The tlab store must be visible before the the vtable store. This could be replaced with a DDS but doing it with IL would be tricky. */
-       mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
-       mono_mb_emit_byte (mb, CEE_MONO_MEMORY_BARRIER);
-       mono_mb_emit_i4 (mb, MONO_MEMORY_BARRIER_REL);
        /* *p = vtable; */
        mono_mb_emit_ldloc (mb, p_var);
        mono_mb_emit_ldarg (mb, 0);
                mono_mb_emit_byte (mb, MONO_CEE_STIND_I4);
        }
  
 +#ifdef MANAGED_ALLOCATOR_CAN_USE_CRITICAL_REGION
 +      EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR (mb, thread_var);
 +      mono_mb_emit_byte (mb, CEE_LDC_I4_0);
 +      mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
 +      mono_mb_emit_byte (mb, CEE_MONO_ATOMIC_STORE_I4);
 +#else
 +      mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
 +      mono_mb_emit_byte (mb, CEE_MONO_MEMORY_BARRIER);
 +#endif
        /*
        We must make sure both vtable and max_length are globaly visible before returning to managed land.
        */
 -      mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
 -      mono_mb_emit_byte (mb, CEE_MONO_MEMORY_BARRIER);
        mono_mb_emit_i4 (mb, MONO_MEMORY_BARRIER_REL);
  
        /* return p */
@@@ -1447,19 -1398,17 +1442,19 @@@ mono_gc_get_managed_allocator (MonoClas
                return NULL;
        if (known_instance_size && ALIGN_TO (klass->instance_size, SGEN_ALLOC_ALIGN) >= SGEN_MAX_SMALL_OBJ_SIZE)
                return NULL;
 -      if (mono_class_has_finalizer (klass) || mono_class_is_marshalbyref (klass) || (mono_profiler_get_events () & MONO_PROFILE_ALLOCATIONS))
 +      if (mono_class_has_finalizer (klass) || mono_class_is_marshalbyref (klass))
                return NULL;
        if (klass->rank)
                return NULL;
 +      if (mono_profiler_get_events () & MONO_PROFILE_ALLOCATIONS)
 +              return NULL;
        if (klass->byval_arg.type == MONO_TYPE_STRING)
 -              return mono_gc_get_managed_allocator_by_type (ATYPE_STRING, FALSE);
 +              return mono_gc_get_managed_allocator_by_type (ATYPE_STRING, MANAGED_ALLOCATOR_REGULAR);
        /* Generic classes have dynamic field and can go above MAX_SMALL_OBJ_SIZE. */
        if (known_instance_size)
 -              return mono_gc_get_managed_allocator_by_type (ATYPE_SMALL, FALSE);
 +              return mono_gc_get_managed_allocator_by_type (ATYPE_SMALL, MANAGED_ALLOCATOR_REGULAR);
        else
 -              return mono_gc_get_managed_allocator_by_type (ATYPE_NORMAL, FALSE);
 +              return mono_gc_get_managed_allocator_by_type (ATYPE_NORMAL, MANAGED_ALLOCATOR_REGULAR);
  #else
        return NULL;
  #endif
@@@ -1479,7 -1428,7 +1474,7 @@@ mono_gc_get_managed_array_allocator (Mo
                return NULL;
        g_assert (!mono_class_has_finalizer (klass) && !mono_class_is_marshalbyref (klass));
  
 -      return mono_gc_get_managed_allocator_by_type (ATYPE_VECTOR, FALSE);
 +      return mono_gc_get_managed_allocator_by_type (ATYPE_VECTOR, MANAGED_ALLOCATOR_REGULAR);
  #else
        return NULL;
  #endif
@@@ -1492,11 -1441,11 +1487,11 @@@ sgen_set_use_managed_allocator (gboolea
  }
  
  MonoMethod*
 -mono_gc_get_managed_allocator_by_type (int atype, gboolean slowpath)
 +mono_gc_get_managed_allocator_by_type (int atype, ManagedAllocatorVariant variant)
  {
  #ifdef MANAGED_ALLOCATION
        MonoMethod *res;
 -      MonoMethod **cache = slowpath ? slowpath_alloc_method_cache : alloc_method_cache;
 +      MonoMethod **cache;
  
        if (!use_managed_allocator)
                return NULL;
        if (!mono_runtime_has_tls_get ())
                return NULL;
  
 +      switch (variant) {
 +      case MANAGED_ALLOCATOR_REGULAR: cache = alloc_method_cache; break;
 +      case MANAGED_ALLOCATOR_SLOW_PATH: cache = slowpath_alloc_method_cache; break;
 +      default: g_assert_not_reached (); break;
 +      }
 +
        res = cache [atype];
        if (res)
                return res;
  
 -      res = create_allocator (atype, slowpath);
 +      res = create_allocator (atype, variant);
        LOCK_GC;
        if (cache [atype]) {
                mono_free_method (res);
diff --combined mono/sgen/sgen-alloc.c
index 0c824caa9eb7a0c4871dfb1151e4c3f46e29b774,221b7ba7ea7145a4d5be21521cd46b8a4f68a289..dcf977c06e1b736e4f46ad3ec4ea9a0d760eb2af
@@@ -68,9 -68,6 +68,9 @@@ static __thread char *tlab_temp_end
  static __thread char *tlab_real_end;
  /* Used by the managed allocator/wbarrier */
  static __thread char **tlab_next_addr MONO_ATTR_USED;
 +#ifndef SGEN_WITHOUT_MONO
 +static __thread volatile int *in_critical_region_addr MONO_ATTR_USED;
 +#endif
  #endif
  
  #ifdef HAVE_KW_THREAD
@@@ -200,11 -197,6 +200,6 @@@ sgen_alloc_obj_nolock (GCVTable vtable
                if (G_LIKELY (new_next < TLAB_TEMP_END)) {
                        /* Fast path */
  
-                       /* 
-                        * FIXME: We might need a memory barrier here so the change to tlab_next is 
-                        * visible before the vtable store.
-                        */
                        CANARIFY_ALLOC(p,real_size);
                        SGEN_LOG (6, "Allocated object %p, vtable: %p (%s), size: %zd", p, vtable, sgen_client_vtable_get_name (vtable), size);
                        binary_protocol_alloc (p , vtable, size, sgen_client_get_provenance ());
@@@ -509,9 -501,6 +504,9 @@@ sgen_init_tlab_info (SgenThreadInfo* in
  
  #ifdef HAVE_KW_THREAD
        tlab_next_addr = &tlab_next;
 +#ifndef SGEN_WITHOUT_MONO
 +      in_critical_region_addr = &info->client_info.in_critical_region;
 +#endif
  #endif
  }
  
@@@ -536,15 -525,13 +531,15 @@@ sgen_init_allocator (void
  #if defined(HAVE_KW_THREAD) && !defined(SGEN_WITHOUT_MONO)
        int tlab_next_addr_offset = -1;
        int tlab_temp_end_offset = -1;
 -
 +      int in_critical_region_addr_offset = -1;
  
        MONO_THREAD_VAR_OFFSET (tlab_next_addr, tlab_next_addr_offset);
        MONO_THREAD_VAR_OFFSET (tlab_temp_end, tlab_temp_end_offset);
 +      MONO_THREAD_VAR_OFFSET (in_critical_region_addr, in_critical_region_addr_offset);
  
        mono_tls_key_set_offset (TLS_KEY_SGEN_TLAB_NEXT_ADDR, tlab_next_addr_offset);
        mono_tls_key_set_offset (TLS_KEY_SGEN_TLAB_TEMP_END, tlab_temp_end_offset);
 +      mono_tls_key_set_offset (TLS_KEY_SGEN_IN_CRITICAL_REGION_ADDR, in_critical_region_addr_offset);
  #endif
  
  #ifdef HEAVY_STATISTICS