Subject: Re: bug#68006: 30.0.50; Image-mode speed

From: Manuel Giraud <[email-masked]>
Date: Mon, 28 Oct 2024 15:31:22 +0100

Hi Eli and Stefan,

Here is a new version for my patch to add some user control on the image
cache.  This time, I have done some tests and benchmarks.

Instead of trying to have a completely different cache, I'm using the
current one but I had a condition to flush (or not) an image based on a
user defined time to live (TTL) for an image.  This TTL is a number of
seconds since the image was last displayed.

I have also tested it in image-mode with the second patch.  I tested by
browsing a directory with some rather large pictures in it (4000x3000
pixels, ≈6MB for each file).  I opened the first image in a 1640x1000
window (so each image is scaled) and did the following:

  - M-: (dotimes (i 10) (image-next-file 1))  ;; for cache warming
  - M-: (dotimes (i 10) (image-next-file -1)) ;; return to first image
  - M-: (benchmark-run 10 (dotimes (i 10) (image-next-file 1))
                          (dotimes (i 10) (image-next-file -1)))

Here is the timings I get:

     - with master (ea685170063): (109.208767158 15 1.0938777159999997)
     - with this patch: (6.934307561 15 1.095243891)

WDYT?
>From cfa3858ab954fd8354d3b080975f2d679004f294 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <[email-masked]>
Date: Mon, 28 Oct 2024 14:35:06 +0100
Subject: [PATCH 1/2] User defined time to live for image

---
 src/dispextern.h |  3 +++
 src/image.c      | 33 +++++++++++++++++++++++++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index cc248a4472e..52887ce1687 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3141,6 +3141,9 @@ reset_mouse_highlight (Mouse_HLInfo *hlinfo)
      in prepare_image_for_display.  */
   struct timespec timestamp;
 
+  /* Time to live after last displayed in seconds.  */
+  EMACS_INT ttl;
+
   /* Pixmaps of the image.  */
   Emacs_Pixmap pixmap, mask;
 
diff --git a/src/image.c b/src/image.c
index 34936977a40..1343276b8c8 100644
--- a/src/image.c
+++ b/src/image.c
@@ -1744,6 +1744,7 @@ make_image (Lisp_Object spec, EMACS_UINT hash)
 {
   struct image *img = xzalloc (sizeof *img);
   Lisp_Object file = image_spec_value (spec, QCfile, NULL);
+  Lisp_Object ttl = image_spec_value (spec, QCttl, NULL);
 
   eassert (valid_image_p (spec));
   img->dependencies = NILP (file) ? Qnil : list1 (file);
@@ -1754,6 +1755,11 @@ make_image (Lisp_Object spec, EMACS_UINT hash)
   img->ascent = DEFAULT_IMAGE_ASCENT;
   img->hash = hash;
   img->corners[BOT_CORNER] = -1;  /* Full image */
+  if (! NILP (ttl) && FIXNUMP (ttl))
+    img->ttl = XFIXNUM (ttl);
+  else
+    img->ttl = 0;
+
   return img;
 }
 
@@ -2258,11 +2264,13 @@ filter_image_spec (Lisp_Object spec)
 	  spec = XCDR (spec);
 
 	  /* Some animation-related data doesn't affect display, but
-	     breaks the image cache.  Filter those out.  */
+	     breaks the image cache.  Filter those out.  Also filter out
+	     the user's cache time to live.  */
 	  if (!(EQ (key, QCanimate_buffer)
 		|| EQ (key, QCanimate_tardiness)
 		|| EQ (key, QCanimate_position)
-		|| EQ (key, QCanimate_multi_frame_data)))
+		|| EQ (key, QCanimate_multi_frame_data)
+		|| EQ (key, QCttl)))
 	    {
 	      out = Fcons (value, out);
 	      out = Fcons (key, out);
@@ -2332,11 +2340,31 @@ clear_image_cache (struct frame *f, Lisp_Object filter)
   if (c && !f->inhibit_clear_image_cache)
     {
       ptrdiff_t i, nfreed = 0;
+      struct timespec now = current_timespec ();
+      struct timespec t, delay;
 
       /* Block input so that we won't be interrupted by a SIGIO
 	 while being in an inconsistent state.  */
       block_input ();
 
+      /* First, filter image cache based on image's time to live.  */
+      for (i = 0; i < c->used; ++i)
+	{
+	  struct image *img = c->images[i];
+	  /* A negative TTL means that this image is cached forever.  */
+	  if (img && img->ttl >= 0)
+	    {
+	      delay.tv_sec = img->ttl;
+	      delay.tv_nsec = 0;
+	      t = timespec_add(now, delay);
+	      if (timespec_cmp (t, img->timestamp) > 0)
+		{
+		  free_image (f, img);
+		  ++nfreed;
+		}
+	    }
+	}
+
       if (!NILP (filter))
 	{
 	  /* Filter image cache.  */
@@ -12979,6 +13007,7 @@ syms_of_image (void)
   DEFSYM (QCcolor_adjustment, ":color-adjustment");
   DEFSYM (QCmask, ":mask");
   DEFSYM (QCflip, ":flip");
+  DEFSYM (QCttl, ":ttl");
 
   /* Other symbols.  */
   DEFSYM (Qlaplace, "laplace");
-- 
2.47.0

>From ca5df9e0e7cc00eea1e7fc74d41b33c52aeddfab Mon Sep 17 00:00:00 2001
From: Manuel Giraud <[email-masked]>
Date: Mon, 28 Oct 2024 14:37:47 +0100
Subject: [PATCH 2/2] Try it in image-mode

---
 lisp/image-mode.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index e75f6ea918f..ffc2697085e 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -949,12 +949,14 @@ image-toggle-display-image
     ;; default scaling based on font size.
     (setq image (if (not edges)
 		    (create-image file-or-data type data-p :scale 1
-                                  :format (and filename data-p))
+                                  :format (and filename data-p)
+                                  :ttl 30)
 		  (create-image file-or-data type data-p :scale 1
 				:max-width max-width
 				:max-height max-height
                                 ;; Type hint.
-                                :format (and filename data-p))))
+                                :format (and filename data-p)
+                                :ttl 30)))
 
     ;; Handle `fit-window'.
     (when (and (eq image-transform-resize 'fit-window)
@@ -964,8 +966,6 @@ image-toggle-display-image
                   (plist-put (cdr image) :width
                              (plist-get (cdr image) :max-width)))))
 
-    ;; Discard any stale image data before looking it up again.
-    (image-flush image)
     (setq image (image--update-properties image (image-transform-properties image)))
     (setq props
 	  `(display ,image
-- 
2.47.0

-- 
Manuel Giraud