Good job trying to tackle it, but this is wrong.

This is not thread-safe and the Eo infrastructure should be. So 
essentially, this optimisation is not allowed. You could maybe do a 
cache per-thread, raster claims that __thread is actually quite alright 
nowadays with GCC, I'm not sure, but this, and the following patch 
should be reverted.

If you have anything to say to defend this approach, please do so by 
Monday, otherwise I'll revert both patches then.

--
Tom.

On 26/08/16 20:14, Cedric BAIL wrote:
> cedric pushed a commit to branch master.
>
> http://git.enlightenment.org/core/efl.git/commit/?id=93a706a947cd2d6ef80f8704f4e23737fea1258f
>
> commit 93a706a947cd2d6ef80f8704f4e23737fea1258f
> Author: Cedric BAIL <ced...@osg.samsung.com>
> Date:   Fri Aug 26 12:03:08 2016 -0700
>
>     eo: general speedup of all Eo related operation.
>
>     This change rely on the fact that we do fetch the same
>     object id over and over again. _efl_object_call_resolve got
>     15% faster, efl_data_scope_get 20%.
> ---
>  src/lib/eo/eo_base_class.c      |  3 +++
>  src/lib/eo/eo_ptr_indirection.x | 20 +++++++++++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/src/lib/eo/eo_base_class.c b/src/lib/eo/eo_base_class.c
> index 732d063..35c7a9c 100644
> --- a/src/lib/eo/eo_base_class.c
> +++ b/src/lib/eo/eo_base_class.c
> @@ -11,6 +11,9 @@
>
>  static int event_freeze_count = 0;
>
> +_Eo_Object *cached_object = NULL;
> +Eo_Id cached_id = 0;
> +
>  typedef struct _Eo_Callback_Description Eo_Callback_Description;
>
>  typedef struct
> diff --git a/src/lib/eo/eo_ptr_indirection.x b/src/lib/eo/eo_ptr_indirection.x
> index d89b1ec..cae59a4 100644
> --- a/src/lib/eo/eo_ptr_indirection.x
> +++ b/src/lib/eo/eo_ptr_indirection.x
> @@ -269,6 +269,9 @@ extern Generation_Counter _eo_generation_counter;
>  /* Macro used for readability */
>  #define TABLE_FROM_IDS _eo_ids_tables[mid_table_id][table_id]
>
> +extern _Eo_Object *cached_object;
> +extern Eo_Id cached_id;
> +
>  static inline _Eo_Object *
>  _eo_obj_pointer_get(const Eo_Id obj_id)
>  {
> @@ -289,6 +292,10 @@ _eo_obj_pointer_get(const Eo_Id obj_id)
>          DBG("obj_id is not a valid object id.");
>          return NULL;
>       }
> +   else if (obj_id == cached_id)
> +     {
> +        return cached_object;
> +     }
>
>     EO_DECOMPOSE_ID(obj_id, mid_table_id, table_id, entry_id, generation);
>
> @@ -297,7 +304,13 @@ _eo_obj_pointer_get(const Eo_Id obj_id)
>       {
>          entry = &(TABLE_FROM_IDS->entries[entry_id]);
>          if (entry && entry->active && (entry->generation == generation))
> -          return entry->ptr;
> +          {
> +             // Cache the result of that lookup
> +             cached_object = entry->ptr;
> +             cached_id = obj_id;
> +
> +             return entry->ptr;
> +          }
>       }
>
>     ERR("obj_id %p is not pointing to a valid object. Maybe it has already 
> been freed.",
> @@ -477,6 +490,11 @@ _eo_id_release(const Eo_Id obj_id)
>                    if (_current_table == table)
>                      _current_table = NULL;
>                 }
> +
> +             // In case an object is destroyed, wipe out the cache
> +             cached_id = 0;
> +             cached_object = NULL;
> +
>               return;
>            }
>       }
>


------------------------------------------------------------------------------
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to