Re: Reusing message info instances after remapping

[Philip Van Hoof <spam pvanhoof be>]

Use this one (enables the expunging when removing items)

On Thu, 2006-11-09 at 16:24 +0100, Philip Van Hoof wrote:
> On Thu, 2006-11-09 at 15:42 +0100, Philip Van Hoof wrote:
> > This one seems to work.
> > 
> > Can people please test this (a LOT)?
> 
> I'm still too unsure about the patch to already commit it (this doesn't
> happen a lot) :-)
> 
> It needs testing, again testing and more testing. Once it works, it will
> solve a lot limitations and problems that the mmap technique introduced
> (mostly the fact that any change to a single such header means having to
> reload all other headers. Which is a consequence of the fact that an
> mmap always means mapping the entire file -- in our case it means this,
> you can indeed map only parts of the file. But it's not practical to map
> regions as large as one header instance --).
> 
> I'm still thinking about removing the messages_uid hashtable. Right now
> I have to duplicate the key (this is the uid) because remapping means
> also remapping the memory where the uid points to. That same memory is
> used as key in the hashtable. So also the hashtable (its keys) become
> invalid after unmapping and remapping the file.
> 
> I solved this by duplicating the uid ans feed the hashtable that
> duplicate as key. This is why I said that this is going to consume a
> little bit more memory (a few bytes per header, added with heap-admin
> cost, multiplied by the amount of headers being used).
> 
> The hashtable is actually only really needed for the
> camel_folder_summary_uid function. It can be implemented without this
> hashtable too, of course (a little search in s->messages).
> 
> Thinking, abusing my brains, etc etc :). I can use opinions of the
> clever guys who sit here and read ;)... on this one.
> 
> 
> 
-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be
gnome: pvanhoof at gnome dot org
work: vanhoof at x-tend dot be
blog: http://pvanhoof.be/blog

Index: libtinymail-camel/tny-camel-folder.c
===================================================================
--- libtinymail-camel/tny-camel-folder.c	(revision 1112)
+++ libtinymail-camel/tny-camel-folder.c	(working copy)
@@ -273,7 +273,7 @@
 	TnyCamelFolderPriv *priv = TNY_CAMEL_FOLDER_GET_PRIVATE (self);
 	CamelException ex = CAMEL_EXCEPTION_INITIALISER;
 
-	if (priv->headers_managed > 0)
+	if (FALSE && priv->headers_managed > 0)
 	{
 		g_critical ("Request to expunge a folder denied: there are still "
 			"header instances of this folder active. Destroy them "
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-summary.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-summary.c	(revision 1112)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-summary.c	(working copy)
@@ -40,7 +40,7 @@
 static int summary_header_load (CamelFolderSummary *);
 static int summary_header_save (CamelFolderSummary *, FILE *);
 
-static CamelMessageInfo *message_info_load (CamelFolderSummary *s);
+static CamelMessageInfo *message_info_load (CamelFolderSummary *s, gboolean *must_add);
 static int message_info_save (CamelFolderSummary *s, FILE *out,
 			      CamelMessageInfo *info);
 static gboolean info_set_user_tag(CamelMessageInfo *info, const char *name, const char  *value);
@@ -206,12 +206,12 @@
 }
 
 static CamelMessageInfo *
-message_info_load (CamelFolderSummary *s)
+message_info_load (CamelFolderSummary *s, gboolean *must_add)
 {
 	CamelMessageInfo *info;
 	CamelImapMessageInfo *iinfo;
 
-	info = camel_imap_summary_parent->message_info_load (s);
+	info = camel_imap_summary_parent->message_info_load (s, must_add);
 	iinfo = (CamelImapMessageInfo*)info;
 
 	if (info) {
Index: libtinymail-camel/camel-lite/camel/providers/imapp/camel-imapp-summary.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imapp/camel-imapp-summary.c	(revision 1112)
+++ libtinymail-camel/camel-lite/camel/providers/imapp/camel-imapp-summary.c	(working copy)
@@ -40,7 +40,7 @@
 static int summary_header_load(CamelFolderSummary *);
 static int summary_header_save(CamelFolderSummary *, FILE *);
 
-static CamelMessageInfo *message_info_load(CamelFolderSummary *s);
+static CamelMessageInfo *message_info_load(CamelFolderSummary *s, gboolean *must_add);
 static int message_info_save(CamelFolderSummary *s, FILE *out, CamelMessageInfo *info);
 
 static void camel_imapp_summary_class_init(CamelIMAPPSummaryClass *klass);
@@ -159,12 +159,12 @@
 
 
 static CamelMessageInfo *
-message_info_load(CamelFolderSummary *s)
+message_info_load(CamelFolderSummary *s, gboolean *must_add)
 {
 	CamelMessageInfo *info;
 	CamelIMAPPMessageInfo *iinfo;
 
-	info = camel_imapp_summary_parent->message_info_load(s);
+	info = camel_imapp_summary_parent->message_info_load(s, must_add);
 	if (info) {
 		unsigned char *ptrchr = s->filepos;
 		iinfo =(CamelIMAPPMessageInfo *)info;
Index: libtinymail-camel/camel-lite/camel/providers/local/camel-mbox-summary.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/local/camel-mbox-summary.c	(revision 1112)
+++ libtinymail-camel/camel-lite/camel/providers/local/camel-mbox-summary.c	(working copy)
@@ -57,7 +57,7 @@
 
 static CamelMessageInfo * message_info_new_from_header(CamelFolderSummary *, struct _camel_header_raw *);
 static CamelMessageInfo * message_info_new_from_parser(CamelFolderSummary *, CamelMimeParser *);
-static CamelMessageInfo * message_info_load (CamelFolderSummary *);
+static CamelMessageInfo * message_info_load (CamelFolderSummary *, gboolean *);
 static int		  message_info_save (CamelFolderSummary *, FILE *, CamelMessageInfo *);
 static int 		  meta_message_info_save(CamelFolderSummary *s, FILE *out_meta, FILE *out, CamelMessageInfo *mi);
 /*static void		  message_info_free (CamelFolderSummary *, CamelMessageInfo *);*/
@@ -378,13 +378,13 @@
 
 
 static CamelMessageInfo *
-message_info_load(CamelFolderSummary *s)
+message_info_load(CamelFolderSummary *s, gboolean *must_add)
 {
 	CamelMessageInfo *mi;
 
 	io(printf("loading mbox message info\n"));
 
-	mi = ((CamelFolderSummaryClass *)camel_mbox_summary_parent)->message_info_load(s);
+	mi = ((CamelFolderSummaryClass *)camel_mbox_summary_parent)->message_info_load(s, must_add);
 	if (mi) {
 		CamelMboxMessageInfo *mbi = (CamelMboxMessageInfo *)mi;
 
Index: libtinymail-camel/camel-lite/camel/providers/local/camel-maildir-summary.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/local/camel-maildir-summary.c	(revision 1112)
+++ libtinymail-camel/camel-lite/camel/providers/local/camel-maildir-summary.c	(working copy)
@@ -48,7 +48,7 @@
 
 #define CAMEL_MAILDIR_SUMMARY_VERSION (0x2000)
 
-static CamelMessageInfo *message_info_load(CamelFolderSummary *s);
+static CamelMessageInfo *message_info_load(CamelFolderSummary *s, gboolean *must_add);
 static CamelMessageInfo *message_info_new_from_header(CamelFolderSummary *, struct _camel_header_raw *);
 static void message_info_free(CamelFolderSummary *, CamelMessageInfo *mi);
 
@@ -385,12 +385,12 @@
 }
 
 static CamelMessageInfo *
-message_info_load(CamelFolderSummary *s)
+message_info_load(CamelFolderSummary *s, gboolean *must_add)
 {
 	CamelMessageInfo *mi;
 	CamelMaildirSummary *mds = (CamelMaildirSummary *)s;
 
-	mi = ((CamelFolderSummaryClass *) parent_class)->message_info_load(s);
+	mi = ((CamelFolderSummaryClass *) parent_class)->message_info_load(s, must_add);
 	if (mi) {
 		char *name;
 
Index: libtinymail-camel/camel-lite/camel/camel-folder-summary.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-folder-summary.c	(revision 1112)
+++ libtinymail-camel/camel-lite/camel/camel-folder-summary.c	(working copy)
@@ -99,7 +99,7 @@
 static CamelMessageInfo * message_info_new_from_header(CamelFolderSummary *, struct _camel_header_raw *);
 static CamelMessageInfo * message_info_new_from_parser(CamelFolderSummary *, CamelMimeParser *);
 static CamelMessageInfo * message_info_new_from_message(CamelFolderSummary *s, CamelMimeMessage *msg);
-static CamelMessageInfo * message_info_load(CamelFolderSummary *);
+static CamelMessageInfo * message_info_load(CamelFolderSummary *, gboolean *must_add);
 static int		  message_info_save(CamelFolderSummary *, FILE *, CamelMessageInfo *);
 static void		  message_info_free(CamelFolderSummary *, CamelMessageInfo *);
 
@@ -125,6 +125,12 @@
 
 static CamelObjectClass *camel_folder_summary_parent;
 
+static void 
+messages_uid_destroy_key (gpointer key)
+{
+	g_free (key);
+}
+
 static void
 camel_folder_summary_init (CamelFolderSummary *s)
 {
@@ -145,10 +151,10 @@
 	s->flags = 0;
 	s->time = 0;
 	s->nextuid = 1;
+	s->in_reload = FALSE;
 
-
 	s->messages = g_ptr_array_new();
-	s->messages_uid = g_hash_table_new(g_str_hash, g_str_equal);
+	s->messages_uid = g_hash_table_new_full (g_str_hash, g_str_equal, messages_uid_destroy_key, NULL);
 	
 	p->summary_lock = g_mutex_new();
 	p->io_lock = g_mutex_new();
@@ -181,17 +187,21 @@
 
 	p = _PRIVATE(s);
 
-	camel_folder_summary_clear(s);
+	/* camel_folder_summary_clear(s); */
+
 	if (s->file)
 		g_mapped_file_free (s->file);
 	s->file = NULL;
+/*
+	camel_folder_summary_remove_range (s, 0, s->messages->len-1);
+	// g_ptr_array_foreach (s->messages, foreach_msginfo, (gpointer)s->message_info_size);
+	//if (s->messages->len > 0)
+	//	g_ptr_array_remove_range (s->messages, 0, s->messages->len-1);
+	//g_hash_table_foreach_remove (s->messages_uid, always_true, NULL);
+*/
 
-	g_ptr_array_foreach (s->messages, foreach_msginfo, (gpointer)s->message_info_size);
-	if (s->messages->len > 0)
-		g_ptr_array_remove_range (s->messages, 0, s->messages->len-1);
-	g_hash_table_foreach_remove (s->messages_uid, always_true, NULL);
 	g_hash_table_foreach(p->filter_charset, free_o_name, 0);
-	
+
 	if (p->filter_index)
 		camel_object_unref((CamelObject *)p->filter_index);
 	if (p->filter_64)
@@ -209,6 +219,7 @@
 		camel_object_unref((CamelObject *)p->filter_stream);
 	if (p->index)
 		camel_object_unref((CamelObject *)p->index);
+
 }
 
 static void
@@ -610,7 +621,6 @@
 	int i;
 	CamelMessageInfo *mi;
 
-
 	if (s->summary_path == NULL || !g_file_test (s->summary_path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR))
 		return 0;
 
@@ -626,7 +636,8 @@
 
 	/* now read in each message ... */
 	for (i=0;i<s->saved_count;i++) {
-		mi = ((CamelFolderSummaryClass *)(CAMEL_OBJECT_GET_CLASS(s)))->message_info_load(s);
+		gboolean must_add = FALSE;
+		mi = ((CamelFolderSummaryClass *)(CAMEL_OBJECT_GET_CLASS(s)))->message_info_load(s, &must_add);
 
 		if (mi == NULL)
 			goto error;
@@ -641,7 +652,8 @@
 			}
 		}
 
-		camel_folder_summary_mmap_add(s, mi);
+		if (must_add)
+			camel_folder_summary_mmap_add(s, mi);
 	}
 
 
@@ -741,13 +753,20 @@
 
 	for (i = 0; i < count; i++) {
 		mi = s->messages->pdata[i];
+
 		if (((CamelFolderSummaryClass *)(CAMEL_OBJECT_GET_CLASS (s)))->message_info_save (s, out, mi) == -1)
 			goto exception;
-		
+
 		if (s->build_content) {
 			if (perform_content_info_save (s, out, ((CamelMessageInfoBase *)mi)->content) == -1)
 				goto exception;
 		}
+
+		if (! (((CamelMessageInfoBase*)mi)->flags & CAMEL_MESSAGE_INFO_UID_NEEDS_FREE))
+		{
+			mi->uid = g_strdup (mi->uid);
+			((CamelMessageInfoBase*)mi)->flags |= CAMEL_MESSAGE_INFO_UID_NEEDS_FREE;
+		}
 	}
 
 	if (fflush (out) != 0 || fsync (fileno (out)) == -1)
@@ -757,6 +776,8 @@
 	
 	fclose (out);
 
+	s->in_reload = TRUE;
+
 	camel_folder_summary_unload_mmap (s);
 #ifdef G_OS_WIN32
 	g_unlink(s->summary_path);
@@ -769,6 +790,8 @@
 	}
 	camel_folder_summary_load (s);
 
+	s->in_reload = FALSE;
+
 	s->flags &= ~CAMEL_SUMMARY_DIRTY;
 
 	g_mutex_unlock (s->dump_lock);
@@ -881,6 +904,8 @@
 void
 camel_folder_summary_add(CamelFolderSummary *s, CamelMessageInfo *info)
 {
+	gchar *uidn;
+
 	g_mutex_lock (s->dump_lock);
 
 	if (info == NULL) 
@@ -905,7 +930,8 @@
 #endif
 
 	g_ptr_array_add(s->messages, info);
-	g_hash_table_insert(s->messages_uid, (char *)camel_message_info_uid(info), info);
+	uidn = g_strdup (camel_message_info_uid(info));
+	g_hash_table_insert(s->messages_uid, uidn, info);
 	s->flags |= CAMEL_SUMMARY_DIRTY;
 
 	CAMEL_SUMMARY_UNLOCK(s, summary_lock);
@@ -916,6 +942,8 @@
 static void
 camel_folder_summary_mmap_add(CamelFolderSummary *s, CamelMessageInfo *info)
 {
+	gchar *uidn;
+
 	CAMEL_SUMMARY_LOCK(s, summary_lock);
 
 /* unnecessary for pooled vectors */
@@ -926,7 +954,8 @@
 #endif
 
 	g_ptr_array_add(s->messages, info);
-	g_hash_table_insert(s->messages_uid, (char *)camel_message_info_uid(info), info);
+	uidn = g_strdup (camel_message_info_uid(info));
+	g_hash_table_insert(s->messages_uid, uidn, info);
 	s->flags |= CAMEL_SUMMARY_DIRTY;
 
 	CAMEL_SUMMARY_UNLOCK(s, summary_lock);
@@ -1869,26 +1898,59 @@
 
 
 static CamelMessageInfo *
-message_info_load(CamelFolderSummary *s)
+message_info_load(CamelFolderSummary *s, gboolean *must_add)
 {
-	CamelMessageInfoBase *mi;
+	CamelMessageInfoBase *mi = NULL;
 	guint count, len;
 	unsigned char *ptrchr = s->filepos;
 	unsigned int i;
+	gchar *theuid;
 
 #ifndef NON_TINYMAIL_FEATURES
 	unsigned int size;
 #endif
 
-	mi = (CamelMessageInfoBase *)camel_message_info_new(s);
-
 	io(printf("Loading message info\n"));
 
 	ptrchr = camel_file_util_mmap_decode_uint32 (ptrchr, &len, TRUE);
 	if (len) 
-		mi->uid = (char*)ptrchr;
+		theuid = (char*)ptrchr;
 	ptrchr += len;
 
+	if (!s->in_reload)
+	{
+		mi = (CamelMessageInfoBase *)camel_message_info_new(s);
+		*must_add = TRUE;
+		mi->uid = theuid;
+	} else
+	{
+		CamelMessageInfoBase *ni = (CamelMessageInfoBase*) camel_folder_summary_uid (s, theuid);
+		
+		if (ni)
+		{
+			printf ("Found %s\n", theuid);
+			//if (ni->flags & CAMEL_MESSAGE_INFO_UID_NEEDS_FREE)
+			//{
+			//	g_free (ni->uid);
+			//	ni->flags &= ~CAMEL_MESSAGE_INFO_UID_NEEDS_FREE;
+			//}
+			mi = ni;
+		}
+	}
+
+	i = 0;
+
+	if (!mi)
+	{
+		printf ("%s not found\n", theuid);
+		mi = (CamelMessageInfoBase *)camel_message_info_new(s);
+		*must_add = TRUE;
+		
+		mi->uid = theuid;
+	}
+
+	
+
 	ptrchr = camel_file_util_mmap_decode_uint32 (ptrchr, &mi->flags, FALSE);
 
 	mi->flags &= ~CAMEL_MESSAGE_INFO_UID_NEEDS_FREE;
Index: libtinymail-camel/camel-lite/camel/camel-folder-summary.h
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-folder-summary.h	(revision 1112)
+++ libtinymail-camel/camel-lite/camel/camel-folder-summary.h	(working copy)
@@ -235,7 +235,7 @@
 	char *summary_path;
 	gboolean build_content;	/* do we try and parse/index the content, or not? */
 
-	GPtrArray *messages;	/* CamelMessageInfo's */
+	GPtrArray *messages; /* CamelMessageInfo's */
 	GHashTable *messages_uid; /* CamelMessageInfo's by uid */
 
 	struct _CamelFolder *folder; /* parent folder, for events */
@@ -244,6 +244,7 @@
 	GMappedFile *file;
 	unsigned char *filepos;
 	GMutex *dump_lock;
+	gboolean in_reload;
 };
 
 struct _CamelFolderSummaryClass {
@@ -257,7 +258,7 @@
 	CamelMessageInfo * (*message_info_new_from_header)(CamelFolderSummary *, struct _camel_header_raw *);
 	CamelMessageInfo * (*message_info_new_from_parser)(CamelFolderSummary *, CamelMimeParser *);
 	CamelMessageInfo * (*message_info_new_from_message)(CamelFolderSummary *, CamelMimeMessage *);
-	CamelMessageInfo * (*message_info_load)(CamelFolderSummary *);
+	CamelMessageInfo * (*message_info_load)(CamelFolderSummary *, gboolean *must_add);
  	int		   (*message_info_save)(CamelFolderSummary *, FILE *, CamelMessageInfo *);
 	int		   (*meta_message_info_save)(CamelFolderSummary *, FILE *, FILE *, CamelMessageInfo *);
 
Index: tinymail/tny-demoui-summary-view.c
===================================================================
--- tinymail/tny-demoui-summary-view.c	(revision 1112)
+++ tinymail/tny-demoui-summary-view.c	(working copy)
@@ -294,11 +294,12 @@
 							(GTK_TREE_MODEL_SORT (model));
 					} else mymodel = model;
 
+					tny_list_remove (TNY_LIST (mymodel), G_OBJECT (header));
 					folder = tny_header_get_folder (header);
 					tny_folder_remove_msg (folder, header);
 
-					tny_list_remove (TNY_LIST (mymodel), G_OBJECT (header));
-
+tny_folder_expunge (folder);
+					
 					g_object_unref (G_OBJECT (folder));
 				}