Merge pull request #4019 from vargaz/get-gclass-locking
authorBernhard Urban <bernhard.urban@xamarin.com>
Thu, 24 Nov 2016 20:57:00 +0000 (21:57 +0100)
committerGitHub <noreply@github.com>
Thu, 24 Nov 2016 20:57:00 +0000 (21:57 +0100)
[runtime] Reduce the size of the critical section in mono_generic_cla…

1  2 
mono/metadata/class.c

diff --combined mono/metadata/class.c
index 2ca778896e82109d162c84a0daa3fd6af7a6ff09,579efab271714861ff39d2eb1e62457a42c54063..b1b803710c946b22cb74cc7dc5f6c0fafe274ec3
@@@ -6081,19 -6081,10 +6081,10 @@@ mono_generic_class_get_class (MonoGener
        if (gclass->cached_class)
                return gclass->cached_class;
  
-       mono_loader_lock ();
-       if (gclass->cached_class) {
-               mono_loader_unlock ();
-               return gclass->cached_class;
-       }
        klass = (MonoClass *)mono_image_set_alloc0 (gclass->owner, sizeof (MonoClassGenericInst));
  
        gklass = gclass->container_class;
  
-       if (record_gclass_instantiation > 0)
-               gclass_recorded_list = g_slist_append (gclass_recorded_list, klass);
        if (gklass->nested_in) {
                /* The nested_in type should not be inflated since it's possible to produce a nested type with less generic arguments*/
                klass->nested_in = gklass->nested_in;
        klass->name = gklass->name;
        klass->name_space = gklass->name_space;
        
-       mono_profiler_class_event (klass, MONO_PROFILE_START_LOAD);
-       
        klass->image = gklass->image;
        klass->type_token = gklass->type_token;
  
  
        klass->cast_class = klass->element_class = klass;
  
-       if (mono_class_is_nullable (klass))
-               klass->cast_class = klass->element_class = mono_class_get_nullable_param (klass);
-       /*
-        * We're not interested in the nested classes of a generic instance.
-        * We use the generic type definition to look for nested classes.
-        */
-       mono_generic_class_setup_parent (klass, gklass);
        if (gclass->is_dynamic) {
                /*
                 * We don't need to do any init workf with unbaked typebuilders. Generic instances created at this point will be later unregistered and/or fixed.
                if (!gklass->wastypebuilder)
                        klass->inited = 1;
  
-               mono_class_setup_supertypes (klass);
                if (klass->enumtype) {
                        /*
                         * For enums, gklass->fields might not been set, but instance_size etc. is 
                         */
                        klass->instance_size = gklass->instance_size;
                        klass->sizes.class_size = gklass->sizes.class_size;
-                       mono_memory_barrier ();
                        klass->size_inited = 1;
                }
        }
  
+       mono_loader_lock ();
+       if (gclass->cached_class) {
+               mono_loader_unlock ();
+               return gclass->cached_class;
+       }
+       if (record_gclass_instantiation > 0)
+               gclass_recorded_list = g_slist_append (gclass_recorded_list, klass);
+       if (mono_class_is_nullable (klass))
+               klass->cast_class = klass->element_class = mono_class_get_nullable_param (klass);
+       mono_profiler_class_event (klass, MONO_PROFILE_START_LOAD);
+       mono_generic_class_setup_parent (klass, gklass);
+       if (gclass->is_dynamic)
+               mono_class_setup_supertypes (klass);
        mono_memory_barrier ();
        gclass->cached_class = klass;
  
@@@ -6479,6 -6475,7 +6475,6 @@@ mono_class_from_generic_parameter (Mono
        return mono_class_from_generic_parameter_internal (param);
  }
  
 -
  MonoClass *
  mono_ptr_class_get (MonoType *type)
  {
  static MonoClass *
  mono_fnptr_class_get (MonoMethodSignature *sig)
  {
 -      MonoClass *result;
 +      MonoClass *result, *cached;
        static GHashTable *ptr_hash = NULL;
  
        /* FIXME: These should be allocate from a mempool as well, but which one ? */
  
        mono_loader_lock ();
 -
        if (!ptr_hash)
                ptr_hash = g_hash_table_new (mono_aligned_addr_hash, NULL);
 -      
 -      if ((result = (MonoClass *)g_hash_table_lookup (ptr_hash, sig))) {
 -              mono_loader_unlock ();
 -              return result;
 -      }
 -      result = g_new0 (MonoClass, 1);
 +      cached = (MonoClass *)g_hash_table_lookup (ptr_hash, sig);
 +      mono_loader_unlock ();
 +      if (cached)
 +              return cached;
  
 -      classes_size += sizeof (MonoClassPointer);
 -      ++class_pointer_count;
 +      result = g_new0 (MonoClass, 1);
  
        result->parent = NULL; /* no parent for PTR types */
        result->name_space = "System";
        result->name = "MonoFNPtrFakeClass";
        result->class_kind = MONO_CLASS_POINTER;
  
 -      mono_profiler_class_event (result, MONO_PROFILE_START_LOAD);
 -
        result->image = mono_defaults.corlib; /* need to fix... */
 -      result->inited = TRUE;
        result->instance_size = sizeof (MonoObject) + sizeof (gpointer);
        result->cast_class = result->element_class = result;
 -      result->blittable = TRUE;
 -
        result->byval_arg.type = MONO_TYPE_FNPTR;
        result->this_arg.type = result->byval_arg.type;
        result->this_arg.data.method = result->byval_arg.data.method = sig;
        result->this_arg.byref = TRUE;
        result->blittable = TRUE;
 +      result->inited = TRUE;
  
        mono_class_setup_supertypes (result);
  
 +      mono_loader_lock ();
 +
 +      cached = (MonoClass *)g_hash_table_lookup (ptr_hash, sig);
 +      if (cached) {
 +              g_free (result);
 +              mono_loader_unlock ();
 +              return cached;
 +      }
 +
 +      mono_profiler_class_event (result, MONO_PROFILE_START_LOAD);
 +
 +      classes_size += sizeof (MonoClassPointer);
 +      ++class_pointer_count;
 +
        g_hash_table_insert (ptr_hash, sig, result);
  
        mono_loader_unlock ();
@@@ -6738,7 -6729,7 +6734,7 @@@ MonoClass 
  mono_bounded_array_class_get (MonoClass *eclass, guint32 rank, gboolean bounded)
  {
        MonoImage *image;
 -      MonoClass *klass;
 +      MonoClass *klass, *cached, *k;
        MonoClass *parent = NULL;
        GSList *list, *rootlist = NULL;
        int nsize;
  
        image = eclass->image;
  
 +      /* Check cache */
 +      cached = NULL;
        if (rank == 1 && !bounded) {
 -              /* 
 -               * This case is very frequent not just during compilation because of calls 
 -               * from mono_class_from_mono_type (), mono_array_new (), 
 +              /*
 +               * This case is very frequent not just during compilation because of calls
 +               * from mono_class_from_mono_type (), mono_array_new (),
                 * Array:CreateInstance (), etc, so use a separate cache + a separate lock.
                 */
                mono_os_mutex_lock (&image->szarray_cache_lock);
                if (!image->szarray_cache)
                        image->szarray_cache = g_hash_table_new (mono_aligned_addr_hash, NULL);
 -              klass = (MonoClass *)g_hash_table_lookup (image->szarray_cache, eclass);
 +              cached = (MonoClass *)g_hash_table_lookup (image->szarray_cache, eclass);
                mono_os_mutex_unlock (&image->szarray_cache_lock);
 -              if (klass)
 -                      return klass;
 -
 -              mono_loader_lock ();
        } else {
                mono_loader_lock ();
 -
                if (!image->array_cache)
                        image->array_cache = g_hash_table_new (mono_aligned_addr_hash, NULL);
 -
 -              if ((rootlist = list = (GSList *)g_hash_table_lookup (image->array_cache, eclass))) {
 -                      for (; list; list = list->next) {
 -                              klass = (MonoClass *)list->data;
 -                              if ((klass->rank == rank) && (klass->byval_arg.type == (((rank > 1) || bounded) ? MONO_TYPE_ARRAY : MONO_TYPE_SZARRAY))) {
 -                                      mono_loader_unlock ();
 -                                      return klass;
 -                              }
 +              rootlist = (GSList *)g_hash_table_lookup (image->array_cache, eclass);
 +              for (list = rootlist; list; list = list->next) {
 +                      k = (MonoClass *)list->data;
 +                      if ((k->rank == rank) && (k->byval_arg.type == (((rank > 1) || bounded) ? MONO_TYPE_ARRAY : MONO_TYPE_SZARRAY))) {
 +                              cached = k;
 +                              break;
                        }
                }
 +              mono_loader_unlock ();
        }
 +      if (cached)
 +              return cached;
  
        parent = mono_defaults.array_class;
        if (!parent->inited)
        klass->name = mono_image_strdup (image, name);
        g_free (name);
  
 -      mono_profiler_class_event (klass, MONO_PROFILE_START_LOAD);
 -
 -      classes_size += sizeof (MonoClassArray);
 -      ++class_array_count;
 -
        klass->type_token = 0;
        klass->parent = parent;
        klass->instance_size = mono_class_instance_size (klass->parent);
        klass->this_arg = klass->byval_arg;
        klass->this_arg.byref = 1;
  
 -      //WTF was this? it's wrong
 -      // klass->generic_container = eclass->generic_container;
 +      mono_loader_lock ();
  
 +      /* Check cache again */
 +      cached = NULL;
        if (rank == 1 && !bounded) {
 -              MonoClass *prev_class;
 +              mono_os_mutex_lock (&image->szarray_cache_lock);
 +              cached = (MonoClass *)g_hash_table_lookup (image->szarray_cache, eclass);
 +              mono_os_mutex_unlock (&image->szarray_cache_lock);
 +      } else {
 +              rootlist = (GSList *)g_hash_table_lookup (image->array_cache, eclass);
 +              for (list = rootlist; list; list = list->next) {
 +                      k = (MonoClass *)list->data;
 +                      if ((k->rank == rank) && (k->byval_arg.type == (((rank > 1) || bounded) ? MONO_TYPE_ARRAY : MONO_TYPE_SZARRAY))) {
 +                              cached = k;
 +                              break;
 +                      }
 +              }
 +      }
 +      if (cached) {
 +              mono_loader_unlock ();
 +              return cached;
 +      }
  
 +      mono_profiler_class_event (klass, MONO_PROFILE_START_LOAD);
 +
 +      classes_size += sizeof (MonoClassArray);
 +      ++class_array_count;
 +
 +      if (rank == 1 && !bounded) {
                mono_os_mutex_lock (&image->szarray_cache_lock);
 -              prev_class = (MonoClass *)g_hash_table_lookup (image->szarray_cache, eclass);
 -              if (prev_class)
 -                      /* Someone got in before us */
 -                      klass = prev_class;
 -              else
 -                      g_hash_table_insert (image->szarray_cache, eclass, klass);
 +              g_hash_table_insert (image->szarray_cache, eclass, klass);
                mono_os_mutex_unlock (&image->szarray_cache_lock);
        } else {
                list = g_slist_append (rootlist, klass);