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 312690 - Optimise book views
Optimise book views
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: Contacts
1.2.x (obsolete)
Other Linux
: Normal major
: ---
Assigned To: Sushma Rai
Evolution QA team
Depends on:
Blocks: 271113 318800 327516
 
 
Reported: 2005-08-05 15:40 UTC by Ross Burton
Modified: 2013-09-10 13:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Pass vCards rather than EContacts (3.34 KB, patch)
2005-08-05 15:40 UTC, Ross Burton
none Details | Review
Improved patch (3.92 KB, patch)
2005-08-09 10:08 UTC, Ross Burton
accepted-commit_now Details | Review

Description Ross Burton 2005-08-05 15:40:11 UTC
This is what happens when a book view is executed in EDS:

[get vCard from database]
contact = e_contact_new_from_vcard(vcard)
e_book_view_notify_update (contact)
vcard = e_contact_to_string_vcard30
[now send string over IPC]

Looks harmless, but if you profile this 68% of the execution time is spend
measuring strings, 99.9% of the calls coming from e_contact_to_string_vcard30.

This is basically because e_data_book_view_notify_update() takes a EContact, but
the database has just retrieved a vCard and the book view needs to send a vCard
over the wire.

Attached is a prototype patch to e-data-book-view.[ch] and
e-book-backend-file.c, which has a non-negligable effect on performance (test
creates 300 book views).

Before:
real    0m12.179s
user    0m1.741s
sys     0m0.046s

After:
real    0m3.737s
user    0m1.740s
sys     0m0.052s

Now, this is a prototype patch as this is probably going to be a right pain for
some backends to support, where vCards are not the native transport format. 
Maybe e-data-book-view needs notify_update(EContact*) and notify_update(char*)?
Comment 1 Ross Burton 2005-08-05 15:40:50 UTC
Created attachment 50284 [details] [review]
Pass vCards rather than EContacts
Comment 2 Ross Burton 2005-08-05 15:53:56 UTC
Whoops, that patch is against the dbus backend.  However, the notify_update()
function is identical so the patch might still apply.
Comment 3 Sushma Rai 2005-08-06 07:45:48 UTC
looks good... but this would break api freeze.
Comment 4 Ross Burton 2005-08-09 10:08:28 UTC
Created attachment 50447 [details] [review]
Improved patch

Attaching an improved patch which doesn't leak and doesn't break existing ABI.
Comment 5 André Klapper 2005-10-14 14:49:39 UTC
adding perf keyword
Comment 6 André Klapper 2005-10-25 15:33:37 UTC
novell folks: please review this and get this in asap, it blocks several bugs.
raising severity; setting target milestone to 1.5. :-/

PS: yeah, everybody hates "asap", me too. ;-)
Comment 7 Veerapuram Varadhan 2006-01-10 09:48:47 UTC
Assigning it to sushma...

sushma: Any comments?
Comment 8 Sushma Rai 2006-01-10 10:11:51 UTC
We are testing this patch.
Comment 9 Devashish Sharma 2006-01-16 13:46:54 UTC
Tested looks good.Can be committed.
Comment 10 Ross Burton 2006-01-16 16:36:22 UTC
Committed to HEAD.

Thanks!