GNOME Bugzilla – Bug 399596
CamelFolderSummary uses an unneeded GHashTable
Last modified: 2013-09-14 16:49:39 UTC
Please describe the problem: And that hashtable consumes a lot memory (try sizeof (GHashNode) and multiply it with the total amount of E-mails that you manage with Evolution, that's in bytes). I will attach a patch that removes this, and replaces the functionality with a equally (if not better) performing helper function for finding the item. Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information:
Created attachment 80937 [details] [review] Patch that removes the hashtable Please review
Oh, you can probably make find_message_info_with_uid a little bit faster by putting G_LIKELY things at the right places. Avoid calling camel_folder_get_message_info for example in combination with camel_folder_get_uids, as it's much more efficient to simply use camel_folder_summary_array or a little bit more hackish but the fastest way to loop all CamelMessageInfo items: g_ptr_array_foreach (folder->summary->messages, my_func_thing, ptr); my_func_thing (CamelMessageInfo *mi) { ... } This is much faster than something like this: GPtrArray *uids = camel_folder_get_uids (folder); g_ptr_array_foreach (uids, my_func_thing, ptr); and my_func_thing (gchar *uid) { mi = camel_folder_get_message_info (folder, uid) } That's indeed because the hashtable (or after this patch the find_message_info_with_uid function) would be called.
And that is also a reason why before committing this patch, some code replacements might be needed. Or at least some performance testing.
why patch camel to remove a hash table lookup so that the code is forced to loop over the entire summary of message infos? I fail to see how this is an improvement.
I have a new way to get rid of the hashtable in camel-lite. What I did was adding two new API's that "forget" and "create" the hash table. I did this because looping over the GPtrArray would otherwise trash the caches. I also searched and studied all occurences of lookups by uid. It was already very much avoided all over the place, but I tried to avoid the lookup whenever possible too. And if not possible, I wrapped the looping code that needs a lot lookups in those two "create" and "forget" APIs. The reason ... the reason is because the hashtable consumes more-than-expected memory per item. Given that there can be a lot summary items (especially if keeping all folders open), this adds. For a feature that is only rarely needed (looking up using the uid) and can most of the times be perfectly avoided (most of the summary-using code simply loops over all items, which would be significantly slower if done by looking up each and every uid. Which therefore isn't/wasn't usually done that way of course). Anyway, if the Evolution project is not at all interested in this (as it's a memory reduction, and for desktops that might not be needed : I do understand that), I'm not planning to put effort in taking out the changes and trying to merge them with a normal Camel. But let me know if it does sound interesting. It is equally fast (some operations that now avoid the hashtable lookup are even really really faster now) and it does consume less memory. But oh well.