Patch for reducing problems with getaddrinfo calls.



	Hi,

	As Philip suggested, I've been working last week in a separate branch.
There are four patches in that branch. First two where already sent to
the mailing list.

	This mail is for the approval of the third patch. It reduces the
problems with blockings for using addrinfo, as it caches the addrinfo
information obtained in camel imap folder. It also includes some locking
changes. It's in the 2478 commit. The changelog:

* libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c:
(imap_refresh_info): now we lock the folder also, before locking the
service, in order to respect better the lock order. This should avoid
some locks.

*libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.[ch]:
now we store a reference to the struct addrinfo used for connections.
When we are successful getting a valid addrinfo, it's stored and reused.
This way we prevent doing lots of calls to getaddrinfo and subsequent
name resolutions. As we're currently blocking while name resolution is
running, this should make the application more reponsive in general and
avoid flooding the connection. We also removed the method
(camel_imap_service_connect) as it's not used.

* libtinymail-camel/camel-lite/camel/camel-private.h:
added parenthesis to some lock macros.

Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(revisión: 2477)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.c	(revisión: 2478)
@@ -349,6 +349,11 @@
 	/* This frees current_folder, folders, authtypes, streams, and namespace. */
 	camel_service_disconnect((CamelService *)imap_store, TRUE, NULL);
 
+	if (imap_store->addrinfo) {
+		freeaddrinfo (imap_store->addrinfo);
+		imap_store->addrinfo = NULL;
+	}
+
 	if (imap_store->summary) {
 		camel_store_summary_save((CamelStoreSummary *)imap_store->summary);
 		camel_object_unref(imap_store->summary);
@@ -403,6 +408,8 @@
 	imap_store->last_folder = NULL;
 	imap_store->connected = FALSE;
 	imap_store->preauthed = FALSE;
+
+	imap_store->addrinfo = NULL;
 	((CamelStore *)imap_store)->flags |= CAMEL_STORE_SUBSCRIPTIONS;
 
 	imap_store->tag_prefix = imap_tag_prefix++;
@@ -1181,6 +1188,7 @@
 	int mode = -1, ret, i, must_tls = 0;
 	char *serv;
 	const char *port;
+	CamelImapStore *store = (CamelImapStore *) service;
 
 #ifndef G_OS_WIN32
 	const char *command;
@@ -1235,29 +1243,24 @@
 	hints.ai_socktype = SOCK_STREAM;
 	hints.ai_family = PF_UNSPEC;
 
-	ai = camel_getaddrinfo(service->url->host, serv, &hints, ex);
+	if (store->addrinfo == NULL)
+		store->addrinfo = camel_getaddrinfo(service->url->host, serv, &hints, ex);
 
-	if (ai == NULL && port != NULL && camel_exception_get_id(ex) != CAMEL_EXCEPTION_USER_CANCEL) 
+	if (store->addrinfo == NULL && port != NULL && 
+	    camel_exception_get_id(ex) != CAMEL_EXCEPTION_USER_CANCEL) 
 	{
 		camel_exception_clear (ex);
-		ai = camel_getaddrinfo(service->url->host, port, &hints, ex);
+		store->addrinfo = camel_getaddrinfo(service->url->host, port, &hints, ex);
 	}
 
-	if (ai == NULL)
+	if (store->addrinfo == NULL)
 		return FALSE;
 
-	ret = connect_to_server (service, ai, mode, must_tls, ex);
-	camel_freeaddrinfo (ai);
+	ret = connect_to_server (service, store->addrinfo, mode, must_tls, ex);
 
 	return ret;
 }
 
-gboolean 
-camel_imap_service_connect (CamelService *service, CamelException *ex)
-{
-	return connect_to_server_wrapper (service, ex);
-}
-
 extern CamelServiceAuthType camel_imap_password_authtype;
 
 static GList *
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.h
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.h	(revisión: 2477)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.h	(revisión: 2478)
@@ -165,6 +165,8 @@
 	gboolean idle_cont, in_idle;
 	guint idle_sleep, getsrv_sleep;
 	gboolean courier_crap;
+
+	struct addrinfo *addrinfo;
 };
 
 typedef struct {
@@ -183,7 +185,6 @@
 ssize_t camel_imap_store_readline_nb (CamelImapStore *store, char **dest, CamelException *ex);
 ssize_t camel_imap_store_readline (CamelImapStore *store, char **dest, CamelException *ex);
 ssize_t camel_imap_store_readline_idle (CamelImapStore *store, char **dest, CamelException *ex);
-gboolean camel_imap_service_connect (CamelService *service, CamelException *ex);
 
 gboolean camel_imap_store_restore_stream_buffer (CamelImapStore *store);
 
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c	(revisión: 2477)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c	(revisión: 2478)
@@ -813,6 +813,7 @@
 	 * Also, if this is the INBOX, some servers (cryus) wont tell
 	 * us with a NOOP of new messages, so force a reselect which
 	 * should do it.  */
+	CAMEL_FOLDER_REC_LOCK(folder, lock);
 	CAMEL_SERVICE_REC_LOCK (imap_store, connect_lock);
 
 	if (!camel_disco_store_check_online ((CamelDiscoStore*)imap_store, ex))
@@ -860,6 +861,7 @@
 	}
 done:
 	CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
+	CAMEL_FOLDER_REC_UNLOCK(folder, lock);
 
 	camel_folder_summary_save(folder->summary);
 
Index: libtinymail-camel/camel-lite/camel/camel-private.h
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-private.h	(revisión: 2477)
+++ libtinymail-camel/camel-lite/camel/camel-private.h	(revisión: 2478)
@@ -77,15 +77,15 @@
 };
 
 #define CAMEL_SERVICE_LOCK(f, l) \
-	(g_static_mutex_lock(&((CamelService *)f)->priv->l))
+	(g_static_mutex_lock(&((CamelService *)(f))->priv->l))
 #define CAMEL_SERVICE_UNLOCK(f, l) \
-	(g_static_mutex_unlock(&((CamelService *)f)->priv->l))
+	(g_static_mutex_unlock(&((CamelService *)(f))->priv->l))
 #define CAMEL_SERVICE_REC_LOCK(f, l) \
-	(g_static_rec_mutex_lock(&((CamelService *)f)->priv->l))
+	(g_static_rec_mutex_lock(&((CamelService *)(f))->priv->l))
 #define CAMEL_SERVICE_REC_UNLOCK(f, l) \
-	(g_static_rec_mutex_unlock(&((CamelService *)f)->priv->l))
+	(g_static_rec_mutex_unlock(&((CamelService *)(f))->priv->l))
 #define CAMEL_SERVICE_REC_TRYLOCK(f, l) \
-	(g_static_rec_mutex_trylock(&((CamelService *)f)->priv->l))
+	(g_static_rec_mutex_trylock(&((CamelService *)(f))->priv->l))
 
 
 struct _CamelSessionPrivate {
Index: ChangeLog
===================================================================
--- ChangeLog	(revisión: 2477)
+++ ChangeLog	(revisión: 2478)
@@ -1,3 +1,20 @@
+2007-07-19  Jose Dapena Paz  <jdapena igalia com>
+
+	* libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-folder.c:
+	(imap_refresh_info): now we lock the folder also, before locking the
+	service, in order to respect better the lock order. This should avoid
+	some locks.
+	* libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-store.[ch]:
+	now we store a reference to the struct addrinfo used for connections. When
+	we are successful getting a valid addrinfo, it's stored and reused. This
+	way we prevent doing lots of calls to getaddrinfo and subsequent name
+	resolutions. As we're currently blocking while name resolution is running,
+	this should make the application more reponsive in general and avoid
+	flooding the connection.
+	We also removed the method (camel_imap_service_connect) as it's not used.
+	* libtinymail-camel/camel-lite/camel/camel-private.h:
+	added parenthesis to some lock macros.
+
 2007-07-17  Jose Dapena Paz  <jdapena igalia com>
 
 	* libtinymail/tny-camel-mime-part.c:


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]