On Thu, 8 Sep 2016 08:36:30 -0700 Cedric BAIL <cedric.b...@free.fr> said:
> On Sep 8, 2016 3:14 AM, "Tom Hacohen" <t...@stosb.com> wrote: > > > > tasn pushed a commit to branch master. > > > > > http://git.enlightenment.org/core/efl.git/commit/?id=6b60560773f59398427f12c21945d31b852db72a > > > > commit 6b60560773f59398427f12c21945d31b852db72a > > Author: Tom Hacohen <t...@stosb.com> > > Date: Thu Sep 8 11:14:32 2016 +0100 > > > > Eo class creation: Simplify code using recursive locks. > > > > Now that we have recursive locks, the class creation code can be much > simpler. > > All the code there was essentially our own implementation of > recursive locks, > > or rather a special case of those. > > > > This is no longer needed. > > I have started to look into implementing a light and fast spinlock. This > make me realize that recursive lock are a pain. There memory usage grow > linearly with the number of locker in a thread obviously, but the book > keeping itself is a serious impact on performance. So if you are going to > use recursive lock, avoid it in the hot path ! Not sure if it apply here, > but better share this with everyone. well i've used a single global recursive lock around ALL shared objects. call a method on any eo objects in the shared table and it will lock all eo shared object access until that method returns. it's recursive or otherwise you could never call a callback or a method on yourself within such an object. it's necessary. well otherwise you have to unlock on every possible exit point from that func and that means it has to be manual and that would duck. so technically if an eo obj is allocated in the shared domain now... it is actually able to be seen by all threads AND is threadsafe. BUT accessing it comes with a higher cost. instead of 2.5% avg eoid resolv cost it goes up to maybe 5-7%. there is ALSO this big fat recursive mutex around the method call. yup. sharing objects is not cheap. do it to very special objects you really MUST share. it's already limited like no parent or child or key->obj property on an obj can cross a domain boundary so that means you cannot just make a button or a window or a timer shared as their parents will be a local domain object, so you break this rule. you have to "deal with it" yourself. so this is a cost, but it should not be paid commonly. rarely. in fact right now we have no objects that can actually become shared. well eo base... that's it. you'd have to inherit your own classes from there. this is why i added the recursive mutex so this could be implemented. it's not easy to make a shared object and will never be that common. > > --- > > src/lib/eo/Eo.h | 24 +++++------------------- > > src/lib/eo/eo.c | 14 +++++++------- > > 2 files changed, 12 insertions(+), 26 deletions(-) > > > > diff --git a/src/lib/eo/Eo.h b/src/lib/eo/Eo.h > > index a530604..f1aabd5 100644 > > --- a/src/lib/eo/Eo.h > > +++ b/src/lib/eo/Eo.h > > @@ -128,7 +128,7 @@ typedef Eo Efl_Object; > > * Don't touch it if you don't know what you are doing. > > * @internal > > */ > > -EAPI extern Eina_Spinlock _efl_class_creation_lock; > > +EAPI extern Eina_Lock _efl_class_creation_lock; > > > > /** > > * @var _efl_object_init_generation > > @@ -320,39 +320,25 @@ const Efl_Class * \ > > class_get_func_name(void) \ > > { \ > > const Efl_Class *_tmp_parent_class; \ > > - static volatile unsigned char lk_init = 0; \ > > - static Eina_Spinlock _my_lock; \ > > static const Efl_Class * volatile _my_class = NULL; \ > > static unsigned int _my_init_generation = 1; \ > > if (EINA_UNLIKELY(_efl_object_init_generation != > _my_init_generation)) \ > > { \ > > _my_class = NULL; /* It's freed in efl_object_shutdown(). */ \ > > - lk_init = 0; \ > > } \ > > if (EINA_LIKELY(!!_my_class)) return _my_class; \ > > \ > > - eina_spinlock_take(&_efl_class_creation_lock); \ > > - if (!lk_init) \ > > - eina_spinlock_new(&_my_lock); \ > > - if (lk_init < 2) eina_spinlock_take(&_my_lock); \ > > - if (!lk_init) \ > > - lk_init = 1; \ > > - else \ > > + eina_lock_take(&_efl_class_creation_lock); \ > > + if (!!_my_class) \ > > { \ > > - if (lk_init < 2) eina_spinlock_release(&_my_lock); \ > > - eina_spinlock_release(&_efl_class_creation_lock); \ > > + eina_lock_release(&_efl_class_creation_lock); \ > > return _my_class; \ > > } \ > > - eina_spinlock_release(&_efl_class_creation_lock); \ > > _tmp_parent_class = parent_class; \ > > _my_class = efl_class_new(class_desc, _tmp_parent_class, > __VA_ARGS__); \ > > _my_init_generation = _efl_object_init_generation; \ > > - eina_spinlock_release(&_my_lock); \ > > + eina_lock_release(&_efl_class_creation_lock); \ > > \ > > - eina_spinlock_take(&_efl_class_creation_lock); \ > > - eina_spinlock_free(&_my_lock); \ > > - lk_init = 2; \ > > - eina_spinlock_release(&_efl_class_creation_lock); \ > > return _my_class; \ > > } > > > > diff --git a/src/lib/eo/eo.c b/src/lib/eo/eo.c > > index 7931753..bfc5be5 100644 > > --- a/src/lib/eo/eo.c > > +++ b/src/lib/eo/eo.c > > @@ -19,7 +19,7 @@ > > #define EFL_OBJECT_OP_IDS_FIRST 1 > > > > /* Used inside the class_get functions of classes, see #EFL_DEFINE_CLASS > */ > > -EAPI Eina_Spinlock _efl_class_creation_lock; > > +EAPI Eina_Lock _efl_class_creation_lock; > > EAPI unsigned int _efl_object_init_generation = 1; > > int _eo_log_dom = -1; > > > > @@ -1342,11 +1342,11 @@ efl_class_new(const Efl_Class_Description *desc, > const Efl_Class *parent_id, ... > > { > > Eo_Id new_id; > > > > - eina_spinlock_take(&_efl_class_creation_lock); > > + eina_lock_take(&_efl_class_creation_lock); > > new_id = (_eo_classes_last_id + 1) | MASK_CLASS_TAG; > > _eo_classes_expand(); > > _eo_classes[_UNMASK_ID(new_id) - 1] = klass; > > - eina_spinlock_release(&_efl_class_creation_lock); > > + eina_lock_release(&_efl_class_creation_lock); > > > > klass->header.id = new_id; > > } > > @@ -1837,7 +1837,7 @@ efl_object_init(void) > > return EINA_FALSE; > > } > > > > - if (!eina_spinlock_new(&_efl_class_creation_lock)) > > + if (!eina_lock_recursive_new(&_efl_class_creation_lock)) > > { > > EINA_LOG_ERR("Could not init lock."); > > return EINA_FALSE; > > @@ -1927,15 +1927,15 @@ efl_object_shutdown(void) > > eo_class_free(*cls_itr); > > } > > > > - eina_spinlock_take(&_efl_class_creation_lock); > > + eina_lock_take(&_efl_class_creation_lock); > > _eo_classes_release(); > > - eina_spinlock_release(&_efl_class_creation_lock); > > + eina_lock_release(&_efl_class_creation_lock); > > > > eina_hash_free(_ops_storage); > > > > eina_spinlock_free(&_super_class_lock); > > eina_spinlock_free(&_ops_storage_lock); > > - eina_spinlock_free(&_efl_class_creation_lock); > > + eina_lock_free(&_efl_class_creation_lock); > > > > _eo_free_ids_tables(_eo_table_data_get()); > > eina_tls_free(_eo_table_data); > > > > -- > > > > > ------------------------------------------------------------------------------ > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ras...@rasterman.com ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
