diff options
-rw-r--r-- | addressbook/ChangeLog | 42 | ||||
-rw-r--r-- | addressbook/backend/ebook/e-book-view-listener.c | 26 | ||||
-rw-r--r-- | addressbook/backend/pas/pas-backend-ldap.c | 172 | ||||
-rw-r--r-- | addressbook/gui/widgets/e-addressbook-model.c | 15 |
4 files changed, 206 insertions, 49 deletions
diff --git a/addressbook/ChangeLog b/addressbook/ChangeLog index a46e0b2448..e144519c4e 100644 --- a/addressbook/ChangeLog +++ b/addressbook/ChangeLog @@ -1,3 +1,45 @@ +2002-02-22 Chris Toshok <toshok@ximian.com> + + [ Fixes bugs 20740, 16680, and god knows what else :) ] + * gui/widgets/e-addressbook-model.c (create_card): double the + allocated size every time we need more space instead of using a + fixed size increment. this helps huge queries. Also, remove the + gtk_object_get of "file_as", as it was dead code. + (book_view_loaded): handle errors here (by popping up a dialog). + + * backend/pas/pas-backend-ldap.c (view_destroy): search_idle -> + search_timeout. + (build_card_from_entry): comment out some spew, and unref ecard + when we're done to plug a memory leak. + (send_pending_adds): send along to the client all the cards we've + been saving up. + (poll_ldap): use a timeout for ldap_result to keep the backend + from blocking (and it turns out keep the frontend from hanging + waiting on a ref to complete) on large db's with few matches. + + Also, add some fairly smart, self-tuning aggregating of cards. + Keep track of the number of cards we've sent the last time through + as well as this time, and estimate the number we want to aggregate + the next time based on them (we average them at the moment), + subject to maximum/minimum number of cards. also, we have a + maximum aggregation time, after which we force a flush if there + are pending cards and recalculate our target pending number. + there's a minimum wait time to possibly keep outselves from + spamming the ui, although it's 0 at the moment. + + Lastly, make sure to only notify the GUI of status messages when + we need to. this results in a *huge* savings. + (ldap_search_handler): initialize all the pending card stuff, and + use a timeout instead of an idle function for poll_ldap. + + * backend/ebook/e-book-view-listener.c + (e_book_view_listener_queue_response): performance optimization + for large adds. If we're a CardAddedEvent and there's an existing + CardAddedEvent at the end of the queue, just concat the lists of + cards together. This is to keep the gui from falling further and + further behind the ldap backend, which is merrily spewing updates + at the gui. + 2002-02-21 Ettore Perazzoli <ettore@ximian.com> * gui/component/Makefile.am: Define $(iconsdir). diff --git a/addressbook/backend/ebook/e-book-view-listener.c b/addressbook/backend/ebook/e-book-view-listener.c index 40e467ef93..9981375631 100644 --- a/addressbook/backend/ebook/e-book-view-listener.c +++ b/addressbook/backend/ebook/e-book-view-listener.c @@ -72,8 +72,30 @@ e_book_view_listener_queue_response (EBookViewListener *listener, g_free (response); return; } - - listener->priv->response_queue = g_list_append (listener->priv->response_queue, response); + + /* a slight optimization for huge ldap queries. if there's an + existing Add response on the end of the queue, and we're an + Add response, we just glom the two lists of cards + together */ + if (response->op == CardAddedEvent) { + GList *last = g_list_last (listener->priv->response_queue); + EBookViewListenerResponse *last_resp = NULL; + + if (last) last_resp = last->data; + + if (last_resp && last_resp->op == CardAddedEvent ) { + response->cards = g_list_concat (last_resp->cards, response->cards); + g_free (response); + /* there should already be a timeout since the + queue isn't empty, so we'll just return + here */ + return; + } + else + listener->priv->response_queue = g_list_append (last, response); + } + else + listener->priv->response_queue = g_list_append (listener->priv->response_queue, response); if (listener->priv->timeout_id == 0) { diff --git a/addressbook/backend/pas/pas-backend-ldap.c b/addressbook/backend/pas/pas-backend-ldap.c index 60f2103c79..e99b51c699 100644 --- a/addressbook/backend/pas/pas-backend-ldap.c +++ b/addressbook/backend/pas/pas-backend-ldap.c @@ -36,6 +36,8 @@ #include "ldap_schema.h" #endif +#include <sys/time.h> + #include <e-util/e-sexp.h> #include <ebook/e-card-simple.h> @@ -49,6 +51,22 @@ #define LDAP_MAX_SEARCH_RESPONSES 100 +/* interval for our poll_ldap timeout */ +#define LDAP_POLL_INTERVAL 20 + +/* timeout for ldap_result */ +#define LDAP_RESULT_TIMEOUT_MILLIS 10 + +/* smart grouping stuff */ +#define GROUPING_INITIAL_SIZE 1 +#define GROUPING_MAXIMUM_SIZE 200 + +/* the next two are in milliseconds */ +#define GROUPING_MINIMUM_WAIT 0 /* we never send updates faster than this, to avoid totally spamming the UI */ +#define GROUPING_MAXIMUM_WAIT 250 /* we always send updates (if there are pending cards) when we hit this */ + +#define TV_TO_MILLIS(timeval) ((timeval).tv_sec * 1000 + (timeval).tv_usec / 1000) + /* the objectClasses we need */ #define TOP "top" #define PERSON "person" @@ -104,9 +122,21 @@ struct _PASBackendLDAPBookView { PASBackendLDAPPrivate *blpriv; gchar *search; PASBackendCardSExp *card_sexp; - int search_idle; + int search_timeout; int search_msgid; LDAPOp *search_op; + + /* grouping stuff */ + + GList *pending_adds; /* the cards we're sending */ + int num_pending_adds; /* the number waiting to be sent */ + int target_pending_adds; /* the cutoff that forces a flush to the client, if it happens before the timeout */ + int num_sent_this_time; /* the number of cards we sent to the client before the most recent timeout */ + int num_sent_last_time; /* the number of cards we sent to the client before the previous timeout */ + glong grouping_time_start; + + /* used by poll_ldap to only send the status messages once */ + gboolean notified_receiving_results; }; typedef gboolean (*LDAPOpHandler)(PASBackend *backend, LDAPOp *op); @@ -256,11 +286,11 @@ view_destroy(GtkObject *object, gpointer data) for (list = bl->priv->book_views; list; list = g_list_next(list)) { PASBackendLDAPBookView *view = list->data; if (view->book_view == PAS_BOOK_VIEW(object)) { - if (view->search_idle != 0) { + if (view->search_timeout != 0) { /* we have a search running on the - ldap connection. remove the idle + ldap connection. remove the timeout handler and anbandon the msg id */ - g_source_remove(view->search_idle); + g_source_remove(view->search_timeout); if (view->search_msgid != -1) ldap_abandon (bl->priv->ldap, view->search_msgid); @@ -2162,7 +2192,7 @@ build_card_from_entry (LDAP *ldap, LDAPMessage *e, GList **existing_objectclasse char *attr; BerElement *ber = NULL; - g_print ("build_card_from_entry, dn = %s\n", dn); + // g_print ("build_card_from_entry, dn = %s\n", dn); e_card_simple_set_id (card, dn); for (attr = ldap_first_attribute (ldap, e, &ber); attr; @@ -2215,9 +2245,21 @@ build_card_from_entry (LDAP *ldap, LDAPMessage *e, GList **existing_objectclasse e_card_simple_sync_card (card); + gtk_object_unref (GTK_OBJECT (ecard)); + return card; } +static void +send_pending_adds (PASBackendLDAPBookView *view) +{ + view->num_sent_this_time += view->num_pending_adds; + pas_book_view_notify_add (view->book_view, view->pending_adds); + g_list_foreach (view->pending_adds, (GFunc)g_free, NULL); + view->pending_adds = NULL; + view->num_pending_adds = 0; +} + static gboolean poll_ldap (LDAPSearchOp *op) { @@ -2226,54 +2268,89 @@ poll_ldap (LDAPSearchOp *op) LDAP *ldap = bl->priv->ldap; int rc; LDAPMessage *res, *e; - GList *cards = NULL; static int received = 0; + GTimeVal cur_time; + glong cur_millis; + struct timeval timeout; - pas_book_view_notify_status_message (view->book_view, _("Receiving LDAP search results...")); - - rc = ldap_result (ldap, view->search_msgid, 0, NULL, &res); - - if (rc == -1 && received == 0) { - pas_book_view_notify_status_message (view->book_view, _("Restarting search.")); - /* connection went down and we never got any. */ - bl->priv->connected = FALSE; + timeout.tv_sec = 0; + timeout.tv_usec = LDAP_RESULT_TIMEOUT_MILLIS * 1000; - /* this will reopen the connection */ - ldap_op_restart ((LDAPOp*)op); - return FALSE; + if (!view->notified_receiving_results) { + view->notified_receiving_results = TRUE; + pas_book_view_notify_status_message (view->book_view, _("Receiving LDAP search results...")); } - if (rc != LDAP_RES_SEARCH_ENTRY) { - view->search_idle = 0; - pas_book_view_notify_complete (view->book_view); - ldap_op_finished ((LDAPOp*)op); - received = 0; - return FALSE; - } + rc = ldap_result (ldap, view->search_msgid, 0, &timeout, &res); + if (rc != 0) {/* rc == 0 means timeout exceeded */ + if (rc == -1 && received == 0) { + pas_book_view_notify_status_message (view->book_view, _("Restarting search.")); + /* connection went down and we never got any. */ + bl->priv->connected = FALSE; - received = 1; - - e = ldap_first_entry(ldap, res); + /* this will reopen the connection */ + ldap_op_restart ((LDAPOp*)op); + return FALSE; + } - while (NULL != e) { - ECardSimple *card = build_card_from_entry (ldap, e, NULL); + if (rc != LDAP_RES_SEARCH_ENTRY) { + view->search_timeout = 0; + if (view->num_pending_adds) + send_pending_adds (view); + pas_book_view_notify_complete (view->book_view); + ldap_op_finished ((LDAPOp*)op); + received = 0; + return FALSE; + } - cards = g_list_append (cards, e_card_simple_get_vcard_assume_utf8 (card)); + received = 1; - gtk_object_unref (GTK_OBJECT(card)); + e = ldap_first_entry(ldap, res); - e = ldap_next_entry(ldap, e); - } + while (NULL != e) { + ECardSimple *card = build_card_from_entry (ldap, e, NULL); - if (cards) { - pas_book_view_notify_add (view->book_view, cards); - - g_list_foreach (cards, (GFunc)g_free, NULL); - g_list_free (cards); - cards = NULL; + view->pending_adds = g_list_append (view->pending_adds, + e_card_simple_get_vcard_assume_utf8 (card)); + view->num_pending_adds ++; + + gtk_object_unref (GTK_OBJECT(card)); + + e = ldap_next_entry(ldap, e); + } + + ldap_msgfree(res); } - ldap_msgfree(res); + g_get_current_time (&cur_time); + cur_millis = TV_TO_MILLIS (cur_time); + + if (cur_millis - view->grouping_time_start > GROUPING_MINIMUM_WAIT) { + + if (view->num_pending_adds >= view->target_pending_adds) + send_pending_adds (view); + + if (cur_millis - view->grouping_time_start > GROUPING_MAXIMUM_WAIT) { + GTimeVal new_start; + + if (view->num_pending_adds) + send_pending_adds (view); + view->target_pending_adds = MIN (GROUPING_MAXIMUM_SIZE, + (view->num_sent_this_time + view->num_sent_last_time) / 2); + view->target_pending_adds = MAX (view->target_pending_adds, 1); + +#ifdef PERFORMANCE_SPEW + printf ("num sent this time %d, last time %d, target pending adds set to %d\n", + view->num_sent_this_time, + view->num_sent_last_time, + view->target_pending_adds); +#endif + g_get_current_time (&new_start); + view->grouping_time_start = TV_TO_MILLIS (new_start); + view->num_sent_last_time = view->num_sent_this_time; + view->num_sent_this_time = 0; + } + } return TRUE; } @@ -2295,6 +2372,17 @@ ldap_search_handler (PASBackend *backend, LDAPOp *op) PASBackendLDAPBookView *view = search_op->view; LDAP *ldap = bl->priv->ldap; int ldap_err; + GTimeVal search_start; + + view->pending_adds = NULL; + view->num_pending_adds = 0; + view->target_pending_adds = GROUPING_INITIAL_SIZE; + + g_get_current_time (&search_start); + view->grouping_time_start = TV_TO_MILLIS (search_start); + view->num_sent_last_time = 0; + view->num_sent_this_time = 0; + view->notified_receiving_results = FALSE; ldap_err = ldap_search_ext (ldap, bl->priv->ldap_rootdn, bl->priv->ldap_scope, @@ -2315,7 +2403,9 @@ ldap_search_handler (PASBackend *backend, LDAPOp *op) return TRUE; /* act synchronous in this case */ } else { - view->search_idle = g_idle_add((GSourceFunc)poll_ldap, search_op); + view->search_timeout = g_timeout_add (LDAP_POLL_INTERVAL, + (GSourceFunc) poll_ldap, + search_op); } /* we're async */ diff --git a/addressbook/gui/widgets/e-addressbook-model.c b/addressbook/gui/widgets/e-addressbook-model.c index 901ec22a0b..0f63c48936 100644 --- a/addressbook/gui/widgets/e-addressbook-model.c +++ b/addressbook/gui/widgets/e-addressbook-model.c @@ -13,6 +13,7 @@ #include <gnome-xml/parser.h> #include <gnome-xml/xmlmemory.h> #include <gnome.h> +#include <gal/widgets/e-gui-utils.h> #define PARENT_TYPE gtk_object_get_type() GtkObjectClass *parent_class; @@ -159,18 +160,13 @@ create_card(EBookView *book_view, if (model->data_count + length > model->allocated_count) { while (model->data_count + length > model->allocated_count) - model->allocated_count += 256; + model->allocated_count = model->allocated_count * 2 + 1; model->data = g_renew(ECard *, model->data, model->allocated_count); } for ( ; cards; cards = cards->next) { - char *file_as; - model->data[model->data_count++] = cards->data; gtk_object_ref (cards->data); - gtk_object_get (GTK_OBJECT (cards->data), - "file_as", &file_as, - NULL); } gtk_signal_emit (GTK_OBJECT (model), @@ -366,7 +362,14 @@ static void book_view_loaded (EBook *book, EBookStatus status, EBookView *book_view, gpointer closure) { EAddressbookModel *model = closure; + remove_book_view(model); + + if (status != E_BOOK_STATUS_SUCCESS) { + e_addressbook_error_dialog (_("Error getting book view"), status); + return; + } + model->book_view = book_view; if (model->book_view) gtk_object_ref(GTK_OBJECT(model->book_view)); |