Re: [E-devel] [EGIT] [core/efl] master 02/05: eo: rework vtable allocation scheme
On Mon, 23 Mar 2020 17:40:56 +0900 Hermet Park said: oh... i was running it for days on several machines with no problems... i guess need to back it out. > This patch occurs memory corruption, vector crashes :( > Here is a sample if you'd like to see it. > https://phab.enlightenment.org/F3858944 > > On Mon, Mar 23, 2020 at 4:05 AM Marcel Hollerbach > wrote: > > > raster pushed a commit to branch master. > > > > > > http://git.enlightenment.org/core/efl.git/commit/?id=3bd16a46f1098f5533723208feae8abafae4e8ab > > > > commit 3bd16a46f1098f5533723208feae8abafae4e8ab > > Author: Marcel Hollerbach > > Date: Fri Mar 20 11:32:38 2020 + > > > > eo: rework vtable allocation scheme > > > > Summary: > > with this commit a new way of allocating vtables arrived. > > The old mechnism was to allocate a table big enough to carry *all* > > functions at once, in order to not allocate that much memory for > > functions that are not implemented on a specific klass, dichchains have > > been used, which can be seens as a 2D matrix, where columns are only > > allocated if min 1 entry needs to be written, this may have been a good > > way to allocate back in the day when all this with eo started, however, > > it showed to not pay off. > > > > With this new way, we allocate a array of arrays. the first lvl array > > is > > carrying enough slots, that *all* up to the time defined > > interfaces/classes/abstracts/mixins can be implemented. The second lvl > > array then has exactly the size of the defined APIs. The second lvl > > array is obviously only allocated if needed. > > > > When comparing the two methods, i messured two things, the usage based > > on memory allocation for vtable-way-1 and vtable-way-2. Additionally, i > > checked the overall memory usage of elementary_test using pmap. The > > first messurement is a little bit more exact. The second messurement is > > more biased, but captures the whole picture. > > > > Memory allocation tracking: > >vtable-way-1 - vtable-way-2 = 74680 Byte > > > > Pmap memory tracking: > >vtable-way1 - vtable-way-2 = 217088 Byte > > > > The second messurement shows a bigger impact, likely because this is > > also showing off all the sideeffects that we are taking place due to > > fewer allocations. > > > > Depends on D11524 > > > > Reviewers: zmike, tasn, stefan_schmidt, woohyun, cedric, raster > > > > Subscribers: #reviewers, #committers > > > > Tags: #efl > > > > Differential Revision: https://phab.enlightenment.org/D11535 > > --- > > src/lib/eo/eo.c | 495 > > +++ > > src/lib/eo/eo_private.h | 32 +-- > > src/tests/eo/suite/eo_test_general.c | 1 - > > 3 files changed, 285 insertions(+), 243 deletions(-) > > > > diff --git a/src/lib/eo/eo.c b/src/lib/eo/eo.c > > index 8ca94bf8fd..b28cb178c3 100644 > > --- a/src/lib/eo/eo.c > > +++ b/src/lib/eo/eo.c > > @@ -90,7 +90,6 @@ static _Efl_Class **_eo_classes = NULL; > > static Eo_Id _eo_classes_last_id = 0; > > static Eo_Id _eo_classes_alloc = 0; > > static int _efl_object_init_count = 0; > > -static Efl_Object_Op _eo_ops_last_id = 0; > > static Eina_Hash *_ops_storage = NULL; > > static Eina_Spinlock _ops_storage_lock; > > > > @@ -104,7 +103,6 @@ static void _eo_condtor_reset(_Eo_Object *obj); > > static inline void *_efl_data_scope_get(const _Eo_Object *obj, const > > _Efl_Class *klass); > > static inline void *_efl_data_xref_internal(const char *file, int line, > > _Eo_Object *obj, const _Efl_Class *klass, const _Eo_Object *ref_obj); > > static inline void _efl_data_xunref_internal(_Eo_Object *obj, void *data, > > const _Eo_Object *ref_obj); > > -static void _vtable_init(Eo_Vtable *vtable, size_t size); > > > > static inline Efl_Object_Op _efl_object_api_op_id_get_internal(const void > > *api_func); > > > > @@ -120,96 +118,175 @@ static inline Efl_Object_Op > > _efl_object_api_op_id_get_internal(const void *api_f > >(_eo_classes[_UNMASK_ID(id) - 1]) : NULL); \ > >}) > > > > -static inline void > > -_vtable_chain2_unref(Dich_Chain2 *chain) > > +#define EFL_OBJECT_OP_CLASS_PART(op) op >> 16 > > +#define EFL_OBJECT_OP_FUNC_PART(op) op & 0x > > +#define EFL_OBJECT_OP_CREATE_OP_ID(class_id, func_id) ((unsigned > > short)class_id)<<16|((unsigned short)func_id&0x) > > + > > +static const _Efl_Class * > > +_eo_op_class_get(Efl_Object_Op op) > > { > > - if (--(chain->refcount) == 0) > > - { > > -free(chain); > > - } > > + short class_id = EFL_OBJECT_OP_CLASS_PART(op); > > + return _eo_classes[class_id]; > > } > > > > -static inline void > > -_vtable_chain_alloc(Dich_Chain1 *chain1) > > +/** > > + * This inits the vtable wit hthe current size of allocated tables > > + */ > > +static void > > +_vtable_init(Eo_Vtable *vtable) > > { > > - chain1->chain2 = calloc(1,
Re: [E-devel] [EGIT] [core/efl] master 02/05: eo: rework vtable allocation scheme
This patch occurs memory corruption, vector crashes :( Here is a sample if you'd like to see it. https://phab.enlightenment.org/F3858944 On Mon, Mar 23, 2020 at 4:05 AM Marcel Hollerbach wrote: > raster pushed a commit to branch master. > > > http://git.enlightenment.org/core/efl.git/commit/?id=3bd16a46f1098f5533723208feae8abafae4e8ab > > commit 3bd16a46f1098f5533723208feae8abafae4e8ab > Author: Marcel Hollerbach > Date: Fri Mar 20 11:32:38 2020 + > > eo: rework vtable allocation scheme > > Summary: > with this commit a new way of allocating vtables arrived. > The old mechnism was to allocate a table big enough to carry *all* > functions at once, in order to not allocate that much memory for > functions that are not implemented on a specific klass, dichchains have > been used, which can be seens as a 2D matrix, where columns are only > allocated if min 1 entry needs to be written, this may have been a good > way to allocate back in the day when all this with eo started, however, > it showed to not pay off. > > With this new way, we allocate a array of arrays. the first lvl array > is > carrying enough slots, that *all* up to the time defined > interfaces/classes/abstracts/mixins can be implemented. The second lvl > array then has exactly the size of the defined APIs. The second lvl > array is obviously only allocated if needed. > > When comparing the two methods, i messured two things, the usage based > on memory allocation for vtable-way-1 and vtable-way-2. Additionally, i > checked the overall memory usage of elementary_test using pmap. The > first messurement is a little bit more exact. The second messurement is > more biased, but captures the whole picture. > > Memory allocation tracking: >vtable-way-1 - vtable-way-2 = 74680 Byte > > Pmap memory tracking: >vtable-way1 - vtable-way-2 = 217088 Byte > > The second messurement shows a bigger impact, likely because this is > also showing off all the sideeffects that we are taking place due to > fewer allocations. > > Depends on D11524 > > Reviewers: zmike, tasn, stefan_schmidt, woohyun, cedric, raster > > Subscribers: #reviewers, #committers > > Tags: #efl > > Differential Revision: https://phab.enlightenment.org/D11535 > --- > src/lib/eo/eo.c | 495 > +++ > src/lib/eo/eo_private.h | 32 +-- > src/tests/eo/suite/eo_test_general.c | 1 - > 3 files changed, 285 insertions(+), 243 deletions(-) > > diff --git a/src/lib/eo/eo.c b/src/lib/eo/eo.c > index 8ca94bf8fd..b28cb178c3 100644 > --- a/src/lib/eo/eo.c > +++ b/src/lib/eo/eo.c > @@ -90,7 +90,6 @@ static _Efl_Class **_eo_classes = NULL; > static Eo_Id _eo_classes_last_id = 0; > static Eo_Id _eo_classes_alloc = 0; > static int _efl_object_init_count = 0; > -static Efl_Object_Op _eo_ops_last_id = 0; > static Eina_Hash *_ops_storage = NULL; > static Eina_Spinlock _ops_storage_lock; > > @@ -104,7 +103,6 @@ static void _eo_condtor_reset(_Eo_Object *obj); > static inline void *_efl_data_scope_get(const _Eo_Object *obj, const > _Efl_Class *klass); > static inline void *_efl_data_xref_internal(const char *file, int line, > _Eo_Object *obj, const _Efl_Class *klass, const _Eo_Object *ref_obj); > static inline void _efl_data_xunref_internal(_Eo_Object *obj, void *data, > const _Eo_Object *ref_obj); > -static void _vtable_init(Eo_Vtable *vtable, size_t size); > > static inline Efl_Object_Op _efl_object_api_op_id_get_internal(const void > *api_func); > > @@ -120,96 +118,175 @@ static inline Efl_Object_Op > _efl_object_api_op_id_get_internal(const void *api_f >(_eo_classes[_UNMASK_ID(id) - 1]) : NULL); \ >}) > > -static inline void > -_vtable_chain2_unref(Dich_Chain2 *chain) > +#define EFL_OBJECT_OP_CLASS_PART(op) op >> 16 > +#define EFL_OBJECT_OP_FUNC_PART(op) op & 0x > +#define EFL_OBJECT_OP_CREATE_OP_ID(class_id, func_id) ((unsigned > short)class_id)<<16|((unsigned short)func_id&0x) > + > +static const _Efl_Class * > +_eo_op_class_get(Efl_Object_Op op) > { > - if (--(chain->refcount) == 0) > - { > -free(chain); > - } > + short class_id = EFL_OBJECT_OP_CLASS_PART(op); > + return _eo_classes[class_id]; > } > > -static inline void > -_vtable_chain_alloc(Dich_Chain1 *chain1) > +/** > + * This inits the vtable wit hthe current size of allocated tables > + */ > +static void > +_vtable_init(Eo_Vtable *vtable) > { > - chain1->chain2 = calloc(1, sizeof(*(chain1->chain2))); > - chain1->chain2->refcount = 1; > + //we assume here that _eo_classes_last_id was called before > + vtable->size = _eo_classes_last_id; > + vtable->chain = calloc(vtable->size, sizeof(Eo_Vtable_Node)); > } > > -static inline void _vtable_chain_write_prepare(Dich_Chain1 *dst); > - > -static inline void > -_vtable_chain_merge(Dich_Chain1 *dst, const Dich_Chain1 *src) > +/** > + *