Re: [Evolution-hackers] vfolder performance problems

[Not Zed <notzed ximian com>]

On Mon, 2005-04-18 at 17:24 -0400, Lee Revell wrote:
> On Sun, 2005-04-17 at 01:31 -0400, Lee Revell wrote:
> > On Sat, 2005-04-16 at 02:58 -0400, Lee Revell wrote:
> > > I have found the bug that has been causing severe performance problems
> > > with vfolders, "Unread Mail" in particular.
> > > 
> > 
> > This patch fixes the problem by only searching the message list if the
> > user has typed something in the search bar.  I'm not sure it's the right
> > fix, but it drastically improves performance for me.
> > 
> 
> It was pointed out to me that my patch did not conform to the patch
> guidelines.  Here is a clean patch.
> 
> I don't see the need to assign copyright as it's basically three one
> line fixes.  I would just like to be credited in the changelog for
> fixing this bug.

Well in that case you should supply the changelog!  Changelogs are also
part of the patch guidelines anyway.

So are "no c++ style comments" (or 'c89'), which your code includes.

> The "hidejunk" fix is a temporary hack until someone implements it in an
> efficient way.  I think it would be sufficient to add a "hasjunk" flag
> to the folder summary so we don't have to check the junk flag on every
> single message in every folder with every message list update.  Or make
> "hidejunk" configurable and default to off.

Well, of course, until the full functionality is maintained, the patch
can't be applied anyway.

I don't think the hasjunk idea is a very scalable one either.  Nor does
it really help - we have to test every flag of every message every time
we build the message-list tree model anyway.  So I'm wondering whether
your patch is just hiding another problem (the lock contention one), and
not really making the big performance difference it appears to be.

> Index: evolution/mail/em-folder-browser.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-folder-browser.c,v
> retrieving revision 1.59
> diff -u -r1.59 em-folder-browser.c
> --- evolution/mail/em-folder-browser.c	28 Feb 2005 04:31:50 -0000	1.59
> +++ evolution/mail/em-folder-browser.c	18 Apr 2005 21:21:13 -0000
> @@ -435,13 +435,14 @@
>  emfb_search_search_activated(ESearchBar *esb, EMFolderBrowser *emfb)
>  {
>  	EMFolderView *emfv = (EMFolderView *) emfb;
> -	char *search_word, *search_state;
> +	char *search_word, *search_state, *text;
>  	
>  	if (emfv->list == NULL || emfv->folder == NULL)
>  		return;
>  	
>  	g_object_get (esb, "query", &search_word, NULL);
> -	message_list_set_search(emfb->view.list, search_word);
> +	g_object_get (esb, "text", &text, NULL);
> +	message_list_set_search(emfb->view.list, text == NULL || text[0] == '\0' ? NULL : search_word);
>  	g_free (search_word);
>  	
>  	g_object_get (esb, "state", &search_state, NULL);
> @@ -937,7 +938,7 @@
>  	   before the folder is open and need to override the
>  	   defaults */
>  	if (folder) {
> -		char *sstate;
> +		char *sstate, *text;
>  		int state;
>  		GConfClient *gconf = mail_config_get_gconf_client();
>  
> @@ -974,7 +975,8 @@
>  		
>  		/* set the query manually, so we dont pop up advanced or saved search stuff */
>  		g_object_get(emfb->search, "query", &sstate, NULL);
> -		message_list_set_search(emfb->view.list, sstate);
> +		g_object_get (emfb->search, "text", &text, NULL);
> +		message_list_set_search(emfb->view.list, text == NULL || text[0] == '\0' ? NULL : sstate);

Don't use '\0', just use 0.  '\0' is just a long-winded way to say
'octal character 0', it isn't a more readable, or more portable way to
say 'nul'.

>  		g_free(sstate);
>  
>  		if ((sstate = camel_object_meta_get (folder, "evolution:selected_uid"))) {
> Index: evolution/mail/message-list.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/message-list.c,v
> retrieving revision 1.414
> diff -u -r1.414 message-list.c
> --- evolution/mail/message-list.c	1 Apr 2005 00:17:24 -0000	1.414
> +++ evolution/mail/message-list.c	18 Apr 2005 21:21:17 -0000
> @@ -2867,7 +2867,8 @@
>  		gconf = mail_config_get_gconf_client ();
>  		hide_deleted = !gconf_client_get_bool (gconf, "/apps/evolution/mail/display/show_deleted", NULL);
>  		message_list->hidedeleted = hide_deleted && !(folder->folder_flags & CAMEL_FOLDER_IS_TRASH);
> -		message_list->hidejunk = junk_folder && !(folder->folder_flags & CAMEL_FOLDER_IS_JUNK) && !(folder->folder_flags & CAMEL_FOLDER_IS_TRASH);
> +		//message_list->hidejunk = junk_folder && !(folder->folder_flags & CAMEL_FOLDER_IS_JUNK) && !(folder->folder_flags & CAMEL_FOLDER_IS_TRASH);
> +		message_list->hidejunk = 0;
>  		
>  		load_hide_state (message_list);
>  		if (message_list->frozen == 0)
> 
> 
> 
> 
> _______________________________________________
> evolution-hackers maillist  -  evolution-hackers lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-hackers