Page MenuHomePhabricator

Memory allocation issue in Evas_Object_Image due to Eina_Cow
Closed, ResolvedPublic

Description

below is the memory allocation data i got it from my sample application (which shows evas_object_image_is_opaque() doing 10% of memory allocation because of the Eina_Cow garbage collection).

We allocate Evas_Object_Image_State object in each frame if there is change in image state. because in evas_object_image_render_post() function we copy the pointer to the previous state using eina_cow_memcpy() which bumps the ref count to 2. so in the next frame if we try to write to that pointer again it will create a new copy and write to it.
and the process repeats again.

Maybe Eina_Cow is not suitable for this usecase as in worst case without it we will only do 2 memory allocation (1 for cur and 1 for prev state)

Fix: Either we could add a new function eina_cow_clone() which will only copy the content. or we can remove the Eina_Cow so that in worst case it will only make 2 allocation per object.

smohanty created this task.Oct 1 2019, 9:43 PM
smohanty added a subscriber: cedric.Oct 1 2019, 9:46 PM

@cedric ,
What do you think ?

cedric added a comment.Oct 2 2019, 7:05 AM

There copy on write mechanism prevent a lot of real memory copy and memory duplication not using it will have performance, memory and battery impact. In most case we do not write and avoid that unnecessary allocation/copy. Do you have a real life silicon that shows this being a problem? With this kind of optimization we rely on the fact that most application do the same thing. Optimizing for an outlier is maybe not necessary.

@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.

The use case is not uncommon when you want to render some pixel using different library and show it in efl app using evas_image.

cedric added a comment.Oct 3 2019, 7:58 PM

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.

Now I am wondering what is the problem really. Have you checked with massif visualizer if the allocation continue to happen during the life of the application or just once.

With cow, you might have an artefact in the profiler, as the function that modify the object first will be the one doing the allocating and copy, but it doesn't mean that this wouldn't have happen as anything that change the state of the object might actually trigger it.

Also have you checked that this structure are properly packed? Over time we sometime get mistake in that increase the size needlessly.

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.

evas_object_image_data_get() to get the buffer pointer.

evas_object_image_pixels_dirty_set(obj, EINA_TRUE) to force the image for a redraw

evas_object_image_pixels_get_callback_set() to get the pixel callback where I will set the same buffer pointer again using evas_object_image_data_set() api.

I don't know if its the correct way or there is some better way to achieve the same use case.

Now I am wondering what is the problem really. Have you checked with massif visualizer if the allocation continue to happen during the life of the application or just once.

This happens continuously as am running the animation in a loop .

With cow, you might have an artefact in the profiler, as the function that modify the object first will be the one doing the allocating and copy, but it doesn't mean that this wouldn't have happen as anything that change the state of > > the object might actually trigger it.

The memory allocation am seeing is not the COW allocation , it's just the book keeping for garbage collection as COW allocation is hidden inside the mempool i guess.

Also have you checked that this structure are properly packed? Over time we sometime get mistake in that increase the size needlessly.

Yet to check that.

Maybe will write a simple application to reproduce this usecase .

cedric added a comment.Oct 7 2019, 9:47 PM

Hum, there isn't much that can be done in this scenario. Basically of we're not using a mempool in conjunction with the cow, you have to provide two state so that evas can compare them and decide what to render. The cow memcpy try to reduce when you do it, but as soon as you need two state it will trigger the allocation and memcpy as it would otherwise do in every case.

One of the idea when I started the vg part of ever was to avoid any buffering during animation as you're going to loose performance there. Maybe that's something that would be good to investigate in your scenario. I have also noticed with expedite that the vg tests trend to over allocate memory like crazy even when rendering the same thing. Forgot which test, but with expedite and Valgrind you should be able to spot what I am talking about.

I think I have an idea to improve this specific case. Basically you should define the content hint on the image to EFL_GFX_IMAGE_CONTENT_HINT_DYNAMIC. In evas, we could now use that hint to decide when needed_gc boolean need to be set to false in the call to eina_cow_done. It would require updating the macro EINA_COW_IMAGE_STATE_WRITE_END to use eina_cow_done directly. This would avoid all the registering in the hash table and most of the allocation in your example.

cedric closed this task as Resolved.Oct 30 2019, 3:19 PM
cedric claimed this task.

I guess it is fixed now. Please reopen if it isn't.