eo: improve speed of walking callback array by sorting them during creation.
authorCedric BAIL <cedric@osg.samsung.com>
Wed, 31 Aug 2016 21:12:02 +0000 (14:12 -0700)
committerCedric Bail <cedric@osg.samsung.com>
Fri, 2 Sep 2016 17:19:45 +0000 (10:19 -0700)
This improve speed of processing events in genlist scrolling benchmark by 30%
inside the efl_object_event_callback_call code. Not a really big deal as it
goes from 0.9% to 0.6% of the total time spend. Welcome to micro optimization.

src/lib/eo/Eo.h
src/lib/eo/efl_object.eo
src/lib/eo/eo.c
src/lib/eo/eo_base_class.c

index 549cb0e..f241ca8 100644 (file)
@@ -1070,23 +1070,33 @@ EAPI const Efl_Event_Description *efl_object_legacy_only_event_description_get(c
 #define EFL_CALLBACK_PRIORITY_AFTER 100
 
 /**
+ * Helper for sorting callbacks array. Automatically used by
+ * @ref EFL_CALLBACKS_ARRAY_DEFINE
+ */
+EAPI int efl_callbacks_cmp(const Efl_Callback_Array_Item *a, const Efl_Callback_Array_Item *b);
+
+/**
  * Helper for creating global callback arrays.
  * The problem is on windows where you can't declare a static array with
  * external symbols in it, because the addresses are only known at runtime.
+ * This also open up the possibility to automatically sort them for better
+ * performance.
  */
-#define EFL_CALLBACKS_ARRAY_DEFINE(Name, ...)                            \
-  static Efl_Callback_Array_Item *                                       \
+#define EFL_CALLBACKS_ARRAY_DEFINE(Name, ...)                           \
+  static Efl_Callback_Array_Item *                                      \
   Name(void)                                                            \
   {                                                                     \
      static Efl_Callback_Array_Item internal[sizeof ((Efl_Callback_Array_Item[]) { __VA_ARGS__ }) / \
                                             sizeof (Efl_Callback_Array_Item) + \
-                                            1] = { { 0, 0 } };          \
+                                             1] = { { 0, 0 } };         \
      if (internal[0].desc == NULL)                                      \
        {                                                                \
           memcpy(internal,                                              \
                  ((Efl_Callback_Array_Item[]) { __VA_ARGS__, { NULL, NULL } }), \
-                 sizeof (Efl_Callback_Array_Item) +                      \
-                 sizeof ((Efl_Callback_Array_Item[]) { __VA_ARGS__ }));  \
+                 sizeof (Efl_Callback_Array_Item) +                     \
+                 sizeof ((Efl_Callback_Array_Item[]) { __VA_ARGS__ })); \
+          qsort(internal, sizeof (internal) / sizeof (internal[0]) - 1, sizeof (internal[0]), \
+                (void*) efl_callbacks_cmp);                              \
        }                                                                \
      return internal;                                                   \
   }
@@ -1108,11 +1118,15 @@ EAPI const Efl_Event_Description *efl_object_legacy_only_event_description_get(c
 
 /**
  * @def efl_event_callback_array_add(obj, desc, cb, data)
- * Add a callback array for an event.
+ * Add an array of callbacks for an event.
+ *
  * @param[in] array an #Efl_Callback_Array_Item of events to listen to.
  * @param[in] data additional data to pass to the callback.
  *
- * callbacks of the same priority are called in reverse order of creation.
+ * Callbacks of the same priority are called in reverse order of creation.
+ * The array should have been created by @ref EFL_CALLBACKS_ARRAY_DEFINE. If
+ * that wasn't the case, be careful of portability issue and make sure that
+ * it is properly sorted with @ref efl_callbacks_cmp.
  *
  * @see efl_event_callback_array_priority_add()
  */
index 8c45863..4daca98 100644 (file)
@@ -341,7 +341,9 @@ abstract Efl.Object ()
          }
       }
       event_callback_array_priority_add {
-         [[Add a callback array for an event with a specific priority.
+         [[Add an array of callbacks created by \@ref EFL_CALLBACKS_ARRAY_DEFINE for an event
+          with a specific priority. The array need to be sorted with \@ref efl_callbacks_cmp
+          if you are not using the \@ref EFL_CALLBACKS_ARRAY_DEFINE macro.
 
            callbacks of the same priority are called in reverse order of
            creation.
index 9286aac..de05690 100644 (file)
@@ -1927,3 +1927,9 @@ efl_manual_free(Eo *obj_id)
 
    return EINA_TRUE;
 }
+
+EAPI int
+efl_callbacks_cmp(const Efl_Callback_Array_Item *a, const Efl_Callback_Array_Item *b)
+{
+   return (const unsigned char *) a->desc - (const unsigned char *) b->desc;
+}
index 0bb5307..8e44255 100644 (file)
@@ -1065,13 +1065,31 @@ _efl_object_event_callback_array_priority_add(Eo *obj, Efl_Object_Data *pd,
                           const void *user_data)
 {
    Eo_Callback_Description *cb = _eo_callback_new();
+#ifdef EO_DEBUG
+   const Efl_Callback_Array_Item *it;
+   const Efl_Callback_Array_Item *prev;
+#endif
 
    if (!cb || !array)
      {
-        ERR("Tried adding array of callbacks with invalid values: cb: %p array: %p\n", cb, array);
+        ERR("Tried adding array of callbacks with invalid values: cb: %p array: %p.", cb, array);
         _eo_callback_free(cb);
         return EINA_FALSE;
      }
+
+#ifdef EO_DEBUG
+   prev = array;
+   for (it = prev + 1; prev->func && it->func; it++, prev++)
+     {
+        if (efl_callbacks_cmp(prev, it) > 0)
+          {
+             ERR("Trying to insert a non sorted array callbacks (%p).", array);
+             _eo_callback_free(cb);
+             return EINA_FALSE;
+          }
+     }
+#endif
+
    cb->func_data = (void *) user_data;
    cb->priority = priority;
    cb->items.item_array = array;
@@ -1182,6 +1200,10 @@ _event_callback_call(Eo *obj_id, Efl_Object_Data *pd,
 
                   for (it = cb->items.item_array; it->func; it++)
                     {
+                       // Array callbacks are sorted, break if we are getting to high.
+                       if (!legacy_compare &&
+                           ((const unsigned char *) desc - (const unsigned char *) it->desc) < 0)
+                         break;
                        if (!_cb_desc_match(it->desc, desc, legacy_compare))
                           continue;
                        if (!it->desc->unfreezable &&