bug#33275: 27.0.50; Image cache pruning

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 33275@debbugs.gnu.org
> Date: Mon, 05 Nov 2018 18:48:33 +0100
> 
> At present, the image cache is purely based on timestamps, if I recall
> correctly?  If we wish to fix this unexpected behaviour, we could also
> look at the size of the image cache and perhaps try to limit it, or if
> we wanted to be really fancy, we could somehow check whether there's any
> references to the images anywhere (beyond the image cache itself).
> 
> I think the latter would mean adding some... fun... to the gc system?
> 
> Or does Emacs have weak hashes?  In which case the image cache could be
> a weak hash and the problem would resolve itself?
> [...]
> Emacs does have weak hashes...  Hm...
> 
> Anyway, there's code that tries to keep the cache small:
> 
>           /* If the number of cached images has grown unusually large,
>              decrease the cache eviction delay (Bug#6230).  */
>           delay = XFIXNUM (Vimage_cache_eviction_delay);
>           if (nimages > 40)
>             delay = 1600 * delay / nimages / nimages;
> 
> But it works solely on the number of images in the cache, and not the
> size of the images.  So it's a heuristic that could be tweaked pretty
> easily, I think?

After thinking about this and looking at the code, I don't think I
understand your proposal(s).  Here are the issues that I saw with what
I think you proposed:

 . The hash table used by the image cache is not a Lisp hash-table, so
   weakness doesn't apply.  We cache pointers to C structs, so using a
   Lisp hash-tables would need "some work".
 . The code you show above is called from redisplay, so it will not be
   executed as long as a Lisp program (such as the one you show at the
   beginning of this discussion) runs, and you will still have the OOM
   killer get you before that code get a chance to clear the cache.
 . Running such code from GC could be tricky, because freeing images
   needs to remove references of those images from the glyph matrices,
   and that cannot be safely done from arbitrary places.
 . I don't think you can ensure GC will run during your program
   anyway, because most of the memory allocated by your program is not
   used to cons Lisp objects, so maybe_gc called by the Lisp
   interpreter will most probably decide there's no need for GC and do
   nothing.  Therefore, I don't think that avoiding to cache the
   images would prevent the OOM situation in your case.

I think if we want to prevent the OOM killer from killing Emacs due to
many cached images, we should inspect inside cache_image the current
VM size of the process (like system_process_attributes does), compare
it to the memory limits, and display a warning and/or signal an error
when we get dangerously close to the limit.  If we want to avoid
similar situations with other objects, we could do these tests inside
xmalloc and mmap_alloc/mmap_realloc.  (We used to do something similar
in the past, but that code needed a libc hook for memory allocation
from the OS, something I understand we cannot rely on now.)

IOW, I don't see how we could prevent being OOM-killed, except if we
monitor the committed memory at the point where it is allocated, as
otherwise the opportunistic memory allocation strategy can always trip
us.