updated

In D11985#228694, @bu5hm4n wrote:

Thank you. I checked upstream that we do not assigned parent_obj *ever* by hand. And we don't. Can you ensure the same in tizen ?

updated review comment.

have also updated this patch for last review comment. please check if this is what you intended.

updated review comment

updated review comment.

thanks

Thanks for the patch . Yup this fix the the leak.

@ali ,
I didn't know evas_object_textblock_style_set() only keeps the reference to the raw pointer.

updated

Logs:

updated

properly deallocate the custom command node.

fixed one more leak

the item class is not freed for some test cases . added elm_genlist_item_class_free() for those cases . and incase if we are keeping a ref then unref in the clear_cb().

@raster ,
Could you please review this when you have time . I don't know if this is the correct fix but the memory leak shows up in address sanitizer.

in _eo_key_generic_direct_del() we remove the node from the ext->generic_data . Because of call_free is false we never call _eo_generic_data_node_free() on the node hence the leak.

In D11137#219880, @stefan_schmidt wrote:

From my side I am waiting for feedback from @smohanty

As written before I am fine with the approach as is. If we go the route of disable all leak checks or only suppress the known ones I have no strong opinion.

In D11784#224601, @raster wrote:

Ummm... the object could be in the scene graph for a render cycle, never be shown or rendered and it'll then hang around in the scene graph just like before until some render cycles flush it out, because it will then be flagged as being in the graph and behave just like before.

and with this patch same use case it takes around 80ms mostly due to the Hash Search of Efl_Provider which is same for both cases. ( anyway hash iterator is another story )

In D11488#219055, @bu5hm4n wrote:

Would you mind adding tests that ensure the behavior that is needed ? otherwise this will break again at some point i guess :/

In D11488#219006, @ali.alzyod wrote:

@smohanty @woohyun
Is the purpose of this patch speed ? or is there another issue?

indeed solves the issue.

The change to be opaque should really happen once. Only if you switch back and forth for each frame will you continuously allocate memory. Basically you fallback to the case where you do not have any cow.

As I am running an animation (using lottie to render to the buffer ) so each frame I update the buffer (part of evas_image) with below Apis.

@cedric ,
Regarding my use case , I am updating image data in every frame by calling evas_object_image_data_get() and evas_object_image_data_set() api . I don't know how Image object can figure out if the content is opaque or not if user has set some pixel data (as it will be expensive to go through each pixel to figure it out) . So maybe we can modify the is_opaque() to return false when user has set some pixel data.

@cedric ,
What do you think ?

Thanks for the detailed explanation after reading it got little bit more understanding of the current issue with edje memory consumption issue.

If you guys knows the history of this code, please let me know your opinion.

In D9739#182258, @raster wrote:

hey @smohanty - keep things from getting personal. we DO have guidelines on how to behave: https://www.enlightenment.org/contact . You're not joking here and it's definitely crossing a line.

In D9739#182086, @tasn wrote:

What am I seeing in these logs? How were they collected?

Taken a Snapshot of Vtable array (after flattening of function array. ) by just printing the content of chain array (0 - not filled , 1 means filled)
Before Patch and after patch the slots that are filled are same. attached the log

Note for future review comment.

In D9794#182033, @tasn wrote:

I don't get this change. Readability is subjective and this doesn't read any more readable to me, and it's 20 lines longer (not that it's the only parameter, but still).
It's also hard to follow when you changed a few unrelated things in one commit (or is it just phab being bad?).

Test Done :

 efl eo test suite passed.
elementary_test example works.

updated the patch so that it will be a better match to the mental model.

In D9739#181840, @tasn wrote:

Could you please fix the formatting in your comment? I'm not sure what you're trying to say there because of the wrong quote and etc. Regarding the MRO: I know it's not the actual MRO, delete the I from there then, it's still the same.