Skip to content

Hording pixbufs - Memory usage

Issue

Noticed a memory build-up, especially bad when changing background styling. Have experienced 4-5G RAM use.

After running libmemleak etc. I believe the culprit is found in the way bixbuf cache is handled in terminal-image-loader.c.

This issue is especially bad the longer one run the terminal. Say one have a terminal running for a few days or more - and change background or other settings along the way – the result is bad 😅

Even shorter runs can be bad especially if one modify the terminal frequently.

Suggestion

In terminal/terminal-image-loader.c

  1. loader->cache_invalid should either not be used at all or have a limit on entries. From what I can see, all it is is a extremely expensive counter where a single int could do the same job
  2. loader->cache should likely have a lifetime or limit on number of entries. Another way would be to give user the ability to clear cache, optionally set no cache or cache limit.

Of course, by implementing 1., one can trigger 2. manually by changing a setting that makes the loader start a new cache list.

I have limited GDK knowledge and have been far to long since I used it, but something like this perhaps:

  if (invalidate) {
    if (G_LIKELY(loader->cache != NULL))
      g_slist_free_full(g_steal_pointer(&loader->cache), g_object_unref);
    //loader->cache_invalid =
    //    g_slist_concat(loader->cache_invalid, loader->cache);
    loader->cache = NULL;
  }

Image loading and pixbufs

A little background and my attempt at deciphering. Have no knowledge of the code base.

The loader has a object calledloader. Three of the properties of interest are:

  1. loader->pixbuf 🖼 : Current loaded image w. styling
  2. loader->cache 🖇 Single linked list with slices of pixbuf used for painting
  3. loader->cache_invalid 🖇 Invalidated caches due to styling changes
  • On re-draw terminal_image_loader_load is called from terminal_screen_draw()
  • On first run and if background image, color, style etc. has changed a new loader->pixbuf is created and the old is de-referenced. At the same time all cached composites of the pixbuf is stored in a single linked list: loader->cache => loader->cache_invalid - which does not seem to have any use.
  • On first run and each time terminal window is resized to a size not in loader->cache a new pixbuf (composite) is created from loader->pixbuf. This one is prepended to the single linked list loader->cache.

So: One start out with one pixbuf, the full image data, and one cache entry which is a composite of this one fitted to the current terminal size. Each time the terminal is resized one check if the same size has been used before and hence cached in loader->cache, if not, create a new one.

If styling changes then push the current loader->cache to loader->cache_invalid and start a new list in loader->cache.

So after some changes, e.g. image and resizeing terminal window, one could have something like this:

loader->pixbuf (image 4)
          |
          +-> loader->cache[0]   80x120 composite - current
          +-> loader->cache[1]   80x63  composite
          +-> loader->cache[2]  120x80  composite
          +-> loader->cache[3]  530x90  composite
          +-> loader->cache[..]  ...x.  composite
          +-> loader->cache[102]  ....
loader->cache_invalid
          [0]->old_cache[80x120, 80x63, 120x80, 530x90, 36x80, ...] (image 1)
          [1]->old_cache[80x120, 75x63, 120x80, 420x90, 42x80, ...] (image 2)
          [3]->old_cache[80x120, 75x63, 120x80, 420x90, 90x33, ...] (image 3)

So say each entry takes 5-10M RAM - one have 500M - 1,000M in cached entries and 2-3G in cache_invalid.

If one have 10 terminals open they all, luckily, share the cache - but this also means that one can not close one to regain memory. One have to kill of all. Running 10-15 terminals with various ssh connections, programs, vim etc. and being forced to close all down due to memory consumption is bad.

More gritty details

One get a request to terminal_image_loader_load(). This seems to happen each time terminal needs to be redrawn. For example each time cursor blinks or pointer is drawn across the window - and of course change in content etc.

For anyone uninitiated note that what is commented as image width and image height - is the size of the canvas / terminal window. Not the size of the picture (as I thought first).

After some rudimentary error checking terminal_image_loader_check() is called. This one check if path to background image has changed. If so it releases it's reference to current pixbuf, loader->pixbuf, and create a new one.

No cache_invalid

This and some other events causes the current loader->cache to be marked as invalid, and here is the main part where I guess one could do better:

  if (invalidate)
    {
      loader->cache_invalid = g_slist_concat (loader->cache_invalid,
                                              loader->cache);
      loader->cache = NULL;
    }

The current cache is added to loader->cache_invalid, but this cache is never used. Only place is in debug code:

#ifdef G_ENABLE_DEBUG
  g_debug ("Image Loader Memory Status: %d images in valid "
           "cache, %d in invalid cache",
           g_slist_length (loader->cache),
           g_slist_length (loader->cache_invalid));
#endif

If this is of interest one could instead add a gint to the loader class and do:

	++loader->invalidated_caches;

Same end result without the huge use of memory.

Limit cached entries

Limit number of entries

Instead of adding to a list with no limit one could perhaps do something like this in place for g_slist_prepend() in terminal_image_loader_load:

	cache_pixbuf(loader, pixbuf);

Where crudely cache_pixbuf:

#define PIXBUF_CACHE_MAXLEN 20
static void cache_pixbuf(TerminalImageLoader *loader, GdkPixbuf *pixbuf)
{
	gint n = g_slist_length(loader->cache);
	GSList *lp;

	if (n > PIXBUF_CACHE_MAXLEN) {
		lp = g_slist_last(loader->cache);
		g_object_unref(G_OBJECT(lp->data));
		loader->cache = g_slist_delete_link(loader->cache, lp);
	}
	loader->cache = g_slist_prepend(loader->cache, pixbuf);
}

Of course, even then, if one have an image as background it can easilly eat 200MB RAM and more.

Limited lifespan

There is of course a lot of other and likely better ways of doing this, but some suggestions at least.

A better way would likely be to use a timer and no limit on cache entries. Fill the cache as needed and set a timeout of say 5 minutes. After this go back in and free the cache list.

This way one limit the stress when resizing + reduce the long-time memory use by a lot.

Sample output from libmemleak

dump

xfce4-terminal: Now: 3234; 	Backtraces: 76621; 	allocations: 77064; 	total memory: 1,416,700,835 bytes.

 backtrace 29144 (value_n: 975.00); [2883,2893>(  10):     6 allocations (     6 total, 100.0%), size 36164232;   0.60 allocations/s, 3616423 bytes/s
 backtrace 29144 (value_n: 975.00); [2867,2883>(  16):    12 allocations (    12 total, 100.0%), size 72328464;   0.75 allocations/s, 4520529 bytes/s
 backtrace 29144 (value_n: 975.00); [2728,2754>(  26):     7 allocations (     7 total, 100.0%), size 42191604;   0.27 allocations/s, 1622754 bytes/s
 backtrace 29144 (value_n: 975.00); [2698,2728>(  30):     8 allocations (     8 total, 100.0%), size 48218976;   0.27 allocations/s, 1607299 bytes/s
 backtrace 29144 (value_n: 975.00); [2651,2698>(  47):    11 allocations (    11 total, 100.0%), size 66301092;   0.23 allocations/s, 1410661 bytes/s
 backtrace 29144 (value_n: 975.00); [2461,2525>(  64):    12 allocations (    12 total, 100.0%), size 72328464;   0.19 allocations/s, 1130132 bytes/s
 backtrace 29144 (value_n: 975.00); [1974,2105>( 131):     4 allocations (     4 total, 100.0%), size 24109488;   0.03 allocations/s, 184041 bytes/s
 backtrace 29144 (value_n: 975.00); [ 652, 783>( 131):    44 allocations (    44 total, 100.0%), size 265204368;   0.34 allocations/s, 2024460 bytes/s
 backtrace 29144 (value_n: 975.00); [ 369, 615>( 246):    72 allocations (    72 total, 100.0%), size 433970784;   0.29 allocations/s, 1764108 bytes/s
 backtrace 29144 (value_n: 975.00); [  74, 318>( 244):    67 allocations (    67 total, 100.0%), size 304631176;   0.27 allocations/s, 1248488 bytes/s

Trace

libmemleak> dump 30584
 #0  00007f0a7291ffbc  in calloc at /home/nick/soft/libmemleak/libmemleak-objdir/src/../../libmemleak/src/memleak.c:1036
 #1  00007f0a6fbb8b11  in g_malloc0 at ./debian/build/deb/glib/../../../../glib/gmem.c:131
 #2  00007f0a6fe8ccdb  in g_closure_new_simple at ./debian/build/deb/gobject/../../../../gobject/gclosure.c:214
 #3  00007f0a6fe8e490  in g_cclosure_new at ./debian/build/deb/gobject/../../../../gobject/gclosure.c:945
 #4  00007f0a6fea76b9  in g_signal_connect_data at ./debian/build/deb/gobject/../../../../gobject/gsignal.c:2510
 #5  0000561915f1d1fc  in terminal_screen_update_misc_bell at /home/nick/soft/xfce4/xfce4-terminal/terminal/terminal-screen.c:1063
 #6  0000561915f1b8a6  in terminal_screen_preferences_changed at /home/nick/soft/xfce4/xfce4-terminal/terminal/terminal-screen.c:569
 #7  00007f0a6fe8e10d  in closure_invoke_notifiers at ./debian/build/deb/gobject/../../../../gobject/gclosure.c:290
 #8  00007f0a6fea105e  in accumulate at ./debian/build/deb/gobject/../../../../gobject/gsignal.c:3140
 #9  00007f0a6fea9715  in g_signal_emit_valist at ./debian/build/deb/gobject/../../../../gobject/gsignal.c:3419
 #10 00007f0a6feaa12f  in g_signal_emit at ./debian/build/deb/gobject/../../../../gobject/gsignal.c:3449
 #11 00007f0a6fe925c4  in g_object_dispatch_properties_changed at ./debian/build/deb/gobject/../../../../gobject/gobject.c:1081
 #12 00007f0a6fe91f6e  in g_object_notify_queue_thaw at ./debian/build/deb/gobject/../../../../gobject/gobject.c:297
 #13 00007f0a6fe94bcb  in g_object_thaw_notify at ./debian/build/deb/gobject/../../../../gobject/gobject.c:1322
 #14 0000561915f1503f  in terminal_preferences_load at /home/nick/soft/xfce4/xfce4-terminal/terminal/terminal-preferences.c:1131
 #15 0000561915f1558d  in terminal_preferences_monitor_changed at /home/nick/soft/xfce4/xfce4-terminal/terminal/terminal-preferences.c:1287
 #16 00007f0a69977dae  in "libffi.so.6.0.4"
 #17 00007f0a6997771f  in "libffi.so.6.0.4"
 #18 00007f0a6fe8eced  in g_cclosure_marshal_generic_va at ./debian/build/deb/gobject/../../../../gobject/gclosure.c:1607
 #19 00007f0a6fe8e346  in closure_invoke_notifiers at ./debian/build/deb/gobject/../../../../gobject/gclosure.c:290
 #20 00007f0a6fea99ff  in g_signal_emit_valist at ./debian/build/deb/gobject/../../../../gobject/gsignal.c:3300
 #21 00007f0a6feaa12f  in g_signal_emit at ./debian/build/deb/gobject/../../../../gobject/gsignal.c:3449
 #22 00007f0a705dd1e9  in g_file_monitor_source_dispatch at ./debian/build/deb/gio/../../../../gio/glocalfilemonitor.c:560
 #23 00007f0a6fbb3417  in g_main_dispatch at ./debian/build/deb/glib/../../../../glib/gmain.c:3180
 #24 00007f0a6fbb3650  in g_main_context_iterate at ./debian/build/deb/glib/../../../../glib/gmain.c:3902
 #25 00007f0a6fbb3962  in g_main_loop_run at ./debian/build/deb/glib/../../../../glib/gmain.c:4097
 #26 00007f0a71546a25  in gtk_main at ./debian/build/deb/gtk/../../../../gtk/gtkmain.c:1324
 #27 0000561915f0d711  in main at /home/nick/soft/xfce4/xfce4-terminal/terminal/main.c:323
 #28 00007f0a6f578b97  in __libc_start_main at /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:344
 #29 0000561915f0c73a  in _start
Edited by M. AB