After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 399596 - CamelFolderSummary uses an unneeded GHashTable
CamelFolderSummary uses an unneeded GHashTable
Status: RESOLVED WONTFIX
Product: evolution-data-server
Classification: Platform
Component: Mailer
1.10.x (obsolete)
Other All
: Normal normal
: ---
Assigned To: evolution-mail-maintainers
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2007-01-22 23:13 UTC by Philip Van Hoof
Modified: 2013-09-14 16:49 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
Patch that removes the hashtable (4.65 KB, patch)
2007-01-22 23:14 UTC, Philip Van Hoof
rejected Details | Review

Description Philip Van Hoof 2007-01-22 23:13:44 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:
Comment 1 Philip Van Hoof 2007-01-22 23:14:48 UTC
Created attachment 80937 [details] [review]
Patch that removes the hashtable

Please review
Comment 2 Philip Van Hoof 2007-01-23 00:43:25 UTC
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.
Comment 3 Philip Van Hoof 2007-01-23 02:19:33 UTC
And that is also a reason why before committing this patch, some code replacements might be needed. Or at least some performance testing.
Comment 4 Jeffrey Stedfast 2007-01-23 18:04:40 UTC
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.
Comment 5 Philip Van Hoof 2007-01-30 13:18:36 UTC
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.