From 81ca5ee36d115134643d875d325d36e270af85ef Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie@collabora.co.uk>
Date: Mon, 7 Mar 2011 17:48:53 +0000
Subject: [PATCH] Remove the per-connection link cache

With fd.o#34393 fixed, retaking the lock to cache unused links
significantly adds to locking overhead (-18% throughput in a synthetic
benchmark on an ARM device). The cache is also unlimited in size, and
probably contributes to memory growth and fragmentation by not being
under the system malloc's control.

Fixing fd.o #34393, but also dropping this cache entirely, turns out to
lead to a 5% increase in throughput on the same synthetic benchmark.
---
 dbus/dbus-connection.c |   59 +++++++++---------------------------------------
 1 files changed, 11 insertions(+), 48 deletions(-)

diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 241c8f1..79c8fc9 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -286,9 +286,6 @@ struct DBusConnection
 
   DBusDispatchStatus last_dispatch_status; /**< The last dispatch status we reported to the application. */
 
-  DBusList *link_cache; /**< A cache of linked list links to prevent contention
-                         *   for the global linked list mempool lock
-                         */
   DBusObjectTree *objects; /**< Object path handlers registered with this connection */
 
   char *server_guid; /**< GUID of server if we are in shared_connections, #NULL if server GUID is unknown or connection is private */
@@ -393,27 +390,15 @@ _dbus_connection_unlock (DBusConnection *connection)
   RELEASING_LOCK_CHECK (connection);
   _dbus_mutex_unlock (connection->mutex);
 
-  for (iter = _dbus_list_get_first_link (&expired_messages);
+  for (iter = _dbus_list_pop_first_link (&expired_messages);
       iter != NULL;
-      iter = _dbus_list_get_next_link (&expired_messages, iter))
+      iter = _dbus_list_pop_first_link (&expired_messages))
     {
       DBusMessage *message = iter->data;
 
       dbus_message_unref (message);
-      iter->data = NULL;
+      _dbus_list_free_link (iter);
     }
-
-  /* Take the lock back for a moment, to copy the links into the link
-   * cache. FIXME: with this extra cost, is it still advantageous to have
-   * the link cache? */
-  _dbus_mutex_lock (connection->mutex);
-
-  for (iter = _dbus_list_pop_first_link (&expired_messages);
-      iter != NULL;
-      iter = _dbus_list_pop_first_link (&expired_messages))
-    _dbus_list_prepend_link (&connection->link_cache, iter);
-
-  _dbus_mutex_unlock (connection->mutex);
 }
 
 /**
@@ -673,9 +658,7 @@ _dbus_connection_message_sent_unlocked (DBusConnection *connection,
    * outgoing limit, there isn't one */
   _dbus_message_remove_counter (message, connection->outgoing_counter,
                                 &link);
-
-  /* Save this link in the link cache also */
-  _dbus_list_prepend_link (&connection->link_cache, link);
+  _dbus_list_free_link (link);
 
   /* The message will actually be unreffed when we unlock */
 }
@@ -1967,31 +1950,13 @@ _dbus_connection_preallocate_send_unlocked (DBusConnection *connection)
   if (preallocated == NULL)
     return NULL;
 
-  if (connection->link_cache != NULL)
-    {
-      preallocated->queue_link =
-        _dbus_list_pop_first_link (&connection->link_cache);
-      preallocated->queue_link->data = NULL;
-    }
-  else
-    {
-      preallocated->queue_link = _dbus_list_alloc_link (NULL);
-      if (preallocated->queue_link == NULL)
-        goto failed_0;
-    }
-  
-  if (connection->link_cache != NULL)
-    {
-      preallocated->counter_link =
-        _dbus_list_pop_first_link (&connection->link_cache);
-      preallocated->counter_link->data = connection->outgoing_counter;
-    }
-  else
-    {
-      preallocated->counter_link = _dbus_list_alloc_link (connection->outgoing_counter);
-      if (preallocated->counter_link == NULL)
-        goto failed_1;
-    }
+  preallocated->queue_link = _dbus_list_alloc_link (NULL);
+  if (preallocated->queue_link == NULL)
+    goto failed_0;
+
+  preallocated->counter_link = _dbus_list_alloc_link (connection->outgoing_counter);
+  if (preallocated->counter_link == NULL)
+    goto failed_1;
 
   _dbus_counter_ref (preallocated->counter_link->data);
 
@@ -2783,8 +2748,6 @@ _dbus_connection_last_unref (DBusConnection *connection)
       _dbus_list_free_link (connection->disconnect_message_link);
     }
 
-  _dbus_list_clear (&connection->link_cache);
-  
   _dbus_condvar_free_at_location (&connection->dispatch_cond);
   _dbus_condvar_free_at_location (&connection->io_path_cond);
 
-- 
1.7.4.1