[evolution-patches] CamelObject with gslice

[Philip Van Hoof <spam pvanhoof be>]

I patched camel-object.c to use g_slice_alloc*

As I expected it doesn't make a big difference in memory consumption and
characteristics. I only tried this because it was a trivial change and
well, because I want to get rid of older libedataserver API.

Note that there's a bug in the current camel-object.c implementation:

The klass->hook (or type->hook) isn't set to NULL. When adding a hook a
on race condition - crash will happen (in co_find_pair). Once replaced
with g_slice_alloc, that bug of course happened more frequently and
that's also how I spotted this.

A bugfix for that is also in this patch. At the type init simply put
klass->hook = NULL. I guess I don't have to make a separate patch for
this? Invoking your patch command is going to take you longer than
typing that line yourself, right?

A noticeable memory consumption difference happens when replacing the
(after the mmap patch remaining) memchunk_alloc's in the camel-folder-
summary.c. The alloc per CamelMessageInfoBase is, indeed, the real
memory consumer.

Mostly admin allocation go down drastically after replacing this with
g_slice_alloc. I'm soon going to produce a patch that not only replaces
all this with g_slice_alloc, but also reduces the size of the struct
itself. I'm expecting this work to reduce memory consumption with
another 15 to 20% (for tinymail at least).

I haven't remove the EMutex -> GMutex replacements of camel-object.c,
done by Matthew, that are since today also in my own copy. Feel free to
re-replace them with EMutex API yourself or else feel free to ignore
this patch.


-- 
Philip Van Hoof, software developer at x-tend 
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
work: vanhoof at x-tend dot be 
http://www.pvanhoof.be - http://www.x-tend.be

--- /home/pvanhoof/repos/gnome/cvs/evolution-data-server/camel/camel-object.c	2006-08-28 19:24:59.000000000 +0200
+++ camel-object.c	2006-10-18 02:11:39.000000000 +0200
@@ -52,7 +52,7 @@
 
 /* A 'locked' hooklist, that is only allocated on demand */
 typedef struct _CamelHookList {
-	EMutex *lock;
+	GStaticRecMutex lock;
 
 	unsigned int depth:30;	/* recursive event depth */
 	unsigned int flags:2;	/* flags, see below */
@@ -125,7 +125,8 @@
 static void camel_object_free_hooks(CamelObject *o);
 static void camel_object_bag_remove_unlocked(CamelObjectBag *inbag, CamelObject *o, CamelHookList *hooks);
 
-#define camel_object_unget_hooks(o) (e_mutex_unlock((CAMEL_OBJECT(o)->hooks->lock)))
+#define camel_object_unget_hooks(o) \
+	(g_static_rec_mutex_unlock(&CAMEL_OBJECT(o)->hooks->lock))
 
 
 /* ********************************************************************** */
@@ -137,12 +138,12 @@
 static unsigned int pair_id = 1;
 
 /* type-lock must be recursive, for atomically creating classes */
-static EMutex *type_lock;
+static GStaticRecMutex type_lock = G_STATIC_REC_MUTEX_INIT;
 /* ref-lock must be global :-(  for object bags to work */
 static GMutex *ref_lock;
 
 static GHashTable *type_table;
-static EMemChunk *type_chunks;
+// static EMemChunk *type_chunks;
 
 /* fundamental types are accessed via global */
 CamelType camel_object_type;
@@ -150,12 +151,12 @@
 
 #define P_LOCK(l) (pthread_mutex_lock(&l))
 #define P_UNLOCK(l) (pthread_mutex_unlock(&l))
-#define E_LOCK(l) (e_mutex_lock(l))
-#define E_UNLOCK(l) (e_mutex_unlock(l))
 #define CLASS_LOCK(k) (g_mutex_lock((((CamelObjectClass *)k)->lock)))
 #define CLASS_UNLOCK(k) (g_mutex_unlock((((CamelObjectClass *)k)->lock)))
 #define REF_LOCK() (g_mutex_lock(ref_lock))
 #define REF_UNLOCK() (g_mutex_unlock(ref_lock))
+#define TYPE_LOCK() (g_static_rec_mutex_lock(&type_lock))
+#define TYPE_UNLOCK() (g_static_rec_mutex_unlock(&type_lock))
 
 static struct _CamelHookPair *
 pair_alloc(void)
@@ -163,7 +164,7 @@
 	CamelHookPair *pair;
 
 	P_LOCK(chunks_lock);
-	pair = e_memchunk_alloc(pair_chunks);
+	pair = g_slice_new (CamelHookPair);
 	pair->id = pair_id++;
 	if (pair_id == 0)
 		pair_id = 1;
@@ -175,10 +176,8 @@
 static void
 pair_free(CamelHookPair *pair)
 {
-	g_assert(pair_chunks != NULL);
-
 	P_LOCK(chunks_lock);
-	e_memchunk_free(pair_chunks, pair);
+	g_slice_free (CamelHookPair, pair);
 	P_UNLOCK(chunks_lock);
 }
 
@@ -188,7 +187,7 @@
 	CamelHookList *hooks;
 
 	P_LOCK(chunks_lock);
-	hooks = e_memchunk_alloc(hook_chunks);
+	hooks = g_slice_new (CamelHookList);
 	P_UNLOCK(chunks_lock);
 
 	return hooks;
@@ -197,10 +196,8 @@
 static void
 hooks_free(CamelHookList *hooks)
 {
-	g_assert(hook_chunks != NULL);
-
 	P_LOCK(chunks_lock);
-	e_memchunk_free(hook_chunks, hooks);
+	g_slice_free (CamelHookList, hooks);
 	P_UNLOCK(chunks_lock);
 }
 
@@ -214,10 +211,9 @@
 		return;
 
 	init = TRUE;
-	pair_chunks = e_memchunk_new(16, sizeof(CamelHookPair));
-	hook_chunks = e_memchunk_new(16, sizeof(CamelHookList));
-	type_lock = e_mutex_new(E_MUTEX_REC);
-	type_chunks = e_memchunk_new(32, sizeof(CamelType));
+	//pair_chunks = e_memchunk_new(16, sizeof(CamelHookPair));
+	//hook_chunks = e_memchunk_new(16, sizeof(CamelHookList));
+	//type_chunks = e_memchunk_new(32, sizeof(CamelType));
 	type_table = g_hash_table_new(NULL, NULL);
 	ref_lock = g_mutex_new();
 }
@@ -471,7 +467,7 @@
 			switch(argv->argv[argv->argc].tag & CAMEL_ARG_TYPE) {
 			case CAMEL_ARG_INT:
 			case CAMEL_ARG_BOO:
-				if (camel_file_util_decode_uint32(fp, &argv->argv[argv->argc].ca_int) == -1)
+				if (camel_file_util_decode_uint32(fp, (guint32*) &argv->argv[argv->argc].ca_int) == -1)
 					goto cleanup;
 				break;
 			case CAMEL_ARG_STR:
@@ -580,7 +576,7 @@
 
 	res = 0;
 abort:
-	for (i=0;i<argv->argc;i++) {
+/*	for (i=0;i<argv->argc;i++) {
 		CamelArg *arg = &argv->argv[i];
 
 		if ((argv->argv[i].tag & CAMEL_ARG_TYPE) == CAMEL_ARG_STR)
@@ -595,7 +591,7 @@
 
 	if (meta)
 		camel_object_free(obj, CAMEL_OBJECT_METADATA, meta);
-
+*/
 	return res;
 }
 
@@ -723,7 +719,7 @@
 	/*int offset;
 	  size_t size;*/
 
-	E_LOCK(type_lock);
+	TYPE_LOCK();
 
 	/* Have to check creation, it might've happened in another thread before we got here */
 	klass = g_hash_table_lookup(type_table, name);
@@ -734,7 +730,7 @@
 			g_warning("camel_type_register: Trying to re-register class '%s'", name);
 			klass = NULL;
 		}
-		E_UNLOCK(type_lock);
+		TYPE_UNLOCK();
 		return klass;
 	}
 
@@ -758,16 +754,16 @@
 	if (parent
 	    && klass_size < parent->klass_size) {
 		g_warning("camel_type_register: '%s' has smaller class size than parent '%s'", name, parent->name);
-		E_UNLOCK(type_lock);
+		TYPE_UNLOCK();
 		return NULL;
 	}
 
-	klass = g_malloc0(klass_size);
+	klass = g_slice_alloc (klass_size);
 	klass->klass_size = klass_size;
 	klass->object_size = object_size;
 	klass->lock = g_mutex_new();
-	klass->instance_chunks = e_memchunk_new(8, object_size);
-	
+	klass->hooks = NULL;
+
 	klass->parent = parent;
 	if (parent) {
 		klass->next = parent->child;
@@ -789,7 +785,7 @@
 
 	camel_type_class_init(klass, klass);
 
-	E_UNLOCK(type_lock);
+	TYPE_UNLOCK();
 
 	return klass;
 }
@@ -848,15 +844,7 @@
 
 	CLASS_LOCK(type);
 
-	o = e_memchunk_alloc0(type->instance_chunks);
-
-#ifdef CAMEL_OBJECT_TRACK_INSTANCES
-	if (type->instances)
-		type->instances->prev = o;
-	o->next = type->instances;
-	o->prev = NULL;
-	type->instances = o;
-#endif
+	o = g_slice_alloc0 (type->object_size);
 
 	CLASS_UNLOCK(type);
 
@@ -889,6 +877,13 @@
 	register CamelObjectClass *klass, *k;
 	CamelHookList *hooks = NULL;
 
+	if (!CAMEL_IS_OBJECT (o))
+	{
+		printf ("BREAK HERE\n");		
+		printf ("BREAK HERE\n");
+		printf ("BREAK HERE\n");
+	}
+	
 	g_return_if_fail(CAMEL_IS_OBJECT(o));
 	
 	klass = o->klass;
@@ -932,15 +927,8 @@
 	o->magic = CAMEL_OBJECT_FINALISED_MAGIC;
 
 	CLASS_LOCK(klass);
-#ifdef CAMEL_OBJECT_TRACK_INSTANCES
-	if (o->prev)
-		o->prev->next = o->next;
-	else
-		klass->instances = o->next;
-	if (o->next)
-		o->next->prev = o->prev;
-#endif
-	e_memchunk_free(klass->instance_chunks, o);
+
+	g_slice_free1 (klass->object_size, o);
 	CLASS_UNLOCK(klass);
 }
 
@@ -1254,7 +1242,7 @@
 			pair_free(pair);
 			pair = next;
 		}
-		e_mutex_destroy(o->hooks->lock);
+		g_static_rec_mutex_free(&o->hooks->lock);
 		hooks_free(o->hooks);
 		o->hooks = NULL;
 	}
@@ -1273,7 +1261,7 @@
 		pthread_mutex_lock(&lock);
 		if (o->hooks == NULL) {
 			hooks = hooks_alloc();
-			hooks->lock = e_mutex_new(E_MUTEX_REC);
+			g_static_rec_mutex_init(&hooks->lock);
 			hooks->flags = 0;
 			hooks->depth = 0;
 			hooks->list_length = 0;
@@ -1283,7 +1271,7 @@
 		pthread_mutex_unlock(&lock);
 	}
 	
-	e_mutex_lock(o->hooks->lock);
+	g_static_rec_mutex_lock(&o->hooks->lock);
 	
 	return o->hooks;	
 }
@@ -1896,9 +1884,6 @@
 object_class_dump_tree_rec(CamelType root, int depth)
 {
 	char *p;
-#ifdef CAMEL_OBJECT_TRACK_INSTANCES
-	struct _CamelObject *o;
-#endif
 
 	p = alloca(depth*2+1);
 	memset(p, ' ', depth*2);
@@ -1916,22 +1901,7 @@
 				pair = pair->next;
 			}
 		}
-#ifdef CAMEL_OBJECT_TRACK_INSTANCES
-		o = root->instances;
-		while (o) {
-			printf("%s instance %p [%d]\n", p, o, o->ref_count);
-			/* todo: should lock hooks while it scans them */
-			if (o->hooks) {
-				CamelHookPair *pair = o->hooks->list;
-
-				while (pair) {
-					printf("%s  hook '%s' func %p data %p\n", p, pair->name, pair->func.event, pair->data);
-					pair = pair->next;
-				}
-			}
-			o = o->next;
-		}
-#endif
+
 		CLASS_UNLOCK(root);
 
 		if (root->child)