[ Fixes bugs 20740, 16680, and god knows what else :) ] double the

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.

svn path=/trunk/; revision=15807

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));