bug#51763: 27.2; Displaying many images take all memory

bug#51763: 27.2; Displaying many images take all memory

[Thierry Volpiatto]

Hello,

I see you changed image-dired to use image-mode in emacs-29 instead of using like
before image-magick.  That's fine I used the same approach two years ago
in Helm, but switched back quickly to image-dired because it was taking
all memory and it was not recoverable until I kill emacs. I thought it
was my fault but I see new image-dired in emacs-29 have same problem:

1) emacs -Q
2) Open a large image directory with dired
3) Open files one by one with C-t i until memory is full (use f3 C-t i
C-n f4 etc...). Memory starts to grow seriously after around 70 files
for me.

Killing the image-dired buffer changes nothing, I have to restart emacs
to recover memory.

And now unfortunately I have no more alternatives with Helm in emacs-29 since
image-dired uses inconditionally this.

You can reproduce the same bug with helm by setting
helm-ff-display-image-native to `t` and from helm-find-files hit
C-<down> repetitively in same image directory (not using
helm-ff-display-image-native i.e. with nil value is now broken with new image-dired).


In GNU Emacs 27.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10)
 of 2021-03-25 built on IPadS340
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Linux Mint 19.3

Recent messages:
C-c C-g is undefined
Quit
Auto-saving...done
Auto-saving...done
Send this bug report to the Emacs maintainers? (y or n) y
Quit
[mu4e] Switch to Posteo
[mu4e] Switched context to Posteo
Mark set [2 times]
Message modified; kill anyway? (y or n) y

Configured using:
 'configure CFLAGS=-O8 --without-dbus --without-gconf
 --without-gsettings --with-mailutils --with-cairo'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM GLIB NOTIFY INOTIFY ACL
LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON
PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: fr_FR.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Elisp

Minor modes in effect:
  bug-reference-prog-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  psession-mode: t
  psession-savehist-mode: t
  global-git-gutter-mode: t
  git-gutter-mode: t
  display-time-mode: t
  winner-mode: t
  show-paren-mode: t
  helm-epa-mode: t
  helm-descbinds-mode: t
  helm-adaptive-mode: t
  helm-mode: t
  shell-dirtrack-mode: t
  helm-popup-tip-mode: t
  async-bytecomp-package-mode: t
  dired-async-mode: t
  minibuffer-depth-indicate-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/thierry/.emacs.d/elpa/seq-2.23/seq hides /usr/local/share/emacs/27.2/lisp/emacs-lisp/seq

Features:
(epa-mail face-remap addressbook-bookmark mu4e-config org-mu4e
mu4e-contrib mu4e-patch mu4e mu4e-org mu4e-view gnus-art mm-uu mml2015
mm-view mml-smime smime dig gnus-sum gnus-group gnus-undo gnus-start
gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int
gnus-range gnus-win mu4e-main mu4e-headers mu4e-lists mu4e-compose
mu4e-draft mu4e-actions smtpmail mu4e-search mu4e-bookmarks mu4e-mark
mu4e-message shr svg dom flow-fill hl-line mu4e-contacts mu4e-update
mu4e-folders mu4e-server mu4e-context mu4e-vars mu4e-helpers ido
mu4e-meta shadow sort mail-extr emacsbug sendmail helm-command epa-file
em-unix em-term term disp-table ehelp em-script em-prompt em-ls em-hist
em-pred em-glob em-dirs esh-var em-cmpl em-basic em-banner em-alias
esh-mode esh-toggle tramp-archive tramp-gvfs dbus helm-x-files
helm-for-files helm-bookmark bookmark pp sh-script smie executable
vc-filewise vc-rcs conf-mode ledger-config ledger-mode ledger-check
ledger-texi ledger-test ledger-sort ledger-report ledger-reconcile
ledger-occur ledger-fonts ledger-fontify ledger-state ledger-complete
ledger-schedule ledger-init ledger-xact ledger-post ledger-exec
ledger-navigate eshell esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg
esh-module esh-groups esh-util ledger-context ledger-commodities
ledger-regex rx bug-reference naquadah-theme solar cal-dst holidays
hol-loaddefs tv-utils yaml-mode undo-tree diff rainbow-mode color
psession frameset log-view pcvs-util pcmpl-git cl-indent ffap thingatpt
autocrypt-message message rmc puny rfc822 mml mml-sec mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
autocrypt-gnus gnus nnheader gnus-util rmail rmail-loaddefs rfc2047
rfc2045 text-property-search mail-utils mm-util mail-prsvr
autocrypt-mu4e autocrypt ietf-drums config-w3m git-gutter mule-util appt
diary-lib diary-loaddefs gud pcomplete-extension pcmpl-unix pcmpl-gnu
iterator pcase wdired dired-extension org-config ob-gnuplot org-crypt
net-utils time all-the-icons all-the-icons-faces data-material
data-weathericons data-octicons data-fileicons data-faicons
data-alltheicons winner autotest-mode autoconf-mode paren woman man
ediff ediff-merg ediff-mult ediff-wind ediff-diff ediff-help ediff-init
ediff-util init-helm helm-ls-git vc-git diff-mode vc vc-dispatcher
helm-fd epa derived epg epg-config helm-epa helm-misc helm-imenu imenu
helm-elisp-package helm-find helm-org org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete
org-list org-faces org-entities noutline outline org-version
ob-emacs-lisp ob-core ob-eval org-table ol org-keys org-compat advice
org-macs org-loaddefs cal-menu calendar cal-loaddefs helm-external
helm-net xml url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap isearch-light
helm-descbinds cus-edit wid-edit helm-ipython helm-elisp helm-eval
edebug backtrace find-func helm-info python tramp-sh helm-adaptive
helm-mode helm-files filenotify tramp tramp-loaddefs trampver
tramp-integration files-x tramp-compat shell pcomplete parse-time
iso8601 time-date ls-lisp helm-buffers helm-occur helm-tags helm-locate
helm-grep wgrep-helm wgrep grep compile comint ansi-color ring
helm-regexp format-spec helm-utils helm-help helm-types
helm-extensions-autoloads helm-config helm-autoloads helm async-bytecomp
helm-global-bindings helm-easymenu helm-source helm-multi-match helm-lib
dired-async dired-aux dired dired-loaddefs async popup diminish cl-extra
help-mode mb-depth server edmacro kmacro avoid cus-start cus-load
use-package use-package-ensure use-package-delight use-package-diminish
use-package-bind-key bind-key easy-mmode use-package-core info w3m-load
package easymenu browse-url url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads inotify lcms2
dynamic-setting font-render-setting cairo move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 453504 131281)
 (symbols 48 36164 19)
 (strings 32 137667 18976)
 (string-bytes 1 4458057)
 (vectors 16 62701)
 (vector-slots 8 1124931 144556)
 (floats 8 1525 567)
 (intervals 56 5485 1151)
 (buffers 1000 94))
<#secure method=pgpmime mode=sign>

-- 
Thierry

bug#51763: 27.2; Displaying many images take all memory

[Stefan Kangas]

Thierry Volpiatto <thievol@posteo.net> writes:

> I see you changed image-dired to use image-mode in emacs-29 instead of using like
> before image-magick.  That's fine I used the same approach two years ago
> in Helm, but switched back quickly to image-dired because it was taking
> all memory and it was not recoverable until I kill emacs. I thought it
> was my fault but I see new image-dired in emacs-29 have same problem:
>
> 1) emacs -Q
> 2) Open a large image directory with dired
> 3) Open files one by one with C-t i until memory is full (use f3 C-t i
> C-n f4 etc...). Memory starts to grow seriously after around 70 files
> for me.
>
> Killing the image-dired buffer changes nothing, I have to restart emacs
> to recover memory.

Thanks for the bug report!

In principle there should be no difference between the two: in both
cases we need to cache the same upscaled image.

However, we currently have an issue with our built-in image scaling that
we cache the image both before resizing and after.  I suspect that this
is the explanation for the higher memory usage you see.

See this comment in `image--scale-within-limits-p':

            ;; Note: `image-size' looks up and thus caches the
            ;; untransformed image.  There's no easy way to
            ;; prevent that.

and the relevant code in image.c that verifies this.

I believe that we could fix this in image.c.  I don't think it's
necessarily very hard, but it does take some coding.

However, I'm curious what you mean when you say that it "never" frees
the memory.  What happens if you set `image-cache-eviction-delay' to
some very low value like 5 seconds?

AFAIU, calling `clear-image-cache' should also the free memory unless we
have a memory leak.  You could also call this function from your code.
Does calling this function free the memory for you?

bug#51763: 27.2; Displaying many images take all memory

[Thierry Volpiatto]

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


Stefan Kangas <stefan@marxist.se> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> I see you changed image-dired to use image-mode in emacs-29 instead of using like
>> before image-magick.  That's fine I used the same approach two years ago
>> in Helm, but switched back quickly to image-dired because it was taking
>> all memory and it was not recoverable until I kill emacs. I thought it
>> was my fault but I see new image-dired in emacs-29 have same problem:
>>
>> 1) emacs -Q
>> 2) Open a large image directory with dired
>> 3) Open files one by one with C-t i until memory is full (use f3 C-t i
>> C-n f4 etc...). Memory starts to grow seriously after around 70 files
>> for me.
>>
>> Killing the image-dired buffer changes nothing, I have to restart emacs
>> to recover memory.
>
> Thanks for the bug report!
>
> In principle there should be no difference between the two: in both
> cases we need to cache the same upscaled image.
>
> However, we currently have an issue with our built-in image scaling that
> we cache the image both before resizing and after.  I suspect that this
> is the explanation for the higher memory usage you see.
>
> See this comment in `image--scale-within-limits-p':
>
>             ;; Note: `image-size' looks up and thus caches the
>             ;; untransformed image.  There's no easy way to
>             ;; prevent that.
>
> and the relevant code in image.c that verifies this.
>
> I believe that we could fix this in image.c.  I don't think it's
> necessarily very hard, but it does take some coding.

Sorry but this is out of my scope.

> However, I'm curious what you mean when you say that it "never" frees
> the memory.  What happens if you set `image-cache-eviction-delay' to
> some very low value like 5 seconds?

Didn't yet tried this one.

> AFAIU, calling `clear-image-cache' should also the free memory unless we
> have a memory leak.  You could also call this function from your code.
> Does calling this function free the memory for you?

Yes it does 🙂.  I didn't know this function (BTW what about renaming to
`image-clear-cache`), works great with my code, perhaps you can do the
same in dired (we use more or less similar code).

Thanks.

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

bug#51763: 27.2; Displaying many images take all memory

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> See this comment in `image--scale-within-limits-p':
>
>             ;; Note: `image-size' looks up and thus caches the
>             ;; untransformed image.  There's no easy way to
>             ;; prevent that.
>
> and the relevant code in image.c that verifies this.

We've previously discussed adding a parameter to image-size to disable
this caching.  Perhaps we should just do that?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51763: 27.2; Displaying many images take all memory

[Stefan Kangas]

Thierry Volpiatto <thievol@posteo.net> writes:

>> AFAIU, calling `clear-image-cache' should also the free memory unless we
>> have a memory leak.  You could also call this function from your code.
>> Does calling this function free the memory for you?
>
> Yes it does 🙂.  I didn't know this function (BTW what about renaming to
> `image-clear-cache`), works great with my code, perhaps you can do the
> same in dired (we use more or less similar code).

Great!  For now, this is just a temporary workaround for high memory
usage, so I'd personally be more inclined to work on the underlying
issue than adding it to Image-Dired.

I also believe that you will want to revisit the decision to use it once
this bug is fixed: it is a bit of a sledgehammer and can lead to
undesirable results (slowdowns).

As for renaming it to `image-clear-cache', I think it would be good but
let's see if anyone else thinks otherwise.

bug#51763: 27.2; Displaying many images take all memory

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> We've previously discussed adding a parameter to image-size to disable
> this caching.  Perhaps we should just do that?

I didn't follow those discussions, but that's what I think we should do
as well.

Should we perhaps also add a max cache size in addition to the
`image-cache-eviction-delay'?  I find it easy to get memory usage up to
several GiB when viewing large images, and all that happens in less than
five minutes.  (That would be a separate feature request though, as this
bug is about the memory usage regression introduced in Image-Dired.)

Relatedly, image-mode could be smarter, and evict images manually from
the cache.  Let's say that when flipping through images in a directory,
we only keep the previous N images (where N is, like, 5 or something).

bug#51763: 27.2; Displaying many images take all memory

[Thierry Volpiatto]

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


Stefan Kangas <stefan@marxist.se> writes:

> I also believe that you will want to revisit the decision to use it once
> this bug is fixed: it is a bit of a sledgehammer and can lead to
> undesirable results (slowdowns).

Yes, looking forward for a real fix.  Thanks.

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

bug#51763: 27.2; Displaying many images take all memory

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> Should we perhaps also add a max cache size in addition to the
> `image-cache-eviction-delay'?  I find it easy to get memory usage up to
> several GiB when viewing large images, and all that happens in less than
> five minutes.  (That would be a separate feature request though, as this
> bug is about the memory usage regression introduced in Image-Dired.)

The question is then how to tune it -- if it's mistuned, it may result
in catastrophic thrashing if you're showing a buffer with a gazillion
images...

> Relatedly, image-mode could be smarter, and evict images manually from
> the cache.  Let's say that when flipping through images in a directory,
> we only keep the previous N images (where N is, like, 5 or something).

Yes, that sounds like a good idea.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51763: 27.2; Displaying many images take all memory

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> The question is then how to tune it -- if it's mistuned, it may result
> in catastrophic thrashing if you're showing a buffer with a gazillion
> images...

I don't think we evict images from the cache if they are being displayed
though, do we?  I imagine that we would evict them if they haven't been
displayed for more than N seconds *and* we are above the max memory
threshold.  Where N is some new variable or constant smaller than
'image-cache-eviction-delay'.

bug#51763: 27.2; Displaying many images take all memory

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

>> The question is then how to tune it -- if it's mistuned, it may result
>> in catastrophic thrashing if you're showing a buffer with a gazillion
>> images...
>
> I don't think we evict images from the cache if they are being displayed
> though, do we?  I imagine that we would evict them if they haven't been
> displayed for more than N seconds *and* we are above the max memory
> threshold.  Where N is some new variable or constant smaller than
> 'image-cache-eviction-delay'.

Right, that could work, I think...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51763: 27.2; Displaying many images take all memory

[Thierry Volpiatto]

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


Stefan Kangas <stefan@marxist.se> writes:

> Relatedly, image-mode could be smarter, and evict images manually from
> the cache.  Let's say that when flipping through images in a directory,
> we only keep the previous N images (where N is, like, 5 or something).

I did this in helm and it works fine.

Also in image-dired when images are too large emacs ask for something
like "file too large really open yes no etc...", this because you use
`find-file-other-window` instead of `find-file-noselect`, do you want me
to open another issue for this? (patch below otherwise):

diff --git a/lisp/image-dired.el b/lisp/image-dired.el
index a2c37f00f23..fed23fb1830 100644
--- a/lisp/image-dired.el
+++ b/lisp/image-dired.el
@@ -1942,10 +1942,11 @@ based on `image-mode'."
         (cur-win (selected-window)))
     (when buf
       (kill-buffer buf))
-    (when-let ((buf (find-file-other-window file)))
+    (when-let ((buf (find-file-noselect file t)))
       (display-buffer buf)
-      (rename-buffer image-dired-display-image-buffer)
-      (image-dired-display-image-mode)
+      (with-current-buffer buf
+        (rename-buffer image-dired-display-image-buffer)
+        (image-dired-display-image-mode))
       (select-window cur-win))))
 
 (defun image-dired-display-thumbnail-original-image (&optional arg)


-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]