[folks] eds: avoid blocking event processing



commit a480638774368497bce03b1db4297f9436725460
Author: Patrick Ohly <patrick ohly intel com>
Date:   Thu Feb 21 19:31:38 2013 +0100

    eds: avoid blocking event processing
    
    When there are many incoming D-Bus change notifications, processing of
    other D-Bus messages can be delayed considerably. This commit is a
    first step towards solving this by caching the change notifications
    and processing them with lower priority in a glib idle callback.
    
    Ordering of the changes relative to each other is preserved, so
    semantically this is the same as immediate processing.
    
    The worst-case scenario is when contacts get added one-by-one to EDS
    address books while folks is running, because then each change
    triggers one D-Bus signal. With 2000 contacts and four address books
    on a fast laptop, the process hosting folks became unresponsive for
    137s. The patch reduced that to 0.1s. The downside is an increase of
    total test runtime of 4%.
    
    In the scenario where data is already in EDS when folks starts, the
    patch reduced response time from 2.3s to 0.2s.
    
    See https://bugs.freedesktop.org/show_bug.cgi?id=60851#c6 for details.

 backends/eds/lib/edsf-persona-store.vala |  127 ++++++++++++++++++++++++++++++
 1 files changed, 127 insertions(+), 0 deletions(-)
---
diff --git a/backends/eds/lib/edsf-persona-store.vala b/backends/eds/lib/edsf-persona-store.vala
index 3d6d37b..dae25fa 100644
--- a/backends/eds/lib/edsf-persona-store.vala
+++ b/backends/eds/lib/edsf-persona-store.vala
@@ -2281,8 +2281,118 @@ public class Edsf.PersonaStore : Folks.PersonaStore
         }
     }
 
+  /*
+   * The EDS store receives change notifications as quickly as it
+   * can and then stores them for later processing in a glib idle
+   * callback.
+   *
+   * This avoids problems where many incoming D-Bus change notifications
+   * block processing of other incoming D-Bus messages. Delays of over a minute
+   * have been observed in worst-case (but not entirely unrealistic) scenarios
+   * (1000 contacts added one-by-one while folks is active). See
+   * https://bugzilla.gnome.org/show_bug.cgi?id=694385 (folks issue) and
+   * http://bugs.freedesktop.org/show_bug.cgi?id=60851 (SyncEvolution issue).
+   *
+   * Cannot store a GLib.SourceFunc directly
+   * in a LinkedList ("Delegates with target are not supported as generic type arguments",
+   * https://mail.gnome.org/archives/vala-list/2011-June/msg00002.html)
+   * and thus have to wrap it in a class. When dealing with delegates, we must be
+   * careful to transfer ownership, because they are not reference counted.
+   */
+  class IdleTask
+    {
+      public GLib.SourceFunc callback;
+    }
+
+
+  private Gee.Queue<IdleTask> _idle_tasks = new Gee.LinkedList<IdleTask> ();
+  private uint _idle_handle = 0;
+
+  /*
+   * Deal with some chunk of work encapsulated in the delegate later.
+   * As in any other SourceFunc, the callback may request to be called
+   * again by returning true. In contrast to Idle.add, _idle_queue
+   * ensures that only one task is processed per idle cycle.
+   */
+  private void _idle_queue (owned GLib.SourceFunc callback)
+    {
+      IdleTask task = new IdleTask ();
+      task.callback = (owned) callback;
+      this._idle_tasks.add (task);
+      /*
+       * Ensure that there is an active idle callback to process
+       * the task, otherwise register it. We cannot just
+       * queue each task separately, because then we might
+       * end up with multiple tasks being done at once
+       * when the process gets idle, instead of one
+       * task at a time.
+       */
+      if (this._idle_handle == 0)
+        {
+          this._idle_handle = Idle.add (this._idle_process);
+        }
+    }
+
+  private bool _idle_process ()
+    {
+      IdleTask? task = this._idle_tasks.peek ();
+      if (task != null)
+        {
+          if (task.callback ())
+            {
+               /* Task is not done yet, run it again later. */
+               return true;
+            }
+          this._idle_tasks.poll ();
+        }
+
+      /* Check for future work. */
+      task = this._idle_tasks.peek ();
+      if (task == null)
+        {
+           /*
+            * Remember that we need to re-register idle
+            * processing when _idle_queue is called again.
+            */
+           this._idle_handle = 0;
+           /* Done, will remove idle handler. */
+           return false;
+        }
+      else
+        {
+           /* Continue processing. */
+           return true;
+        }
+    }
+
+  private Gee.LinkedList<E.Contact> _copy_contacts (GLib.List<E.Contact> contacts)
+    {
+      var copy = new Gee.LinkedList<E.Contact> ();
+      foreach (E.Contact c in contacts)
+        {
+          copy.add (c);
+        }
+      return copy;
+    }
+
+  private Gee.LinkedList<string> _copy_contacts_ids (GLib.List<string> contacts_ids)
+    {
+      var copy = new Gee.LinkedList<string> ();
+      foreach (unowned string s in contacts_ids)
+        {
+          copy.add (s);
+        }
+      return copy;
+    }
+
   private void _contacts_added_cb (GLib.List<E.Contact> contacts)
     {
+      var copy = this._copy_contacts (contacts);
+      this._idle_queue (() => { return this._contacts_added_idle (copy); });
+    }
+
+  private bool _contacts_added_idle (Gee.List<E.Contact> contacts)
+    {
       HashSet<Persona> added_personas;
 
       /* If the persona store hasn't yet reached quiescence, queue up the
@@ -2318,10 +2428,19 @@ public class Edsf.PersonaStore : Folks.PersonaStore
         {
           this._emit_personas_changed (added_personas, null);
         }
+
+      /* Done. */
+      return false;
     }
 
   private void _contacts_changed_cb (GLib.List<E.Contact> contacts)
     {
+      var copy = this._copy_contacts (contacts);
+      this._idle_queue (() => { return this._contacts_changed_idle (copy); });
+    }
+
+  private bool _contacts_changed_idle (Gee.List<E.Contact> contacts)
+    {
       foreach (E.Contact c in contacts)
         {
           var iid = Edsf.Persona.build_iid_from_contact (this.id, c);
@@ -2331,10 +2450,17 @@ public class Edsf.PersonaStore : Folks.PersonaStore
               ((!) persona)._update (c);
             }
         }
+      return false;
     }
 
   private void _contacts_removed_cb (GLib.List<string> contacts_ids)
     {
+      var copy = this._copy_contacts_ids (contacts_ids);
+      this._idle_queue (() => { return this._contacts_removed_idle (copy); });
+    }
+
+  private bool _contacts_removed_idle (Gee.List<string> contacts_ids)
+    {
       var removed_personas = new HashSet<Persona> ();
 
       foreach (string contact_id in contacts_ids)
@@ -2359,6 +2485,7 @@ public class Edsf.PersonaStore : Folks.PersonaStore
          {
            this._emit_personas_changed (null, removed_personas);
          }
+      return false;
     }
 
   private void _contacts_complete_cb (Error err)


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