Free images based on allocated memory

Free images based on allocated memory

[Andreas Politz]

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]


Recently there was a discussion about how to limit the amount of memory
the pdf-tools package uses by means of the image-cache.

The package renders pages to a PNG image, each one having a size of a
couple of MB.  Thus, skimming through a large document can quickly fill
the cache with images in the GB range.

Anyway, it occurred to me, that freeing the cache via clear-image-cache
etc. is much like counting conses and triggering garbage-collection
manually.  So I wondered if there is some interest to add an automatic
eviction of the cache based on it's current size.

For this I've added a draft patch, which has some drawbacks:

1. The accounting is frame-local, which means the cache could still grow
   unbounded as a function of the number of frames.
2. The computation of memory used by an image is very simple.

Andreas

P.S.: Please CC me, since I'm currently not subscribed.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: image-cache-size-limit.patch --]
[-- Type: text/x-patch, Size: 4549 bytes --]

diff --git a/src/image.c b/src/image.c
index bcc61dfccd..85adaa61f8 100644
--- a/src/image.c
+++ b/src/image.c
@@ -1018,6 +1018,8 @@ free_image (struct frame *f, struct image *img)
 
       c->images[img->id] = NULL;
 
+      c->allocated_image_size -= img->allocated_size;
+      eassert (c->allocated_image_size >= 0);
 #ifdef HAVE_XRENDER
       if (img->picture)
         XRenderFreePicture (FRAME_X_DISPLAY (f), img->picture);
@@ -1464,6 +1466,7 @@ make_image_cache (void)
   c->used = c->refcount = 0;
   c->images = xmalloc (c->size * sizeof *c->images);
   c->buckets = xzalloc (IMAGE_CACHE_BUCKETS_SIZE * sizeof *c->buckets);
+  c->allocated_image_size = 0;
   return c;
 }
 
@@ -1540,6 +1543,53 @@ free_image_cache (struct frame *f)
     }
 }
 
+/* Compare images inducing an order of increasing timestamps followed
+   by NULL images . */
+static int
+compare_images_by_timestamp (const void *e0, const void* e1)
+{
+  const struct image *i0 = *((const struct image**) e0);
+  const struct image *i1 = *((const struct image**) e1);
+
+  return ! i0 ? 1 : ! i1 ? -1 : timespec_cmp (i0->timestamp, i1->timestamp);
+}
+
+static ptrdiff_t
+clear_image_cache_by_cache_size_limit (struct frame *f)
+{
+  struct image_cache *c = FRAME_IMAGE_CACHE (f);
+
+  if (! FIXNUMP (Vimage_cache_size_limit)
+      || ! c
+      || c->allocated_image_size < 2 * XFIXNUM (Vimage_cache_size_limit))
+    return 0;
+
+  USE_SAFE_ALLOCA;
+  ptrdiff_t nfreed = 0;
+  int size_limit = XFIXNUM (Vimage_cache_size_limit);
+  struct image **images;
+  int bytes_freed = 0;
+  int allocated_image_size = c->allocated_image_size;
+
+  SAFE_NALLOCA (images, sizeof *images, c->used);
+  memcpy (images, c->images, c->used * sizeof *images);
+  qsort (images, c->used, sizeof *images, compare_images_by_timestamp);
+
+  for (int i = 0;
+       i < c->used && images[i] && c->allocated_image_size > size_limit;
+       ++i)
+    {
+      bytes_freed += images[i]->allocated_size;
+      free_image (f, images[i]);
+      ++nfreed;
+    }
+  SAFE_FREE ();
+  fprintf (stderr, "[IMAGE CACHE]: %d KB - %d KB = %d KB.\n",
+           allocated_image_size / 1024,
+           bytes_freed/ 1024,
+           c->allocated_image_size / 1024);
+  return nfreed;
+}
 
 /* Clear image cache of frame F.  FILTER=t means free all images.
    FILTER=nil means clear only images that haven't been
@@ -1608,6 +1658,9 @@ clear_image_cache (struct frame *f, Lisp_Object filter)
 	    }
 	}
 
+      if (NILP (filter))
+        nfreed += clear_image_cache_by_cache_size_limit (f);
+
       /* We may be clearing the image cache because, for example,
 	 Emacs was iconified for a longer period of time.  In that
 	 case, current matrices may still contain references to
@@ -1931,11 +1984,15 @@ lookup_image (struct frame *f, Lisp_Object spec)
   if (img == NULL)
     {
       block_input ();
+      /* Maybe free some images here so the cache does not get to
+         large.  Otherwise this is only done now and then in
+         redisplay. */
+      clear_image_cache_by_cache_size_limit (f);
       img = make_image (spec, hash);
-      cache_image (f, img);
       img->load_failed_p = ! img->type->load (f, img);
       img->frame_foreground = FRAME_FOREGROUND_PIXEL (f);
       img->frame_background = FRAME_BACKGROUND_PIXEL (f);
+      cache_image (f, img);
 
       /* If we can't load the image, and we don't have a width and
 	 height, use some arbitrary width and height so that we can
@@ -2046,6 +2103,16 @@ cache_image (struct frame *f, struct image *img)
     img->next->prev = img;
   img->prev = NULL;
   c->buckets[i] = img;
+
+  /* Account for it's allocated memory */
+  if (! img->load_failed_p)
+    {
+      if (img->ximg)
+        img->allocated_size += img->ximg->height * img->ximg->bytes_per_line;
+      if (img->mask_img)
+        img->allocated_size += img->mask_img->height * img->mask_img->bytes_per_line;
+      c->allocated_image_size += img->allocated_size;
+    }
 }
 
 
@@ -10194,6 +10261,11 @@ The value can also be nil, meaning the cache is never cleared.
 
 The function `clear-image-cache' disregards this variable.  */);
   Vimage_cache_eviction_delay = make_fixnum (300);
+
+  DEFVAR_LISP ("image-cache-size-limit", Vimage_cache_size_limit,
+    doc: /* Maximum size of an image cache before images are removed.*/);
+  Vimage_cache_size_limit = make_fixnum (1024 * 1024 * 128);
+
 #ifdef HAVE_IMAGEMAGICK
   DEFVAR_INT ("imagemagick-render-type", imagemagick_render_type,
     doc: /* Integer indicating which ImageMagick rendering method to use.

Re: Free images based on allocated memory

[Stefan Monnier]

> Recently there was a discussion about how to limit the amount of memory
> the pdf-tools package uses by means of the image-cache.

The same problem affects doc-view, BTW.

> For this I've added a draft patch, which has some drawbacks:
> 1. The accounting is frame-local, which means the cache could still grow
>    unbounded as a function of the number of frames.

Why do you keep the counter in the frame/cache rather than in (say)
a global variable or inside the terminal object?


        Stefan

Re: Free images based on allocated memory

[Eli Zaretskii]

> From: Andreas Politz <politza@hochschule-trier.de>
> Date: Tue, 29 Jan 2019 19:56:45 +0100
> 
> Recently there was a discussion about how to limit the amount of memory
> the pdf-tools package uses by means of the image-cache.
> 
> The package renders pages to a PNG image, each one having a size of a
> couple of MB.  Thus, skimming through a large document can quickly fill
> the cache with images in the GB range.
> 
> Anyway, it occurred to me, that freeing the cache via clear-image-cache
> etc. is much like counting conses and triggering garbage-collection
> manually.  So I wondered if there is some interest to add an automatic
> eviction of the cache based on it's current size.

We already clear the image caches from time to time, see
clear_face_cache.  If that is not good enough for some reason (and if
so, can you explain why?), would it make sense to enhance what we
already have, rather than invent a new mechanism?

Thanks.