GNOME Bugzilla – Bug 585425
GIF loader uses about 280M of VmRSS for a 200Kb GIF
Last modified: 2014-10-22 14:23:51 UTC
Here we have an interesting GIF file that apparently has its Logical Screen Size at 45x73. Regretfully the people who made the image decided that the individual frames would each be sized 86x13364. Reading the specification if GIF this appears to be possible. This back-trace however will happen for each and every such frame. Which for the attached image means that it happens ~ 194 times. gdk_pixbuf__gif_image_load_increment -gif_main_loop ---case GIF_GET_LZW -----gif_get_lzw -------gdk_pixbuf_gif_anim_frame_composite ---------gdk_pixbuf_copy at io-gif-animation.c:469 -----------gdk_pixbuf_new_subpixbuf The width and height of these pixbufs are each 86x133364, multiplied by three bytes for the RGB data that's about 3MB per frame. Looking at it with valgrind's massif tool, and by looking at /proc/$PID/status I noticed that during image loading tc3.gif (with a GdkPixbufLoader, the file size is is 275Kb in size) VmRSS grows to about 280MB. Apparently some of the GdkPixbuf instances for compositing the frames together are indeed destroyed, else the memory usage would grow to said 3MB x 194 frames, or about 600MB. This ain't a leak because once loaded, the VmRss usage goes back to more sane values. My kernel took some time to recover from the abuse on VmRSS, but eventually it prevailed. I'm a bit reluctant to call this normal ... In any case, it makes it nearly impossible to use such relatively small GIF files on certain mobile devices. Using +200MB VmRSS on such devices is unacceptable memory consumption. Regretfully there's no way to detect that this will happen for the file in question. That's because the "size-prepared" signal will emit with the logical screen size, not with the individual frames' sizes. (image will be attached shortly after committing this description)
Created attachment 136328 [details] Image that causes this behaviour
Created attachment 204830 [details] [review] Reduce memory usage For me looks as reduced usage from 892MiB to 231MiB (570MiB in peak).
Created attachment 204831 [details] Test program i looks memory usage in gnome system monitor. I usage ubuntu 11.10(gdk-pixbuf-2.24.0)
Created attachment 204832 [details] [review] Same patch but for current git version
Created attachment 204836 [details] [review] Fix memory usage for 2.24.0
Created attachment 204837 [details] [review] Fix memory usage for git master(same as for 2.24.0)
After patches looks as used 23.2MiB. I added code for clean cached pixbuf (composited) when cache not used. Now deleted cache if exist next frame with composited pixbuf. And when created many new frame use old frame memory(cache not created) and cache created only for last frame in list.
Please someone make CR.
Created attachment 250747 [details] [review] Minimize memory usage for gif animation. Have cleaned up white spaces in previous patches, checked changes on current master. As test program can be used eog, so old test program made absolute also.
Bug 698770 is duplication of this bug.
*** Bug 698770 has been marked as a duplicate of this bug. ***
*** Bug 703794 has been marked as a duplicate of this bug. ***
Review of attachment 250747 [details] [review]: There's many things I don't really understand in this patch. Why do you go through the list of frames after the previous and the current one in gdk_pixbuf_gif_anim_iter_clean_previous()? Wouldn't poke at the prev and current be enough? Your changes in frame_composite() can probably use a function, instead of repeating the same changes 3 times. A more detailed commit message would also be useful. ::: gdk-pixbuf/io-gif-animation.c @@ +213,3 @@ +{ + GList *tmp = initial; + while (tmp != NULL) { We prefer for() loops, as in gdk_pixbuf_gif_anim_finalize(). @@ +222,3 @@ + if (tmp == NULL) + return; + GdkPixbufFrame *prev_frame = tmp->data; Declare new variables at the top of the block, not in the middle of it. @@ +297,2 @@ tmp = tmp->next; + if (tmp) { No need for braces for one-line conditionals.
Created attachment 275192 [details] [review] Updated patch Thank you. I have updated patch with such changes: fix code style and logic for cleanup previous frames. About for - in such case we don't needed full loop - because mainly we have maximum one not cleaned frame before current and while iterated backward only once.
Created attachment 289115 [details] [review] gif: Minimize memory usage for gif animation Update the logic to create the next frame. When the new frame uses a buffer from the previous frame, in retain, revert or dispose mode, try to reuse the allocated buffer from the previous frame. This reduces allocated memory by an order of magnitude. For other cases when we can't use the old buffer, clean up the previous buffers after the new frame has been handled.
Attachment 289115 [details] pushed as 1d5bd7c - gif: Minimize memory usage for gif animation
*** Bug 344731 has been marked as a duplicate of this bug. ***