GNU bug report logs -
#68006
30.0.50; Image-mode speed
Previous Next
To reply to this bug, email your comments to 68006 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sun, 24 Dec 2023 16:45:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Manuel Giraud <manuel <at> ledu-giraud.fr>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sun, 24 Dec 2023 16:45:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Hi,
I was looking into performance issue using image-mode when going from
one image to the next/previous (with 'image-next/previous-file') and I
found those lines in "image-mode.el":
--8<---------------cut here---------------start------------->8---
;; Discard any stale image data before looking it up again.
(image-flush image)
(setq image (append image (image-transform-properties image)))
(setq props
`(display ,image
;; intangible ,image
rear-nonsticky (display) ;; intangible
read-only t front-sticky (read-only)))
--8<---------------cut here---------------end--------------->8---
The call to 'image-flush' defeats the entire purpose of having an image
cache. If I remove it, moving from previous/next is much more faster
(once this image is in the cache, of course). Do you know why this code
is like that?
Best regards,
In GNU Emacs 30.0.50 (build 2, x86_64-unknown-openbsd7.4) of 2023-12-24
built on computer
Repository revision: 13e46e2c1d33a3a48ecdcb56b745dbc53a4a3831
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101009
System Description: OpenBSD computer 7.4 GENERIC.MP#1549 amd64
Configured using:
'configure CC=egcc CPPFLAGS=-I/usr/local/include
LDFLAGS=-L/usr/local/lib MAKEINFO=gmakeinfo --prefix=/home/manuel/emacs
--bindir=/home/manuel/bin --with-x-toolkit=no --without-cairo
--without-dbus --without-gconf --without-gsettings --without-sound
--without-compress-install'
Configured features:
FREETYPE GIF GLIB GMP GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBXML2
MODULES NOTIFY KQUEUE OLDXMENU PDUMPER PNG RSVG SQLITE3 THREADS TIFF
TREE_SITTER WEBP X11 XDBE XFT XIM XINPUT2 XPM ZLIB
Important settings:
value of $LC_CTYPE: en_US.UTF-8
locale-coding-system: utf-8-unix
Major mode: ELisp/l
Minor modes in effect:
bug-reference-prog-mode: t
paredit-mode: t
display-time-mode: t
display-battery-mode: t
desktop-save-mode: t
server-mode: t
override-global-mode: t
repeat-mode: t
global-eldoc-mode: t
eldoc-mode: t
show-paren-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
minibuffer-regexp-mode: t
line-number-mode: t
transient-mark-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
Load-path shadows:
/home/manuel/.emacs.d/elpa/ef-themes-1.4.1/theme-loaddefs hides /home/manuel/emacs/share/emacs/30.0.50/lisp/theme-loaddefs
Features:
(shadow sort mail-extr emacsbug vc-annotate smerge-mode diff vc-bzr
vc-src vc-sccs vc-svn vc-cvs vc-rcs log-view pcvs-util whitespace
image-file image-converter org-agenda org-indent org-element org-persist
org-id avl-tree oc-basic ol-eww ol-rmail ol-mhe ol-irc ol-info ol-gnus
nnselect ol-docview doc-view image-mode exif ol-bibtex bibtex ol-bbdb
ol-w3m ol-doi org-link-doi gnus-icalendar org-capture org-refile org ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-src ob-comint
org-pcomplete org-list org-footnote org-faces org-entities ob-emacs-lisp
ob-core ob-eval org-cycle org-table ol org-fold org-fold-core org-keys
oc org-loaddefs org-version org-compat org-macs autorevert filenotify
vc-hg sh-script smie treesit executable eww url-queue mm-url mule-util
jka-compr on-screen vc-dir ewoc vc vc-git diff-mode vc-dispatcher
bug-reference paredit gnus-dired time battery cus-load desktop frameset
exwm-randr xcb-randr exwm-config ido exwm exwm-input xcb-keysyms xcb-xkb
exwm-manage exwm-floating xcb-cursor xcb-render exwm-layout
exwm-workspace exwm-core xcb-ewmh xcb-icccm xcb xcb-xproto xcb-types
xcb-debug server modus-operandi-theme modus-themes zone speed-type
url-http url-auth url-gw nsm compat ytdious mingus libmpdee reporter
edebug debug backtrace transmission color calc-bin calc-ext calc
calc-loaddefs rect calc-macs supercite regi ebdb-message ebdb-gnus
gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig
gnus-sum shr pixel-fill kinsoku url-file svg dom gnus-group gnus-undo
gnus-start gnus-dbus dbus xml gnus-cloud nnimap nnmail mail-source utf7
nnoo gnus-spec gnus-int gnus-range message sendmail yank-media puny
rfc822 mml mml-sec epa epg rfc6068 epg-config mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums gmm-utils
mailheader gnus-win ebdb-mua ebdb-com crm ebdb-format ebdb mailabbrev
eieio-opt speedbar ezimage dframe find-func eieio-base timezone
icalendar gnus nnheader gnus-util mail-utils range mm-util mail-prsvr
wid-edit web-mode derived disp-table erlang-start skeleton cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs slime-asdf grep slime-tramp tramp rx trampver tramp-integration
files-x tramp-message tramp-compat xdg shell pcomplete parse-time
iso8601 time-date format-spec tramp-loaddefs slime-fancy
slime-indentation slime-cl-indent cl-indent slime-trace-dialog
slime-fontifying-fu slime-package-fu slime-references
slime-compiler-notes-tree advice slime-scratch slime-presentations
bridge slime-macrostep macrostep slime-mdot-fu slime-enclosing-context
slime-fuzzy slime-fancy-trace slime-fancy-inspector slime-c-p-c
slime-editing-commands slime-autodoc slime-repl slime-parse slime
apropos compile text-property-search etags fileloop generator xref
project arc-mode archive-mode noutline outline icons pp comint ansi-osc
ansi-color ring hyperspec thingatpt slime-autoloads edmacro kmacro
use-package-bind-key bind-key appt diary-lib diary-loaddefs cal-menu
calendar cal-loaddefs pcase dired-x dired-aux dired dired-loaddefs
cl-extra help-mode use-package-core repeat easy-mmode debbugs-autoloads
ebdb-autoloads ef-themes-autoloads exwm-autoloads hyperbole-autoloads
magit-autoloads git-commit-autoloads magit-section-autoloads
dash-autoloads on-screen-autoloads osm-autoloads paredit-autoloads
rust-mode-autoloads speed-type-autoloads transmission-autoloads
with-editor-autoloads info compat-autoloads ytdious-autoloads package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache
json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs
cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd touch-screen tool-bar dnd fontset image regexp-opt
fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode
register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
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 emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads kqueue lcms2 dynamic-setting font-render-setting xinput2 x
multi-tty move-toolbar make-network-process emacs)
Memory information:
((conses 16 793954 635507) (symbols 48 52684 2)
(strings 32 255659 44607) (string-bytes 1 7175025)
(vectors 16 150516) (vector-slots 8 2617297 42627)
(floats 8 567 3230) (intervals 56 20138 6985) (buffers 992 49))
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sun, 24 Dec 2023 17:03:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 24 Dec 2023 17:44:37 +0100
> From: Manuel Giraud via "Bug reports for GNU Emacs,
> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>
> I was looking into performance issue using image-mode when going from
> one image to the next/previous (with 'image-next/previous-file') and I
> found those lines in "image-mode.el":
>
> --8<---------------cut here---------------start------------->8---
> ;; Discard any stale image data before looking it up again.
> (image-flush image)
> (setq image (append image (image-transform-properties image)))
> (setq props
> `(display ,image
> ;; intangible ,image
> rear-nonsticky (display) ;; intangible
> read-only t front-sticky (read-only)))
> --8<---------------cut here---------------end--------------->8---
>
> The call to 'image-flush' defeats the entire purpose of having an image
> cache. If I remove it, moving from previous/next is much more faster
> (once this image is in the cache, of course). Do you know why this code
> is like that?
Look at its Git history, and look up discussions and bugs from those
times.
AFAIR, one problem is that images can change behind our backs. But
maybe there are other reasons.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 25 Dec 2023 10:36:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
[...]
>> The call to 'image-flush' defeats the entire purpose of having an image
>> cache. If I remove it, moving from previous/next is much more faster
>> (once this image is in the cache, of course). Do you know why this code
>> is like that?
>
> Look at its Git history, and look up discussions and bugs from those
> times.
>
> AFAIR, one problem is that images can change behind our backs. But
> maybe there are other reasons.
So I trace this call back to dc255b24e30a (Chong Yidong, 2007-05-21). At
that time 'image-flush' was called 'image-refresh' with the same
functionality of uncaching an image given its spec.
Before that, the whole image cache was cleared (fcfc4732e0d9, Chong
Yidong, 2006-02-09). I did not find a bug reference and I still don't
understand why it is/was needed.
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 25 Dec 2023 13:37:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: 68006 <at> debbugs.gnu.org
> Date: Mon, 25 Dec 2023 11:34:49 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > Look at its Git history, and look up discussions and bugs from those
> > times.
> >
> > AFAIR, one problem is that images can change behind our backs. But
> > maybe there are other reasons.
>
> So I trace this call back to dc255b24e30a (Chong Yidong, 2007-05-21). At
> that time 'image-flush' was called 'image-refresh' with the same
> functionality of uncaching an image given its spec.
>
> Before that, the whole image cache was cleared (fcfc4732e0d9, Chong
> Yidong, 2006-02-09). I did not find a bug reference and I still don't
> understand why it is/was needed.
I think the reason is what I mentioned above.
You seem to interpret the purpose of the image-cache differently from
its real purpose. The real purpose of the image cache is not to hold
the image in memory for displaying it again and again, the purpose is
to allow the display engine to generate the pixels once, then reuse
those pixels during the current redisplay cycle or a few following
redisplay cycles. That's why we can so easily evict images from the
cache when some time has passed: the cache is ephemeral anyway, and is
intended to serve only as a short-term memory.
When working with external image files, flushing the image cache is a
good measure to make sure we always display the file on disk, not some
previous copy of it.
If you have a specific application in mind where this cache must be
kept for longer times, we could perhaps arrange for that, but we will
then also need to make sure images are not evicted from the cache
during that time.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 25 Dec 2023 19:00:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
[...]
> I think the reason is what I mentioned above.
>
> You seem to interpret the purpose of the image-cache differently from
> its real purpose. The real purpose of the image cache is not to hold
> the image in memory for displaying it again and again, the purpose is
> to allow the display engine to generate the pixels once, then reuse
> those pixels during the current redisplay cycle or a few following
> redisplay cycles. That's why we can so easily evict images from the
> cache when some time has passed: the cache is ephemeral anyway, and is
> intended to serve only as a short-term memory.
Ok, I guess I understand this purpose...
> When working with external image files, flushing the image cache is a
> good measure to make sure we always display the file on disk, not some
> previous copy of it.
... but then this does not seem like a good fit for the usage in
image-mode. When using 'image-next-file'/'image-previous-file', I
imagine that the idea is browse some pictures seamlessly. Removing an
image from the cache each time seems to go against this.
About being sure to display the file on disk, maybe we could call
'image-flush' only if the file has changed since its display. WDYT?
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 25 Dec 2023 19:32:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: 68006 <at> debbugs.gnu.org
> Date: Mon, 25 Dec 2023 19:59:10 +0100
>
> About being sure to display the file on disk, maybe we could call
> 'image-flush' only if the file has changed since its display. WDYT?
Provided that the check is reliable, I guess so.
In any case, I think we should be cautious and leave a knob to get
back the old behavior, in case there are some situations we don't
anticipate that need to flush the caches.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 26 Dec 2023 17:17:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: 68006 <at> debbugs.gnu.org
> Date: Tue, 26 Dec 2023 15:45:11 +0100
>
> >> About being sure to display the file on disk, maybe we could call
> >> 'image-flush' only if the file has changed since its display. WDYT?
> >
> > Provided that the check is reliable, I guess so.
> >
> > In any case, I think we should be cautious and leave a knob to get
> > back the old behavior, in case there are some situations we don't
> > anticipate that need to flush the caches.
>
> Hi,
>
> What do you think of this? Of course, it will need a NEWS entry but I
> wanted to polish it first. I have made it opt-in. I have used it a bit
> the cache could grow fast but I find it quite pleasant to use (for
> Docview also).
Using img->timestamp is not reliable enough, since that timestamp is
updated each time we call prepare_image_for_display, which can happen
many times during a session for the same image, and not necessarily
for actually displaying the image in a window. For example, AFAICT if
you move across an image with C-n/C-p, we update the time stamp each
time vertical motion crosses the screen line with the image. So I
think we'd need to store the file's time stamp or some other
signature.
The comparison with times-less-p is also risky: what if someone
replaces the image file with an older file?
I'd trust some kind of file checksum better, which we will have to
store alongside the image spec or as part of it. (Don't we already do
something like that somewhere in image-*.el files?)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 26 Dec 2023 17:32:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 68006 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: 68006 <at> debbugs.gnu.org
>> Date: Mon, 25 Dec 2023 19:59:10 +0100
>>
>> About being sure to display the file on disk, maybe we could call
>> 'image-flush' only if the file has changed since its display. WDYT?
>
> Provided that the check is reliable, I guess so.
>
> In any case, I think we should be cautious and leave a knob to get
> back the old behavior, in case there are some situations we don't
> anticipate that need to flush the caches.
Hi,
What do you think of this? Of course, it will need a NEWS entry but I
wanted to polish it first. I have made it opt-in. I have used it a bit
the cache could grow fast but I find it quite pleasant to use (for
Docview also).
[0001-Add-some-user-control-on-image-caching.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 26 Dec 2023 18:08:01 GMT)
Full text and
rfc822 format available.
Message #29 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
[...]
> Using img->timestamp is not reliable enough, since that timestamp is
> updated each time we call prepare_image_for_display, which can happen
> many times during a session for the same image, and not necessarily
> for actually displaying the image in a window. For example, AFAICT if
> you move across an image with C-n/C-p, we update the time stamp each
> time vertical motion crosses the screen line with the image. So I
> think we'd need to store the file's time stamp or some other
> signature.
Right.
> The comparison with times-less-p is also risky: what if someone
> replaces the image file with an older file?
I don't understand what the risk is here? It is just a cache.
> I'd trust some kind of file checksum better, which we will have to
> store alongside the image spec or as part of it. (Don't we already do
> something like that somewhere in image-*.el files?)
For image-dired, there is 'image-dired-contents-sha1'. Maybe this the
one you thought about?
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 26 Dec 2023 18:44:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: 68006 <at> debbugs.gnu.org
> Date: Tue, 26 Dec 2023 19:07:00 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > The comparison with times-less-p is also risky: what if someone
> > replaces the image file with an older file?
>
> I don't understand what the risk is here? It is just a cache.
The risk is that you won't notice the fact that the image file was
replaced.
> > I'd trust some kind of file checksum better, which we will have to
> > store alongside the image spec or as part of it. (Don't we already do
> > something like that somewhere in image-*.el files?)
>
> For image-dired, there is 'image-dired-contents-sha1'. Maybe this the
> one you thought about?
Probably.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Wed, 27 Dec 2023 12:14:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
[...]
>> For image-dired, there is 'image-dired-contents-sha1'. Maybe this the
>> one you thought about?
>
> Probably.
Hi Eli,
I was comtemplating the idea of using 'image-dired-contents-sha1' to
ensure that the image contents has not changed on disk. But then I
asked myself if a function "give me a unique id of this file based on
its content" does not already exist in Emacs. I could not found one but
maybe we could create one (eg. file-unique-identifier)?
Or do you prefer to keep this into the image library? In which case,
i'll move this function to "image.el" or "image-mode.el".
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Wed, 27 Dec 2023 13:38:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: 68006 <at> debbugs.gnu.org
> Date: Wed, 27 Dec 2023 13:13:13 +0100
>
> I was comtemplating the idea of using 'image-dired-contents-sha1' to
> ensure that the image contents has not changed on disk. But then I
> asked myself if a function "give me a unique id of this file based on
> its content" does not already exist in Emacs. I could not found one but
> maybe we could create one (eg. file-unique-identifier)?
>
> Or do you prefer to keep this into the image library? In which case,
> i'll move this function to "image.el" or "image-mode.el".
I think for now it should be kept in the image library, yes. I don't
quite see why it would be needed for other files (if it was, we would
have had it already by now).
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Fri, 29 Dec 2023 11:12:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 68006 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: 68006 <at> debbugs.gnu.org
>> Date: Wed, 27 Dec 2023 13:13:13 +0100
>>
>> I was comtemplating the idea of using 'image-dired-contents-sha1' to
>> ensure that the image contents has not changed on disk. But then I
>> asked myself if a function "give me a unique id of this file based on
>> its content" does not already exist in Emacs. I could not found one but
>> maybe we could create one (eg. file-unique-identifier)?
>>
>> Or do you prefer to keep this into the image library? In which case,
>> i'll move this function to "image.el" or "image-mode.el".
>
> I think for now it should be kept in the image library, yes. I don't
> quite see why it would be needed for other files (if it was, we would
> have had it already by now).
Hi Eli,
What about this new patch? It works for me as intended but might need
better names and better docs. Thanks.
[0001-Add-some-user-control-in-image-mode-caching.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Fri, 29 Dec 2023 12:14:01 GMT)
Full text and
rfc822 format available.
Message #44 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: 68006 <at> debbugs.gnu.org
> Date: Fri, 29 Dec 2023 12:11:44 +0100
>
> What about this new patch? It works for me as intended but might need
> better names and better docs. Thanks.
>
> +(defcustom image-mode-eager-cache-flush t
> + "Non-nil means flush image from cache eagerly.
> +When set to nil, be aware that the image cache could grow fast
> +but an image previously displayed will show faster."
This doc string should explain the effects better, in terms of
user-facing behavior, not in terms of technical aspects of the code.
> - ;; Discard any stale image data before looking it up again.
> - (image-flush image)
> + (if image-mode-eager-cache-flush
> + ;; Discard any stale image data before looking it up again.
> + (image-flush image)
> + ;; Add a content based hash into image spec to be sure that the
> + ;; cache is updated should the on disk image change.
> + (when (and filename (file-exists-p filename))
> + (setq image (append image (list :hash (image-contents-sha1 filename))))))
> +
I'm probably missing something: how would this assure that if the
image file is replaced, we re-read it from disk?
> +(defun image-contents-sha1 (filename)
> + "Compute the SHA-1 of the first 4KiB of FILENAME's contents. The
> +first 4KiB does not represent the whole file but was chosen
> +because it is fast enough for interactive usage."
The first line of a doc string should be a single complete sentence.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sat, 30 Dec 2023 11:38:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
[...]
>> - ;; Discard any stale image data before looking it up again.
>> - (image-flush image)
>> + (if image-mode-eager-cache-flush
>> + ;; Discard any stale image data before looking it up again.
>> + (image-flush image)
>> + ;; Add a content based hash into image spec to be sure that the
>> + ;; cache is updated should the on disk image change.
>> + (when (and filename (file-exists-p filename))
>> + (setq image (append image (list :hash (image-contents-sha1 filename))))))
>> +
>
> I'm probably missing something: how would this assure that if the
> image file is replaced, we re-read it from disk?
The logic goes like this:
- I compute a new hash of the content on disk
- This hash is add to the spec
- Now a key in the image cache is computed for this spec
(i.e. filename, size and *hash* included)
- If the hash was changed the cache will miss and read the new image
content. If hash and size had not changed the cache will hit and
return the correct image.
Is it more clear? Or maybe I do not answer your question?
I'll try to address your other remarks in a new version of this patch.
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sat, 30 Dec 2023 12:38:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 68006 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: 68006 <at> debbugs.gnu.org
>> Date: Fri, 29 Dec 2023 12:11:44 +0100
>>
>> What about this new patch? It works for me as intended but might need
>> better names and better docs. Thanks.
>>
>> +(defcustom image-mode-eager-cache-flush t
>> + "Non-nil means flush image from cache eagerly.
>> +When set to nil, be aware that the image cache could grow fast
>> +but an image previously displayed will show faster."
>
> This doc string should explain the effects better, in terms of
> user-facing behavior, not in terms of technical aspects of the code.
And here is a new version of the patch. Maybe, we need a NEWS entry for
this too.
[0001-Add-some-user-control-in-image-mode-caching.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sat, 30 Dec 2023 23:58:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> gnu.org> writes:
> And here is a new version of the patch. Maybe, we need a NEWS entry for
> this too.
If we have an option, then yes we should add it to NEWS too.
But I'm not sure about this one:
Flushing the image cache all the time is fine for images that are 4KB in
size, but these days we routinely work with images that are several
megabytes in size (to give an idea, searching online tells me that
Twitter allows uploading photos up to 2 MB, Facebook up to 15 MB).
Furthermore, my experience with image-dired has taught me that Emacs is
terribly slow when compared to all other programs capable of viewing
images out there. So I think we should think a bit more about this one.
Taking a step back, why are images treated differently from other
buffers? If the risk is that the image changes on disk without us
noticing, then users should need to either run `revert-buffer' or enable
`auto-revert-mode'. If we are talking about images that are inline in a
buffer, the cache should be flushed only when the buffer itself is
reverted. What am I missing?
Lastly, I'm not sure about the "hash the first 4 KB of an image"
heuristic, for starters because images can be several megabytes in size.
Would it not be both more accurate and faster to check the mtime and/or
the size of the file? Or do we need different heuristic for different
image formats? (Maybe JPEG changes the first 4 KB on save, but I don't
think all bitmap formats do.) But wouldn't it be even better to use the
notify system when possible, like with `auto-revert-use-notify'?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sun, 31 Dec 2023 07:17:01 GMT)
Full text and
rfc822 format available.
Message #56 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Sat, 30 Dec 2023 15:57:28 -0800
> Cc: 68006 <at> debbugs.gnu.org
>
> Taking a step back, why are images treated differently from other
> buffers? If the risk is that the image changes on disk without us
> noticing, then users should need to either run `revert-buffer' or enable
> `auto-revert-mode'. If we are talking about images that are inline in a
> buffer, the cache should be flushed only when the buffer itself is
> reverted. What am I missing?
See my explanation of the purpose of this particular cache here:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68006#14
In a nutshell, this cache is ephemeral anyway, and will be flushed at
some arbitrary time whether we want it or not, because its purpose is
not what you think it is.
In any case, if you intend to not flush the cache at all, then what
does that mean for Emacs sessions running for days and weeks, let
alone months, on end? will they keep these images in memory forever?
Or should GC sometimes evict those images from the cache, and if so,
under what conditions?
As for the differences between images and other buffers, there are
some:
. many buffers are smaller than images nowadays
. buffers with images are in many cases showing text we are
interested in temporarily, like Web pages (program source code
rarely if ever includes images, right?)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 01 Jan 2024 10:11:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Stefan Kangas <stefankangas <at> gmail.com> writes:
> Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs <at> gnu.org> writes:
>
>> And here is a new version of the patch. Maybe, we need a NEWS entry for
>> this too.
>
> If we have an option, then yes we should add it to NEWS too.
>
> But I'm not sure about this one:
>
> Flushing the image cache all the time is fine for images that are 4KB in
> size, but these days we routinely work with images that are several
> megabytes in size (to give an idea, searching online tells me that
> Twitter allows uploading photos up to 2 MB, Facebook up to 15 MB).
Hi Stefan,
First, I want to say that here we're just talking about image-mode (and
its derivatives like docview) and how the image cache is managed in this
mode only.
"Flushing the image cache all the time" is what is done *now* in
image-mode. In fact, this new option permits to inhibit it.
> Furthermore, my experience with image-dired has taught me that Emacs is
> terribly slow when compared to all other programs capable of viewing
> images out there. So I think we should think a bit more about this
> one.
Yes and this slowness was what I wanted to speed up in the first place
in image-mode: moving from one image to the next is slower than any
other program. I know that working on the image cache is picking a
low-hanging fruit here but I'm not an expert in fast image processing.
> Taking a step back, why are images treated differently from other
> buffers? If the risk is that the image changes on disk without us
> noticing, then users should need to either run `revert-buffer' or enable
> `auto-revert-mode'. If we are talking about images that are inline in a
> buffer, the cache should be flushed only when the buffer itself is
> reverted. What am I missing?
I guess that makes sense but would it be easy to plug the
'revert-buffer' machinery into the image cache internals? I don't know.
> Lastly, I'm not sure about the "hash the first 4 KB of an image"
> heuristic, for starters because images can be several megabytes in size.
> Would it not be both more accurate and faster to check the mtime and/or
> the size of the file? Or do we need different heuristic for different
> image formats? (Maybe JPEG changes the first 4 KB on save, but I don't
> think all bitmap formats do.) But wouldn't it be even better to use the
> notify system when possible, like with `auto-revert-use-notify'?
You're right that this heuristic might not be the best one. I
repurposed it from image-dired when Eli suggested something content
based. Maybe we could have a modification time of the image into its
spec and compare it to said image file before display.
Best regards,
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 02 Jan 2024 00:20:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Sat, 30 Dec 2023 15:57:28 -0800
>> Cc: 68006 <at> debbugs.gnu.org
>>
>> Taking a step back, why are images treated differently from other
>> buffers? If the risk is that the image changes on disk without us
>> noticing, then users should need to either run `revert-buffer' or enable
>> `auto-revert-mode'. If we are talking about images that are inline in a
>> buffer, the cache should be flushed only when the buffer itself is
>> reverted. What am I missing?
>
> See my explanation of the purpose of this particular cache here:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68006#14
I read that, yes. Quoting what you wrote there, and I hope I'm quoting
the most relevant part:
> The real purpose of the image cache is not to hold the image in
> memory for displaying it again and again, the purpose is to allow
> the display engine to generate the pixels once, then reuse those
> pixels during the current redisplay cycle or a few following
> redisplay cycles.
Basically, I don't see how this difference relates to manually evicting
the cache or not.
> In a nutshell, this cache is ephemeral anyway, and will be flushed at
> some arbitrary time whether we want it or not, because its purpose is
> not what you think it is.
If it is flushed anyways, then that is exactly what we want here, I
think. The idea is to flush it less often, AFAIU.
> In any case, if you intend to not flush the cache at all, then what
> does that mean for Emacs sessions running for days and weeks, let
> alone months, on end? will they keep these images in memory forever?
> Or should GC sometimes evict those images from the cache, and if so,
> under what conditions?
Are you saying that, if this particular call is removed, we will never
flush the cache (i.e. we will have memory leaks)?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 02 Jan 2024 12:11:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Stefan Kangas <stefankangas <at> gmail.com> writes:
[...]
>> In a nutshell, this cache is ephemeral anyway, and will be flushed at
>> some arbitrary time whether we want it or not, because its purpose is
>> not what you think it is.
>
> If it is flushed anyways, then that is exactly what we want here, I
> think. The idea is to flush it less often, AFAIU.
Yes that's it: flush less often (at the expense of more used memory of
course)
>> In any case, if you intend to not flush the cache at all, then what
>> does that mean for Emacs sessions running for days and weeks, let
>> alone months, on end? will they keep these images in memory forever?
>> Or should GC sometimes evict those images from the cache, and if so,
>> under what conditions?
>
> Are you saying that, if this particular call is removed, we will never
> flush the cache (i.e. we will have memory leaks)?
I don't know what Eli had in mind but removing this particular call
would not remove the timely cache eviction mechanism (via
'image-cache-eviction-delay')
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 02 Jan 2024 12:50:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Mon, 1 Jan 2024 16:19:31 -0800
> Cc: manuel <at> ledu-giraud.fr, 68006 <at> debbugs.gnu.org
>
> > The real purpose of the image cache is not to hold the image in
> > memory for displaying it again and again, the purpose is to allow
> > the display engine to generate the pixels once, then reuse those
> > pixels during the current redisplay cycle or a few following
> > redisplay cycles.
>
> Basically, I don't see how this difference relates to manually evicting
> the cache or not.
My point was that you are expecting from this cache something that it
wasn't intended to provide. As a corollary, I think if we want to
speed up paging through images, we should design and implement a
separate cache, which is designed to support the use case of going
through many images one by one, back and forth.
> > In a nutshell, this cache is ephemeral anyway, and will be flushed at
> > some arbitrary time whether we want it or not, because its purpose is
> > not what you think it is.
>
> If it is flushed anyways, then that is exactly what we want here, I
> think. The idea is to flush it less often, AFAIU.
My point is that you cannot currently control when it is flushed, not
fully anyway. Again, the reason is that the cache was not for the
purpose we need caching images in the case that's being discussed
here.
> > In any case, if you intend to not flush the cache at all, then what
> > does that mean for Emacs sessions running for days and weeks, let
> > alone months, on end? will they keep these images in memory forever?
> > Or should GC sometimes evict those images from the cache, and if so,
> > under what conditions?
>
> Are you saying that, if this particular call is removed, we will never
> flush the cache
Not just this call: you'd need to remove or disable the code which
evicts images from the cache "when the display engine feels like it".
And if you do that, then yes, the images will never be flushed, since
there will be nothing the flushes the cache.
> (i.e. we will have memory leaks)?
Not a memory leak, but memory that is never released.
Basically, we lack a good way of knowing that a given cached image is
no longer needed. The cache as it exists now doesn't care, since it
isn't supposed to hold the image long enough for that to matter; but
if you want a tighter control on when an image is uncached, we need to
design and implement a way for that, and the main stumbling block in
that case is how to define conditions under which an image can be
evicted from the cache.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 02 Jan 2024 16:05:01 GMT)
Full text and
rfc822 format available.
Message #71 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
[...]
> My point was that you are expecting from this cache something that it
> wasn't intended to provide. As a corollary, I think if we want to
> speed up paging through images, we should design and implement a
> separate cache, which is designed to support the use case of going
> through many images one by one, back and forth.
Do you think we need a completely different cache or just some handles
on the current one?
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 02 Jan 2024 17:03:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>, 68006 <at> debbugs.gnu.org
> Date: Tue, 02 Jan 2024 17:04:09 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > My point was that you are expecting from this cache something that it
> > wasn't intended to provide. As a corollary, I think if we want to
> > speed up paging through images, we should design and implement a
> > separate cache, which is designed to support the use case of going
> > through many images one by one, back and forth.
>
> Do you think we need a completely different cache or just some handles
> on the current one?
A different cache, I think.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Thu, 04 Jan 2024 16:48:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: Stefan Kangas <stefankangas <at> gmail.com>, 68006 <at> debbugs.gnu.org
>> Date: Tue, 02 Jan 2024 17:04:09 +0100
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> > My point was that you are expecting from this cache something that it
>> > wasn't intended to provide. As a corollary, I think if we want to
>> > speed up paging through images, we should design and implement a
>> > separate cache, which is designed to support the use case of going
>> > through many images one by one, back and forth.
>>
>> Do you think we need a completely different cache or just some handles
>> on the current one?
>
> A different cache, I think.
Hi,
I've tried to think about it a bit but I don't understand why do you
think it should be a different one. With `image-cache-eviction-delay'
being 300 seconds by default, I think this is enough time for this usage
too.
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Thu, 04 Jan 2024 17:45:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Thu, 04 Jan 2024 17:47:42 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> Do you think we need a completely different cache or just some handles
> >> on the current one?
> >
> > A different cache, I think.
>
> Hi,
>
> I've tried to think about it a bit but I don't understand why do you
> think it should be a different one.
Because its purpose is different, and the conditions to flush are also
different.
> With `image-cache-eviction-delay' being 300 seconds by default, I
> think this is enough time for this usage too.
I'm not sure I understand why time since caching is relevant for the
purpose that you want to cache images. Suppose there's an image that
should be displayed during the entire session -- how does it make
sense to remove it from the cache after 5 minutes? OTOH, another
image that is displayed just once doesn't need to be cached for more
than a few seconds.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Thu, 04 Jan 2024 18:44:01 GMT)
Full text and
rfc822 format available.
Message #83 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
[...]
>> I've tried to think about it a bit but I don't understand why do you
>> think it should be a different one.
>
> Because its purpose is different, and the conditions to flush are also
> different.
>
>> With `image-cache-eviction-delay' being 300 seconds by default, I
>> think this is enough time for this usage too.
>
> I'm not sure I understand why time since caching is relevant for the
> purpose that you want to cache images. Suppose there's an image that
> should be displayed during the entire session -- how does it make
> sense to remove it from the cache after 5 minutes? OTOH, another
> image that is displayed just once doesn't need to be cached for more
> than a few seconds.
My idea was just to have image-mode to feel that it display images
faster. In the first time, by caching already displayed images and
maybe, in a second time, by doing some prefetching of nearby images.
For this usage, I thought that 5 minutes was also a quite good value.
But anyway, you're thinking about another image cache completely user
manageable? I guess it is a much more harder problem to tackle. For
starters, as the current image-mode is using the 'image' display
property which uses the current image cache, I imagine that it should
then be replaced by something else?
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Thu, 04 Jan 2024 18:57:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Thu, 04 Jan 2024 19:42:53 +0100
>
> But anyway, you're thinking about another image cache completely user
> manageable?
Yes.
> I guess it is a much more harder problem to tackle.
Probably. But anything else will be IMO a kludge. (We do have
kludges in Emacs, so we could have one here as well, of course. I
just thought that it would be good to avoid it if possible).
> For starters, as the current image-mode is using the 'image' display
> property which uses the current image cache, I imagine that it
> should then be replaced by something else?
Yes, probably. And if you are going for the simple solution, please
keep in mind that image-cache-eviction-delay is a global variable, and
the images are cached on a per-frame basis. So if image-mode enlarges
image-cache-eviction-delay, it will affect all the images, not just
those in image-mode buffers. One more reason why I think we should
provide a separate and differently managed cache for this purpose.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Thu, 04 Jan 2024 19:17:01 GMT)
Full text and
rfc822 format available.
Message #89 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
>> Date: Thu, 04 Jan 2024 19:42:53 +0100
>>
>> But anyway, you're thinking about another image cache completely user
>> manageable?
>
> Yes.
>
>> I guess it is a much more harder problem to tackle.
>
> Probably. But anything else will be IMO a kludge. (We do have
> kludges in Emacs, so we could have one here as well, of course. I
> just thought that it would be good to avoid it if possible).
Ok.
>> For starters, as the current image-mode is using the 'image' display
>> property which uses the current image cache, I imagine that it
>> should then be replaced by something else?
Thinking about that, maybe we could have this other cache on top of the
current one (and still on top of the image display property).
> Yes, probably. And if you are going for the simple solution, please
> keep in mind that image-cache-eviction-delay is a global variable, and
> the images are cached on a per-frame basis. So if image-mode enlarges
> image-cache-eviction-delay, it will affect all the images, not just
> those in image-mode buffers. One more reason why I think we should
> provide a separate and differently managed cache for this purpose.
Ok. Maybe we should re-use the current C cache code with a "nice"
interface for this too.
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Thu, 04 Jan 2024 19:56:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Thu, 04 Jan 2024 20:16:19 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > Yes, probably. And if you are going for the simple solution, please
> > keep in mind that image-cache-eviction-delay is a global variable, and
> > the images are cached on a per-frame basis. So if image-mode enlarges
> > image-cache-eviction-delay, it will affect all the images, not just
> > those in image-mode buffers. One more reason why I think we should
> > provide a separate and differently managed cache for this purpose.
>
> Ok. Maybe we should re-use the current C cache code with a "nice"
> interface for this too.
Yes, that can be done. But we might need to store additional
information there, like the buffer of the image, for example.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Fri, 05 Jan 2024 10:51:02 GMT)
Full text and
rfc822 format available.
Message #95 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
>> Date: Thu, 04 Jan 2024 20:16:19 +0100
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> > Yes, probably. And if you are going for the simple solution, please
>> > keep in mind that image-cache-eviction-delay is a global variable, and
>> > the images are cached on a per-frame basis. So if image-mode enlarges
>> > image-cache-eviction-delay, it will affect all the images, not just
>> > those in image-mode buffers. One more reason why I think we should
>> > provide a separate and differently managed cache for this purpose.
>>
>> Ok. Maybe we should re-use the current C cache code with a "nice"
>> interface for this too.
>
> Yes, that can be done.
Ok. More thinking: my idea is to have an interface to create/destroy
image caches and feed/query them. And why not have a Lisp interface to
it so something like image-mode could use this to manage its own cache.
> But we might need to store additional information there, like the
> buffer of the image, for example.
I don't understand. Where do you think that the buffer of the image
should be stored?
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Fri, 05 Jan 2024 11:26:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Fri, 05 Jan 2024 11:50:24 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> Ok. Maybe we should re-use the current C cache code with a "nice"
> >> interface for this too.
> >
> > Yes, that can be done.
>
> Ok. More thinking: my idea is to have an interface to create/destroy
> image caches and feed/query them. And why not have a Lisp interface to
> it so something like image-mode could use this to manage its own cache.
Yes, of course. We have a Lisp API for the existing cache as well,
but this new one will need it much more, since we basically rely on
the Lisp programs to manage this new cache.
> > But we might need to store additional information there, like the
> > buffer of the image, for example.
>
> I don't understand. Where do you think that the buffer of the image
> should be stored?
In the cache, together with the image, I think. How else would the
Lisp programs know which images "belong" to them, and when to uncache
them? I guess some other information relevant to the cache-removal
decision will also need to be stored there.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Fri, 05 Jan 2024 13:27:02 GMT)
Full text and
rfc822 format available.
Message #101 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
[...]
>> > But we might need to store additional information there, like the
>> > buffer of the image, for example.
>>
>> I don't understand. Where do you think that the buffer of the image
>> should be stored?
>
> In the cache, together with the image, I think.
You mean in struct image_cache?
> How else would the Lisp programs know which images "belong" to them,
> and when to uncache them?
I don't know exactly. My idea was that an image-mode buffer has (buffer
local I guess) variable that will hold its image cache.
> I guess some other information relevant to the cache-removal decision
> will also need to be stored there.
Yes, I imagine.
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Fri, 05 Jan 2024 13:41:02 GMT)
Full text and
rfc822 format available.
Message #104 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Fri, 05 Jan 2024 14:26:20 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> > But we might need to store additional information there, like the
> >> > buffer of the image, for example.
> >>
> >> I don't understand. Where do you think that the buffer of the image
> >> should be stored?
> >
> > In the cache, together with the image, I think.
>
> You mean in struct image_cache?
Yes.
> > How else would the Lisp programs know which images "belong" to them,
> > and when to uncache them?
>
> I don't know exactly. My idea was that an image-mode buffer has (buffer
> local I guess) variable that will hold its image cache.
That is also possible, but then will the cache itself be implemented
only in Lisp? I assumed at least some of it will be in C.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Fri, 05 Jan 2024 14:36:01 GMT)
Full text and
rfc822 format available.
Message #107 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
>> Date: Fri, 05 Jan 2024 14:26:20 +0100
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> > But we might need to store additional information there, like the
>> >> > buffer of the image, for example.
>> >>
>> >> I don't understand. Where do you think that the buffer of the image
>> >> should be stored?
>> >
>> > In the cache, together with the image, I think.
>>
>> You mean in struct image_cache?
>
> Yes.
>
>> > How else would the Lisp programs know which images "belong" to them,
>> > and when to uncache them?
>>
>> I don't know exactly. My idea was that an image-mode buffer has (buffer
>> local I guess) variable that will hold its image cache.
>
> That is also possible, but then will the cache itself be implemented
> only in Lisp? I assumed at least some of it will be in C.
I also imagine it would be mostly in C. But looking at the code, I
think I have overlooked something: lookup_image is what currently does
most of the work (with caching) and this is in called in many places in
"xdisp.c". Of course, I think we should keep that... but then how could
we design this new cache?
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Fri, 05 Jan 2024 14:43:01 GMT)
Full text and
rfc822 format available.
Message #110 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Fri, 05 Jan 2024 15:35:42 +0100
>
> I also imagine it would be mostly in C. But looking at the code, I
> think I have overlooked something: lookup_image is what currently does
> most of the work (with caching) and this is in called in many places in
> "xdisp.c". Of course, I think we should keep that... but then how could
> we design this new cache?
For example, lookup_image could look in both the existing and this new
cache.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Fri, 05 Jan 2024 14:56:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
>> Date: Fri, 05 Jan 2024 15:35:42 +0100
>>
>> I also imagine it would be mostly in C. But looking at the code, I
>> think I have overlooked something: lookup_image is what currently does
>> most of the work (with caching) and this is in called in many places in
>> "xdisp.c". Of course, I think we should keep that... but then how could
>> we design this new cache?
>
> For example, lookup_image could look in both the existing and this new
> cache.
Yes why not. So your idea is to have just one more other cache that we
can access differently than the current one. I was thinking more of
something to let Lisp programs create their own image cache but that may
be less practical.
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sat, 06 Jan 2024 13:08:01 GMT)
Full text and
rfc822 format available.
Message #116 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Hi,
Here is a diff of where I'm headed to. 'user-image-cache' is not used
at all yet. I imagine that a user could fill it with a call to
'make-image-cache' (when it exists). Anyway, WDYT?
diff --git a/src/image.c b/src/image.c
index 252b83da992..27d564fb49c 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2086,7 +2086,7 @@ image_alloc_image_color (struct frame *f, struct image *img,
Image Cache
***********************************************************************/
-static void cache_image (struct frame *f, struct image *img);
+static void cache_image (struct image_cache **pc, struct image *img);
/* Return a new, initialized image cache that is allocated from the
heap. Call free_image_cache to free an image cache. */
@@ -2103,15 +2103,14 @@ make_image_cache (void)
return c;
}
-/* Find an image matching SPEC in the cache, and return it. If no
+/* Find an image matching SPEC in the cache C, and return it. If no
image is found, return NULL. */
static struct image *
-search_image_cache (struct frame *f, Lisp_Object spec, EMACS_UINT hash,
+search_image_cache (struct image_cache *c, Lisp_Object spec, EMACS_UINT hash,
unsigned long foreground, unsigned long background,
int font_size, char *font_family, bool ignore_colors)
{
struct image *img;
- struct image_cache *c = FRAME_IMAGE_CACHE (f);
int i = hash % IMAGE_CACHE_BUCKETS_SIZE;
if (!c) return NULL;
@@ -2185,12 +2184,13 @@ uncache_image (struct frame *f, Lisp_Object spec)
{
struct image *img;
EMACS_UINT hash = sxhash (filter_image_spec (spec));
+ struct image_cache *cache = FRAME_IMAGE_CACHE (f);
/* Because the background colors are based on the current face, we
can have multiple copies of an image with the same spec. We want
to remove them all to ensure the user doesn't see an old version
of the image when the face changes. */
- while ((img = search_image_cache (f, spec, hash, 0, 0, 0, NULL, true)))
+ while ((img = search_image_cache (cache, spec, hash, 0, 0, 0, NULL, true)))
{
free_image (f, img);
/* As display glyphs may still be referring to the image ID, we
@@ -3359,6 +3359,7 @@ lookup_image (struct frame *f, Lisp_Object spec, int face_id)
unsigned long background = face->background;
int font_size = face->font->pixel_size;
char *font_family = SSDATA (face->lface[LFACE_FAMILY_INDEX]);
+ struct image_cache *cache;
/* F must be a window-system frame, and SPEC must be a valid image
specification. */
@@ -3367,7 +3368,13 @@ lookup_image (struct frame *f, Lisp_Object spec, int face_id)
/* Look up SPEC in the hash table of the image cache. */
hash = sxhash (filter_image_spec (spec));
- img = search_image_cache (f, spec, hash, foreground, background,
+
+ if (!NILP (Vuser_image_cache))
+ cache = XUNTAG (Vuser_image_cache, Lisp_Vectorlike, struct image_cache);
+ else
+ cache = FRAME_IMAGE_CACHE (f);
+
+ img = search_image_cache (cache, spec, hash, foreground, background,
font_size, font_family, false);
if (img && img->load_failed_p)
{
@@ -3380,7 +3387,7 @@ lookup_image (struct frame *f, Lisp_Object spec, int face_id)
{
block_input ();
img = make_image (spec, hash);
- cache_image (f, img);
+ cache_image (&cache, img);
img->face_foreground = foreground;
img->face_background = background;
img->face_font_size = font_size;
@@ -3470,16 +3477,17 @@ lookup_image (struct frame *f, Lisp_Object spec, int face_id)
}
-/* Cache image IMG in the image cache of frame F. */
+/* Cache image IMG in the image cache C. */
static void
-cache_image (struct frame *f, struct image *img)
+cache_image (struct image_cache **pc, struct image *img)
{
- struct image_cache *c = FRAME_IMAGE_CACHE (f);
+ struct image_cache *c;
ptrdiff_t i;
- if (!c)
- c = FRAME_IMAGE_CACHE (f) = make_image_cache ();
+ if (!*pc)
+ *pc = make_image_cache ();
+ c = *pc;
/* Find a free slot in c->images. */
for (i = 0; i < c->used; ++i)
@@ -12975,6 +12983,10 @@ syms_of_image (void)
The function `clear-image-cache' disregards this variable. */);
Vimage_cache_eviction_delay = make_fixnum (300);
+
+ DEFVAR_LISP ("user-image-cache", Vuser_image_cache,
+ doc: /* TBD. */);
+ Vuser_image_cache = Qnil;
#ifdef HAVE_IMAGEMAGICK
DEFVAR_INT ("imagemagick-render-type", imagemagick_render_type,
doc: /* Integer indicating which ImageMagick rendering method to use.
--
Manuel Giraud
Severity set to 'wishlist' from 'normal'
Request was from
Stefan Kangas <stefankangas <at> gmail.com>
to
control <at> debbugs.gnu.org.
(Wed, 10 Jan 2024 17:34:01 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Thu, 17 Oct 2024 09:52:02 GMT)
Full text and
rfc822 format available.
Message #121 received at 68006 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi,
I'm trying to revisit this idea of having a user controlled image cache.
The goal is to have, for instance, image-mode use it because (hopefully)
it would have a better idea of when to cache/uncache images for its
usage.
Here is a patch of such a prototype « user image cache ».
For the moment, I wanted to keep it minimal so the only interface to use
it goes as follow. When a user (or a program) use the image attribute
":ttl TIME_IN_SECONDS" with `create-image' such image will be stored in
the « user cache » and not in the internal one. Images lookups use both
caches (internal and internal, in that order). A negative
TIME_IN_SECONDS means « cache this image forever ».
I think there will be a need for user function that completely flushes
the images in this new cache. But do you think we need others commands
for such interface?
[0001-User-controlled-image-cache.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sat, 19 Oct 2024 09:35:02 GMT)
Full text and
rfc822 format available.
Message #124 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Thu, 17 Oct 2024 11:51:08 +0200
>
> I'm trying to revisit this idea of having a user controlled image cache.
> The goal is to have, for instance, image-mode use it because (hopefully)
> it would have a better idea of when to cache/uncache images for its
> usage.
>
> Here is a patch of such a prototype « user image cache ».
>
> For the moment, I wanted to keep it minimal so the only interface to use
> it goes as follow. When a user (or a program) use the image attribute
> ":ttl TIME_IN_SECONDS" with `create-image' such image will be stored in
> the « user cache » and not in the internal one. Images lookups use both
> caches (internal and internal, in that order). A negative
> TIME_IN_SECONDS means « cache this image forever ».
Why do we need a separate cache for this? couldn't we instead just add
the EOL field to the single cache we already have? having two caches
requires to search both of them, which slows down image look up, so if
we can avoid that, it's better.
> I think there will be a need for user function that completely flushes
> the images in this new cache. But do you think we need others commands
> for such interface?
We probably need a way for modifying the time an image is to be kept,
at least.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 21 Oct 2024 10:13:02 GMT)
Full text and
rfc822 format available.
Message #127 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
[...]
>> For the moment, I wanted to keep it minimal so the only interface to use
>> it goes as follow. When a user (or a program) use the image attribute
>> ":ttl TIME_IN_SECONDS" with `create-image' such image will be stored in
>> the « user cache » and not in the internal one. Images lookups use both
>> caches (internal and internal, in that order). A negative
>> TIME_IN_SECONDS means « cache this image forever ».
>
> Why do we need a separate cache for this? couldn't we instead just add
> the EOL field to the single cache we already have? having two caches
> requires to search both of them, which slows down image look up, so if
> we can avoid that, it's better.
That's you (in this thread) that suggested having another cache for more
"user controlled" usage and suggested that lookup_image could search in
both caches.
>> I think there will be a need for user function that completely flushes
>> the images in this new cache. But do you think we need others commands
>> for such interface?
>
> We probably need a way for modifying the time an image is to be kept,
> at least.
Yes. Anyway, my last patch was completely wrong and I am working on
another one. Tell me if you're interested to see where I'm headed to.
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 21 Oct 2024 10:35:02 GMT)
Full text and
rfc822 format available.
Message #130 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Mon, 21 Oct 2024 12:12:12 +0200
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> [...]
>
> >> For the moment, I wanted to keep it minimal so the only interface to use
> >> it goes as follow. When a user (or a program) use the image attribute
> >> ":ttl TIME_IN_SECONDS" with `create-image' such image will be stored in
> >> the « user cache » and not in the internal one. Images lookups use both
> >> caches (internal and internal, in that order). A negative
> >> TIME_IN_SECONDS means « cache this image forever ».
> >
> > Why do we need a separate cache for this? couldn't we instead just add
> > the EOL field to the single cache we already have? having two caches
> > requires to search both of them, which slows down image look up, so if
> > we can avoid that, it's better.
>
> That's you (in this thread) that suggested having another cache for more
> "user controlled" usage and suggested that lookup_image could search in
> both caches.
I said that assuming the difference will be more prominent than just
one field.
The implementation could be a single cache with a field or fields that
distinguish between the two kinds of cached images.
> >> I think there will be a need for user function that completely flushes
> >> the images in this new cache. But do you think we need others commands
> >> for such interface?
> >
> > We probably need a way for modifying the time an image is to be kept,
> > at least.
>
> Yes. Anyway, my last patch was completely wrong and I am working on
> another one. Tell me if you're interested to see where I'm headed to.
It's your call. I and others are here to provide feedback if you feel
you need that.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 21 Oct 2024 14:26:01 GMT)
Full text and
rfc822 format available.
Message #133 received at 68006 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
>> Date: Mon, 21 Oct 2024 12:12:12 +0200
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> [...]
>>
>> >> For the moment, I wanted to keep it minimal so the only interface to use
>> >> it goes as follow. When a user (or a program) use the image attribute
>> >> ":ttl TIME_IN_SECONDS" with `create-image' such image will be stored in
>> >> the « user cache » and not in the internal one. Images lookups use both
>> >> caches (internal and internal, in that order). A negative
>> >> TIME_IN_SECONDS means « cache this image forever ».
>> >
>> > Why do we need a separate cache for this? couldn't we instead just add
>> > the EOL field to the single cache we already have? having two caches
>> > requires to search both of them, which slows down image look up, so if
>> > we can avoid that, it's better.
>>
>> That's you (in this thread) that suggested having another cache for more
>> "user controlled" usage and suggested that lookup_image could search in
>> both caches.
>
> I said that assuming the difference will be more prominent than just
> one field.
>
> The implementation could be a single cache with a field or fields that
> distinguish between the two kinds of cached images.
Yes, that makes sense.
>> >> I think there will be a need for user function that completely flushes
>> >> the images in this new cache. But do you think we need others commands
>> >> for such interface?
>> >
>> > We probably need a way for modifying the time an image is to be kept,
>> > at least.
>>
>> Yes. Anyway, my last patch was completely wrong and I am working on
>> another one. Tell me if you're interested to see where I'm headed to.
>
> It's your call. I and others are here to provide feedback if you feel
> you need that.
Anyway, here is a patch of what I have done.
Since in the end an image is identify by an ID which is its index in the
images vector, I choose to take this vector out of the cache struct and
put it into what I called an image_store.
Now there are two caches (internal (as before) and user's) that both use
the image_store. In order to make this work, I needed to add a
reference to the cache that manage it in each image.
But, as you said, for just one other field, this code might be
over-engineered and also quite error-prone.
[0001-User-controlled-image-cache.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 21 Oct 2024 15:09:02 GMT)
Full text and
rfc822 format available.
Message #136 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Mon, 21 Oct 2024 16:25:22 +0200
>
> >> > We probably need a way for modifying the time an image is to be kept,
> >> > at least.
> >>
> >> Yes. Anyway, my last patch was completely wrong and I am working on
> >> another one. Tell me if you're interested to see where I'm headed to.
> >
> > It's your call. I and others are here to provide feedback if you feel
> > you need that.
>
> Anyway, here is a patch of what I have done.
>
> Since in the end an image is identify by an ID which is its index in the
> images vector, I choose to take this vector out of the cache struct and
> put it into what I called an image_store.
What advantages did you see in doing it this way, wrt the original
code?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Tue, 22 Oct 2024 16:30:02 GMT)
Full text and
rfc822 format available.
Message #139 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
>> Date: Mon, 21 Oct 2024 16:25:22 +0200
>>
>> >> > We probably need a way for modifying the time an image is to be kept,
>> >> > at least.
>> >>
>> >> Yes. Anyway, my last patch was completely wrong and I am working on
>> >> another one. Tell me if you're interested to see where I'm headed to.
>> >
>> > It's your call. I and others are here to provide feedback if you feel
>> > you need that.
>>
>> Anyway, here is a patch of what I have done.
>>
>> Since in the end an image is identify by an ID which is its index in the
>> images vector, I choose to take this vector out of the cache struct and
>> put it into what I called an image_store.
>
> What advantages did you see in doing it this way, wrt the original
> code?
I don't think there is any great advantage. I think I was more guide to
doing it this way because I thought I need to make it into another
(different) cache.
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Mon, 28 Oct 2024 14:33:01 GMT)
Full text and
rfc822 format available.
Message #142 received at 68006 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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?
[0001-User-defined-time-to-live-for-image.patch (text/x-patch, attachment)]
[0002-Try-it-in-image-mode.patch (text/x-patch, attachment)]
[Message part 4 (text/plain, inline)]
--
Manuel Giraud
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sat, 02 Nov 2024 11:09:02 GMT)
Full text and
rfc822 format available.
Message #145 received at 68006 <at> debbugs.gnu.org (full text, mbox):
> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
> Date: Mon, 28 Oct 2024 15:31:22 +0100
>
> 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)
The C part looks OK, but I don't understand how will this be used by
Lisp programs. We don't intend to use the new TTL member by default
each time we display an image, do we? So I think we need to analyze
the various uses of images and decide which ones will use TTL (and
what values to use in that case) and which won't.
I also envision a need to have a Lisp API which takes an existing
image and sets its TTL to a given value. And maybe other APIs as
well, I don't know. But without such APIs and without sensible
defaults about them, this features sounds incomplete.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#68006; Package
emacs.
(Sun, 03 Nov 2024 15:11:02 GMT)
Full text and
rfc822 format available.
Message #148 received at 68006 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Manuel Giraud <manuel <at> ledu-giraud.fr>
>> Cc: stefankangas <at> gmail.com, 68006 <at> debbugs.gnu.org
>> Date: Mon, 28 Oct 2024 15:31:22 +0100
>>
>> 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)
>
> The C part looks OK, but I don't understand how will this be used by
> Lisp programs. We don't intend to use the new TTL member by default
> each time we display an image, do we? So I think we need to analyze
> the various uses of images and decide which ones will use TTL (and
> what values to use in that case) and which won't.
Yes I thought that image-mode was a good candidate because I'm typically
using it on directory with lots of images that I'd like to browse (with
'n' and 'p'). But I imagine that this is not a use case for everybody.
In the same idea, I thought that DocView may also be a good candidate.
What I imagined (on the Lisp side) is that a program could load a subset
of images that are close to (in the sense of `image-next-file') the
current one with a TTL set.
> I also envision a need to have a Lisp API which takes an existing
> image and sets its TTL to a given value. And maybe other APIs as
> well, I don't know. But without such APIs and without sensible
> defaults about them, this features sounds incomplete.
Yes, sure there is a need for such Lisp API. I'll try to come up with
something.
--
Manuel Giraud
This bug report was last modified 223 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.