eo: event callback call optimization
authorMarcel Hollerbach <mail@marcel-hollerbach.de>
Wed, 5 Aug 2020 08:45:29 +0000 (10:45 +0200)
committerJongmin Lee <jm105.lee@samsung.com>
Thu, 6 Aug 2020 21:29:09 +0000 (06:29 +0900)
a little overfiew to how eos event emission is working:
- In eo there are 2 types of events restart events and normal events.
  Normals are *always* emitted from callback_max to 0
  Restarts are emitted from the previous emissions current idx to 0. The
  previous emission will stop then
- To keep track of the previous event emissions for restarts there was
  an eina inlist.
- To keep track of other things in eo, there was an additional eo event
  stack frame (newly inserted events etc. etc.)

This commit now uses this event stack frame for implementing the details
about the restart events. This has the advatage that every
efl_object_data contains one byte less, and the up to date keeping of
the id's in the restart infrastructure is not needed anymore, making the
whole event emission code less instructions and faster.

How this now works:
- Every emission has a event emission frame, when the emission starts,
  the frame is added, next points to the next older event emission. In
  this event stack frame we are storing the current idx, the description
  and a few other things that are not really relevant at this point.
- when a restart event is started, this event stack is searched from the
  current frame up to the next one featuring the same event description.
  The event frame stack is then remembered, if the event emission is
  done, the current idx (normally 0 or something higher when callback
  was stopped) is then copyied back to the earlier frame.
- Based on the nature of the event frame stacks, the idx is updated
  every iteration. And for the restart events, we only need to know the
  next, which removes the need of permanently updating the idx in the
  current stack.
- The event frame stack is not allocated on the heap, hence more things
  in there does not matter at all.

Reviewed-by: Cedric BAIL <cedric.bail@free.fr>
Differential Revision: https://phab.enlightenment.org/D12097

src/lib/eo/eo_base_class.c

index 565f480..8132bce 100644 (file)
@@ -38,6 +38,7 @@ struct _Efl_Event_Forwarder
 
 struct _Efl_Event_Callback_Frame
 {
+   const Efl_Event_Description    *desc;
    Efl_Event_Callback_Frame *next;
    unsigned int              idx;
    unsigned int              inserted_before;
@@ -65,8 +66,6 @@ struct _Efl_Object_Data
 
    Efl_Object_Extension      *ext;
 
-   Eina_Inlist               *current;
-
    Efl_Event_Callback_Frame  *event_frame;
    Eo_Callback_Description  **callbacks;
 #ifdef EFL64
@@ -2035,11 +2034,12 @@ _event_callback_call(Eo *obj_id, Efl_Object_Data *pd,
                      Eina_Bool legacy_compare)
 {
    Eo_Callback_Description **cb;
-   Eo_Current_Callback_Description *lookup, saved;
+   Efl_Event_Callback_Frame *restart_lookup = NULL; //a pointer to a frame, which is high up the stack, which we use to restore
    Efl_Event ev;
    unsigned int idx;
    Eina_Bool callback_already_stopped, ret;
    Efl_Event_Callback_Frame frame = {
+      .desc = desc,
       .next = NULL,
       .idx = 0,
       .inserted_before = 0,
@@ -2069,7 +2069,6 @@ _event_callback_call(Eo *obj_id, Efl_Object_Data *pd,
 
    EVENT_STACK_PUSH(pd, &frame);
 
-   lookup = NULL;
    callback_already_stopped = pd->callback_stopped;
    pd->callback_stopped = EINA_FALSE;
    ret = EINA_TRUE;
@@ -2109,9 +2108,6 @@ restart_back:
                            (event_freeze_count || pd->event_freeze_count))
                           continue;
 
-                       // Handle nested restart of walking list
-                       if (lookup) lookup->current = idx - 1;
-
                        it->func((void *) (*cb)->func_data, &ev);
                        /* Abort callback calling if the func says so. */
                        if (pd->callback_stopped)
@@ -2119,10 +2115,6 @@ restart_back:
                             ret = EINA_FALSE;
                             goto end;
                          }
-                       // We have actually walked this list during a nested call
-                       if (lookup &&
-                           lookup->current == 0)
-                         goto end;
                     }
                }
              else
@@ -2133,9 +2125,6 @@ restart_back:
                       (event_freeze_count || pd->event_freeze_count))
                     continue;
 
-                  // Handle nested restart of walking list
-                  if (lookup) lookup->current = idx - 1;
-
                   (*cb)->items.item.func((void *) (*cb)->func_data, &ev);
                   /* Abort callback calling if the func says so. */
                   if (pd->callback_stopped)
@@ -2143,23 +2132,19 @@ restart_back:
                        ret = EINA_FALSE;
                        goto end;
                     }
-                  // We have actually walked this list during a nested call
-                  if (lookup &&
-                      lookup->current == 0)
-                    goto end;
                }
           }
-        idx += frame.inserted_before;
+        /*
+         * copy back the idx that might have changed due to restarts, (theoretically only needed with restarts, condition made everything slower)
+         * additionally adjust to event subscriptions that have been added in a event callback
+         */
+        idx = frame.idx + frame.inserted_before;
         frame.inserted_before = 0;
      }
 
 end:
    // Handling restarting list walking complete exit.
-   if (lookup) lookup->current = 0;
-   if (lookup == &saved)
-     {
-        pd->current = eina_inlist_remove(pd->current, EINA_INLIST_GET(lookup));
-     }
+   if (restart_lookup) restart_lookup->idx = 0;
 
    EVENT_STACK_POP(pd);
 
@@ -2169,26 +2154,22 @@ end:
 
    return ret;
 restart:
-   EINA_INLIST_FOREACH(pd->current, lookup)
+   // Search for the next frame that has the same event description
+   for (restart_lookup = frame.next; restart_lookup; restart_lookup = restart_lookup->next)
      {
-        if (lookup->desc == desc) break;
+        if (restart_lookup->desc == desc) break;
      }
 
-   // This is the first event to trigger it, so register it here
-   if (!lookup)
-     {
-        // This following trick get us a zero allocation list
-        saved.desc = desc;
-        saved.current = 0;
-        lookup = &saved;
-        // Ideally there will most of the time be only one item in this list
-        // But just to speed up things, prepend so we find it fast at the end
-        // of this function
-        pd->current = eina_inlist_prepend(pd->current, EINA_INLIST_GET(lookup));
-     }
+   // Ensure that the idx is the next from the previous run, minimum number 0
+   if (restart_lookup) {
+     idx = restart_lookup->idx - 1;
+   } else {
+     idx = 0;
+   }
+   // If this is 0, then we are restarting
+   if (!idx)
+     idx = pd->callbacks_count;
 
-   if (!lookup->current) lookup->current = pd->callbacks_count;
-   idx = lookup->current;
    goto restart_back;
 }