GNOME Bugzilla – Bug 318880
EContact does linear searches for field information
Last modified: 2013-09-10 13:42:05 UTC
EContact does linear searches for over field_info for field information every time a field is accessed. This is pretty inefficient when the key for the table is an integer... Attaching a patch which sorts the table into the same order as the enumeration, so instead of doing a linear search (which is O(n)) it can lookup the data directly (O(1)).
Created attachment 53479 [details] [review] Change from linear searches to indexing
Created attachment 53574 [details] [review] Better patch
confirming and adding keyword
Good one. But we can not take it now, as it breaks ABI :(
No it doesn't, that table is internal and not exposed: the point is that the internal table is changed to match the public listing and thus allowing an optimisation.
sush: poke!
sushma: poke. can we PLEASE get this in before API freeze takes place? sush, you already commented that this patch is good, please take the minutes to get this in.
Ping, can someone review this? It's doesn't change ABI but does have a good performance effect, basically it does this: - for (i = 0; i < G_N_ELEMENTS (field_info); i ++) { - if (field_id == field_info[i].field_id) - return field_info[i].vcard_field_name; - } + return field_info[field_id].vcard_field_name;
Bah, this doesn't apply to current CVS. I'll re-generate it.
Created attachment 69239 [details] [review] Revised patch against HEAD
reviewed , Ross commit this please. Just check once again if there are any other places where this new logic (changing from for loop to direct indexing) can be applied. Also make sure nothing has been left out from field_info, i have checked this but please double check.
ross: ping :-)
Created attachment 70378 [details] [review] Revised New patch incorporating the GaduGadu change. Is it too late to commit?
Devashish ?
Looks good, commit it please.
Added ChangeLog and committed. Thanks for the patch.