Wed Mar 29 18:24:42 CEST 2006 Paolo Molaro <lupus@ximian.com>

	* threads.c, object-internals.h, verify.c: changed the culture caching
	code to use a normal MonoArray for storage so the GC can keep track of
	them easily. Fixed bits of the cache logic, too and simplified the
	code.

svn path=/trunk/mono/; revision=58739

diff --git a/mono/metadata/ChangeLog b/mono/metadata/ChangeLog
index ef5908f0a5e9a7afda7ed76976092bb0dc571035..42648aec7be78d8645cefbfda93faa1721500b1c 100644 (file)
--- a/mono/metadata/ChangeLog
+++ b/mono/metadata/ChangeLog
@@ -1,4 +1,11 @@
 
+Wed Mar 29 18:24:42 CEST 2006 Paolo Molaro <lupus@ximian.com>
+
+       * threads.c, object-internals.h, verify.c: changed the culture caching
+       code to use a normal MonoArray for storage so the GC can keep track of
+       them easily. Fixed bits of the cache logic, too and simplified the
+       code.
+
 Wed Mar 29 17:18:16 CEST 2006 Paolo Molaro <lupus@ximian.com>
 
        * gc-internal.h, null-gc.c, boehm-gc.c, gc.c: enable the finalizer

diff --git a/mono/metadata/object-internals.h b/mono/metadata/object-internals.h
index f08ad969fd30678cfc6037a5debf23e13a3b53e9..f84035dc8ea747d0ec1503282f24d7ccda312327 100644 (file)
--- a/mono/metadata/object-internals.h
+++ b/mono/metadata/object-internals.h
@@ -221,8 +221,8 @@ struct _MonoThread {
        MonoObject  obj;
        int         lock_thread_id; /* to be used as the pre-shifted thread id in thin locks */
        HANDLE      handle;
-       MonoObject **culture_info;
-       MonoObject **ui_culture_info;
+       MonoArray  *cached_culture_info;
+       gpointer    unused1;
        MonoBoolean threadpool_thread;
        gunichar2  *name;
        guint32     name_len;

diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c
index 4b13c46e484187ef3bb5b07ca09efd7e3a4f0a4e..4e38fd3cfffce0a13ae286b28399bc4cad2d1d6e 100644 (file)
--- a/mono/metadata/threads.c
+++ b/mono/metadata/threads.c
@@ -66,8 +66,13 @@ typedef struct {
        int offset;
 } StaticDataInfo;
 
-/* Number of cached culture objects in the MonoThread->culture_info array */
+/* Number of cached culture objects in the MonoThread->cached_culture_info array
+ * (per-type): we use the first NUM entries for CultureInfo and the last for
+ * UICultureInfo. So the size of the array is really NUM_CACHED_CULTURES * 2.
+ */
 #define NUM_CACHED_CULTURES 4
+#define CULTURES_START_IDX 0
+#define UICULTURES_START_IDX NUM_CACHED_CULTURES
 
 /*
  * The "os_handle" field of the WaitHandle class.
@@ -228,8 +233,7 @@ static void thread_cleanup (MonoThread *thread)
        if (thread->serialized_culture_info)
                g_free (thread->serialized_culture_info);
 
-       mono_gc_free_fixed (thread->culture_info);
-       mono_gc_free_fixed (thread->ui_culture_info);
+       thread->cached_culture_info = NULL;
 
        if (mono_thread_cleanup)
                mono_thread_cleanup (thread);
@@ -653,18 +657,16 @@ ves_icall_System_Threading_Thread_SetName_internal (MonoThread *this_obj, MonoSt
        mono_monitor_exit (this_obj->synch_lock);
 }
 
-MonoObject*
-ves_icall_System_Threading_Thread_GetCachedCurrentCulture (MonoThread *this)
+static MonoObject*
+lookup_cached_culture (MonoThread *this, MonoDomain *domain, int start_idx)
 {
        MonoObject *res;
-       MonoDomain *domain;
        int i;
 
-       /* No need to lock here */
-       if (this->culture_info) {
+       if (this->cached_culture_info) {
                domain = mono_domain_get ();
-               for (i = 0; i < NUM_CACHED_CULTURES; ++i) {
-                       res = this->culture_info [i];
+               for (i = start_idx; i < start_idx + NUM_CACHED_CULTURES; ++i) {
+                       res = mono_array_get (this->cached_culture_info, MonoObject*, i);
                        if (res && res->vtable->domain == domain)
                                return res;
                }
@@ -673,6 +675,12 @@ ves_icall_System_Threading_Thread_GetCachedCurrentCulture (MonoThread *this)
        return NULL;
 }
 
+MonoObject*
+ves_icall_System_Threading_Thread_GetCachedCurrentCulture (MonoThread *this)
+{
+       return lookup_cached_culture (this, mono_domain_get (), CULTURES_START_IDX);
+}
+
 MonoArray*
 ves_icall_System_Threading_Thread_GetSerializedCurrentCulture (MonoThread *this)
 {
@@ -690,32 +698,47 @@ ves_icall_System_Threading_Thread_GetSerializedCurrentCulture (MonoThread *this)
        return res;
 }
 
-void
-ves_icall_System_Threading_Thread_SetCachedCurrentCulture (MonoThread *this, MonoObject *culture)
+static void
+cache_culture (MonoThread *this, MonoObject *culture, int start_idx)
 {
        int i;
        MonoDomain *domain = mono_domain_get ();
+       MonoObject *obj;
+       int free_slot = -1;
+       int same_domain_slot = -1;
 
        mono_monitor_enter (this->synch_lock);
-       if (!this->culture_info) {
-               this->culture_info = mono_gc_alloc_fixed (sizeof (MonoObject*) * NUM_CACHED_CULTURES, NULL);
-       }
-
-       for (i = 0; i < NUM_CACHED_CULTURES; ++i) {
-               if (this->culture_info [i]) {
-                       if (this->culture_info [i]->vtable->domain == domain)
-                               /* Replace */
-                               break;
+       if (!this->cached_culture_info)
+               this->cached_culture_info = mono_array_new (mono_object_domain (this), mono_defaults.object_class, NUM_CACHED_CULTURES * 2);
+
+       for (i = start_idx; i < start_idx + NUM_CACHED_CULTURES; ++i) {
+               obj = mono_array_get (this->cached_culture_info, MonoObject*, i);
+               /* Free entry */
+               if (!obj) {
+                       free_slot = i;
+                       /* we continue, because there may be a slot used with the same domain */
+                       continue;
                }
-               else
-                       /* Free entry */
+               /* Replace */
+               if (obj->vtable->domain == domain) {
+                       same_domain_slot = i;
                        break;
+               }
        }
-       if (i < NUM_CACHED_CULTURES)
-               this->culture_info [i] = culture;
+       if (same_domain_slot >= 0)
+               mono_array_setref (this->cached_culture_info, same_domain_slot, culture);
+       else if (free_slot >= 0)
+               mono_array_setref (this->cached_culture_info, free_slot, culture);
+       /* we may want to replace an existing entry here, even when no suitable slot is found */
        mono_monitor_exit (this->synch_lock);
 }
 
+void
+ves_icall_System_Threading_Thread_SetCachedCurrentCulture (MonoThread *this, MonoObject *culture)
+{
+       cache_culture (this, culture, CULTURES_START_IDX);
+}
+
 void
 ves_icall_System_Threading_Thread_SetSerializedCurrentCulture (MonoThread *this, MonoArray *arr)
 {
@@ -732,21 +755,7 @@ ves_icall_System_Threading_Thread_SetSerializedCurrentCulture (MonoThread *this,
 MonoObject*
 ves_icall_System_Threading_Thread_GetCachedCurrentUICulture (MonoThread *this)
 {
-       MonoObject *res;
-       MonoDomain *domain;
-       int i;
-
-       /* No need to lock here */
-       if (this->ui_culture_info) {
-               domain = mono_domain_get ();
-               for (i = 0; i < NUM_CACHED_CULTURES; ++i) {
-                       res = this->ui_culture_info [i];
-                       if (res && res->vtable->domain == domain)
-                               return res;
-               }
-       }
-
-       return NULL;
+       return lookup_cached_culture (this, mono_domain_get (), UICULTURES_START_IDX);
 }
 
 MonoArray*
@@ -769,27 +778,7 @@ ves_icall_System_Threading_Thread_GetSerializedCurrentUICulture (MonoThread *thi
 void
 ves_icall_System_Threading_Thread_SetCachedCurrentUICulture (MonoThread *this, MonoObject *culture)
 {
-       int i;
-       MonoDomain *domain = mono_domain_get ();
-
-       mono_monitor_enter (this->synch_lock);
-       if (!this->ui_culture_info) {
-               this->ui_culture_info = mono_gc_alloc_fixed (sizeof (MonoObject*) * NUM_CACHED_CULTURES, NULL);
-       }
-
-       for (i = 0; i < NUM_CACHED_CULTURES; ++i) {
-               if (this->ui_culture_info [i]) {
-                       if (this->ui_culture_info [i]->vtable->domain == domain)
-                               /* Replace */
-                               break;
-               }
-               else
-                       /* Free entry */
-                       break;
-       }
-       if (i < NUM_CACHED_CULTURES)
-               this->ui_culture_info [i] = culture;
-       mono_monitor_exit (this->synch_lock);
+       cache_culture (this, culture, UICULTURES_START_IDX);
 }
 
 void
@@ -2352,17 +2341,13 @@ clear_cached_culture (gpointer key, gpointer value, gpointer user_data)
        int i;
 
        /* No locking needed here */
+       /* FIXME: why no locking? writes to the cache are protected with synch_lock above */
 
-       if (thread->culture_info) {
-               for (i = 0; i < NUM_CACHED_CULTURES; ++i) {
-                       if (thread->culture_info [i] && thread->culture_info [i]->vtable->domain == domain)
-                               thread->culture_info [i] = NULL;
-               }
-       }
-       if (thread->ui_culture_info) {
-               for (i = 0; i < NUM_CACHED_CULTURES; ++i) {
-                       if (thread->ui_culture_info [i] && thread->ui_culture_info [i]->vtable->domain == domain)
-                               thread->ui_culture_info [i] = NULL;
+       if (thread->cached_culture_info) {
+               for (i = 0; i < NUM_CACHED_CULTURES * 2; ++i) {
+                       MonoObject *obj = mono_array_get (thread->cached_culture_info, MonoObject*, i);
+                       if (obj && obj->vtable->domain == domain)
+                               mono_array_set (thread->cached_culture_info, MonoObject*, i, NULL);
                }
        }
 }

diff --git a/mono/metadata/verify.c b/mono/metadata/verify.c
index 29225d6379deaa654de88a882a20c6898a09a3bd..05815d1c827dc960ac8e5000d3601cba988ec28e 100644 (file)
--- a/mono/metadata/verify.c
+++ b/mono/metadata/verify.c
@@ -2505,7 +2505,6 @@ wait_handle_fields[] = {
 static FieldDesc 
 thread_fields[] = {
        {"system_thread_handle", G_STRUCT_OFFSET (MonoThread, handle)},
-       {"current_culture", G_STRUCT_OFFSET (MonoThread, culture_info)},
        {"threadpool_thread", G_STRUCT_OFFSET (MonoThread, threadpool_thread)},
        {"state", G_STRUCT_OFFSET (MonoThread, state)},
        {"abort_exc", G_STRUCT_OFFSET (MonoThread, abort_exc)},