GNU bug report logs -
#45224
28.0.50; eww and GIFS (cpu usage shoots through the roof)
Previous Next
Reported by: Madhavan Krishnan <krishnanmadhavan000 <at> gmail.com>
Date: Sun, 13 Dec 2020 17:25:01 UTC
Severity: normal
Tags: confirmed
Found in version 28.0.50
Fixed in version 29.1
Done: Lars Ingebrigtsen <larsi <at> gnus.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45224 in the body.
You can then email your comments to 45224 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Sun, 13 Dec 2020 17:25:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Madhavan Krishnan <krishnanmadhavan000 <at> gmail.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sun, 13 Dec 2020 17:25:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Hello there,
This is on Emacs 28.0.50 (MAIN branch) with "-Q"
Steps to reproduce the bug:
1) M-x eww 2) Insert this url at the prompt.
https://github.com/manateelazycat/emacs-application-framework
as "fix" I was told (setq shr-image-animate nil) would help, but would
defeat the purpose of GIFS
Below you can find the profile report for the cpu.
#+begin_example
- command-execute 4514 91%
- call-interactively 4514 91%
- byte-code 4197 85%
- read-extended-command 4197 85%
- completing-read 4197 85%
- completing-read-default 4197 85%
- read-from-minibuffer 4135 83%
- timer-event-handler 3423 69%
- apply 3423 69%
- image-animate-timeout 3421 69%
image-multi-frame-p 3420 69%
time-since 1 0%
#<compiled 0xbc1b6d5890d29> 1 0%
- url-http-generic-filter 91 1%
- url-http-content-length-after-change-function 89 1%
- url-http-activate-callback 81 1%
- apply 81 1%
- url-queue-callback-function 81 1%
- apply 81 1%
+ shr-image-fetched 81 1%
- file-size-human-readable-iec 6 0%
file-size-human-readable 3 0%
apply 2 0%
- redisplay_internal (C function) 39 0%
- funcall 35 0%
- #<compiled 0x1c38e9a167702bb5> 35 0%
+ gui-backend-selection-exists-p 35 0%
- tool-bar-make-keymap 2 0%
- tool-bar-make-keymap-1 2 0%
- mapcar 2 0%
#<compiled 0xd4d9e7eb5da225a> 2 0%
+ #<compiled 0x37cabbbdf65a500> 2 0%
- funcall-interactively 317 6%
- execute-extended-command 317 6%
- sit-for 317 6%
- read-event 174 3%
- timer-event-handler 161 3%
+ apply 161 3%
- url-http-generic-filter 2 0%
- url-http-content-length-after-change-function 2 0%
url-percentage 1 0%
- redisplay_internal (C function) 1 0%
+ #<compiled 0x37cabbbdf65a500> 1 0%
- input-pending-p 108 2%
- timer-event-handler 108 2%
- apply 108 2%
- image-animate-timeout 108 2%
image-multi-frame-p 108 2%
+ redisplay 27 0%
+ timer-event-handler 348 7%
+ ... 71 1%
+ redisplay_internal (C function) 2 0%
+ url-http-generic-filter 1 0%
#+end_example
Regards
--
Madhavan Krishnan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Mon, 14 Dec 2020 17:06:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 45224 <at> debbugs.gnu.org (full text, mbox):
Madhavan Krishnan <krishnanmadhavan000 <at> gmail.com> writes:
> Steps to reproduce the bug:
>
> 1) M-x eww 2) Insert this url at the prompt.
> https://github.com/manateelazycat/emacs-application-framework
Yes, Emacs's animation code needs to cache the images better.
In any case, Emacs should be stopping animations that take too much CPU.
Do you get the "Stopping animation; animation possibly too big"
messages?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Added tag(s) confirmed.
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org.
(Mon, 14 Dec 2020 17:07:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Tue, 15 Dec 2020 05:42:02 GMT)
Full text and
rfc822 format available.
Message #13 received at 45224 <at> debbugs.gnu.org (full text, mbox):
(Please keep the debbugs address in the Cc headers -- otherwise your
mails won't reach the Emacs bug tracker.)
Madhavan Krishnan <krishnanmadhavan000 <at> gmail.com> writes:
>>> In any case, Emacs should be stopping animations that take too much CPU.
>>> Do you get the "Stopping animation; animation possibly too big"
>>> messages?
>>
>> Nope, I was hardly able to scroll to the end of the page, at any rate my
>> cpu fan kicked in and I killed the entire session.
>
> My bad, I miss spoke yes the message does show "Stopping animation;
> animation possibly too big" (when you scroll the buffer message is gone)
> but the cpu usage is still not down (untill the buffer is killed
> manually)
I see Emacs gradually killing more and more of the animations until
Emacs has reached a usable state again, but it takes a while. Perhaps
it should be more aggressive in stopping the animations...
Anyway, that's a side issue -- this should really be fixed by making the
GIF animations faster. I'm not sure whether there's been any work done
on that -- it's been mentioned a few times, but possibly nobody has done
the work?
If I remember correctly, the current code will decode the entire GIF
file for each frame, which is pretty pessimal. The ImageMagick version
of the animation code keeps a special cache to avoid doing all those
decodings, so perhaps that code can be reused...
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Fri, 18 Dec 2020 09:40:01 GMT)
Full text and
rfc822 format available.
Message #16 received at 45224 <at> debbugs.gnu.org (full text, mbox):
On 2020-12-15, 06:41 +0100, Lars Ingebrigtsen <larsi <at> gnus.org> wrote:
> I see Emacs gradually killing more and more of the animations until
> Emacs has reached a usable state again, but it takes a while. Perhaps
> it should be more aggressive in stopping the animations...
>
> Anyway, that's a side issue -- this should really be fixed by making the
> GIF animations faster. I'm not sure whether there's been any work done
> on that -- it's been mentioned a few times, but possibly nobody has done
> the work?
>
> If I remember correctly, the current code will decode the entire GIF
> file for each frame, which is pretty pessimal. The ImageMagick version
> of the animation code keeps a special cache to avoid doing all those
> decodings, so perhaps that code can be reused...
Thank you for investigating this issue, I am not entirely sure of what
the next step would be. Is there a sample code that I can refer to for
the ImageMagic version you are referring to? (to get some idea about how
it can be patched)
Regards
--
Madhavan Krishnan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Fri, 18 Dec 2020 09:49:02 GMT)
Full text and
rfc822 format available.
Message #19 received at 45224 <at> debbugs.gnu.org (full text, mbox):
Madhavan Krishnan <krishnanmadhavan000 <at> gmail.com> writes:
> Thank you for investigating this issue, I am not entirely sure of what
> the next step would be. Is there a sample code that I can refer to for
> the ImageMagic version you are referring to? (to get some idea about how
> it can be patched)
imagemagick_get_animation_cache and friends in src/image.c.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Sun, 31 Oct 2021 01:51:01 GMT)
Full text and
rfc822 format available.
Message #22 received at 45224 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Madhavan Krishnan <krishnanmadhavan000 <at> gmail.com> writes:
>
>> Thank you for investigating this issue, I am not entirely sure of what
>> the next step would be. Is there a sample code that I can refer to for
>> the ImageMagic version you are referring to? (to get some idea about how
>> it can be patched)
>
> imagemagick_get_animation_cache and friends in src/image.c.
I've been looking into this a bit, and here are my preliminary
conclusions. I hope it is clear, and appreciate any feedback!
The first part of the problem is making sure that we can cache animated
images, which currently never happens. Once that is in place, the
second part is to cache all animated images up-front. I only discuss
the first part below.
Besides the special cache for ImageMagick we also have tho one used for
*all* images. So my thinking is: why not use the "real" cache also for
animated images? Why implement a specialized cache just for that?
I believe it is possible.[1] And actually, the first question is: why
aren't they already cached? The reason is this: in search_image_cache
(image.c:1599), we use sxhash and Fequal to compare the Lisp image
descriptor (or "image specification" or "spec" as its called in image.c)
to the one we have in cache. But in an animated gif the image spec is
changed in image.el:
(image :type gif
:file "/some/file.gif"
:scale 1
:max-width 1248
:max-height 1321
:format nil
:transform-smoothing t
:animate-buffer "file.gif"
:animate-tardiness 0.9810895737588246
:animate-multi-frame-data (62 . 0.04)
:index 61)
The value of :animate-tardiness is updated in image.el on every
iteration. When image.el updates :animate-tardiness, it will never be
equal to something in the cache and we always get a cache miss!
I have attached a patch that will demonstrate how we get back caching by
simply disabling the updating of the image spec in image.el. This is
completely the wrong thing to do, but demonstrates what is going on.
With that patch, caching will be enabled, and you can see it the second
time you try to run an animated gif. (Use RET to start the animation.)
The correct fix here is to somehow *only* compare the relevant
attributes above, and ignore or leave out e.g. :animate-tardiness.
I have experimented with different approaches. My first idea was to
create a new comparison function "image_cache_equal" that basically only
compares the ones. But then the problem was that we use the "sxhash"
function to find which hash bucket to find the list in, so that didn't
help.
I can think of two main ways to do this:
A) We create a new stripped down Lisp plist containing only the
attributes we are interested in and use that to compare against the
cache. We obviously need to make sure the cache also contains only
stripped down list. This means we can keep using Fequal and sxhash,
but we also need to create a new list for *every cache lookup*!
B) We parse the relevant parts of the image spec into a C struct and
then use that struct for comparisons. It turns out we already have
most of the code in place to do this, see parse_image_spec
(image.c:898), so most of it should be some restructuring, and then
writing up a comparison function. With that in place, we would just
need to .
Option A) seems ugly to me: why would we be consing up Lisp lists on
such a low level, which also makes me worry about creating a lot of
unnecessary garbage. So I prefer Option B). The struct also seems more
clean to me. Perhaps there are some performance implications I'm not
thinking of? Or perhaps there is some even better way to do the cache
check than a struct, such as using the "image struct" directly?
A third alternative is to somehow change image.el to put this
information outside the image specifier, but that leaves unfixed a
rather subtle issue with caching. That issue may or may not bite
someone later.
Comments about all this is very welcome!
Footnotes:
[1] There is a comment in the ImageMagick cache saying: "We have to
maintain a cache separate from the image cache, because the images
may be scaled before display." However, this was written before we
had native image scaling, and actually it seems to me either incorrect,
based on the above, or correct but only applicable to
ImageMagick, or perhaps more likely, I am missing something important.
[0001-gif-load-debug.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Sun, 31 Oct 2021 01:56:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 45224 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Madhavan Krishnan <krishnanmadhavan000 <at> gmail.com> writes:
>
>> Thank you for investigating this issue, I am not entirely sure of what
>> the next step would be. Is there a sample code that I can refer to for
>> the ImageMagic version you are referring to? (to get some idea about how
>> it can be patched)
>
> imagemagick_get_animation_cache and friends in src/image.c.
I've been looking into this a bit, and here are my preliminary
conclusions. I hope it is clear, and appreciate any feedback!
The first part of the problem is making sure that we can cache animated
images, which currently never happens. Once that is in place, the
second part is to cache all animated images up-front. I only discuss
the first part below.
Besides the special cache for ImageMagick we also have tho one used for
*all* images. So my thinking is: why not use the "real" cache also for
animated images? Why implement a specialized cache just for that?
I believe it is possible.[1] And actually, the first question is: why
aren't they already cached? The reason is this: in search_image_cache
(image.c:1599), we use sxhash and Fequal to compare the Lisp image
descriptor (or "image specification" or "spec" as its called in image.c)
to the one we have in cache. But in an animated gif the image spec is
changed in image.el:
(image :type gif
:file "/some/file.gif"
:scale 1
:max-width 1248
:max-height 1321
:format nil
:transform-smoothing t
:animate-buffer "file.gif"
:animate-tardiness 0.9810895737588246
:animate-multi-frame-data (62 . 0.04)
:index 61)
The value of :animate-tardiness is updated in image.el on every
iteration. When image.el updates :animate-tardiness, it will never be
equal to something in the cache and we always get a cache miss!
I have attached a patch that will demonstrate how we get back caching by
simply disabling the updating of the image spec in image.el. This is
completely the wrong thing to do, but demonstrates what is going on.
With that patch, caching will be enabled, and you can see it the second
time you try to run an animated gif. (Use RET to start the animation.)
The correct fix here is to somehow *only* compare the relevant
attributes above, and ignore or leave out e.g. :animate-tardiness.
I have experimented with different approaches. My first idea was to
create a new comparison function "image_cache_equal" that basically
only compares the attributes we are interested. But then the problem
was that we use the "sxhash" function to find which hash bucket to
find the list in, so that didn't help much. We would also need to
replace the sxhash with something else. Perhaps sxhash_equal? Hmm.
So I thought of two other ways to go about this:
A) We create a new stripped down Lisp plist containing only the
attributes we are interested in and use that to compare against the
cache. We obviously need to make sure the cache also contains only
stripped down list. This means we can keep using Fequal and sxhash,
but we also need to create a new list for *every cache lookup*!
B) We parse the relevant parts of the image spec into a C struct and
then use that struct for comparisons. It turns out we already have
most of the code in place to do this, see parse_image_spec
(image.c:898), so most of it should be some restructuring, and then
writing up a comparison function. With that in place, we would just
need to .
Option A) seems ugly to me: why would we be consing up Lisp lists on
such a low level, which also makes me worry about creating a lot of
unnecessary garbage. So I prefer Option B). The struct also seems
more clean to me. Perhaps there are some performance implications I'm
not thinking of? Or perhaps there is some even better way to do the
cache check than a new struct, such as using the "image struct"
directly?
A third alternative is to somehow change image.el to put this
information outside the image specifier, but that leaves unfixed a
rather subtle issue with caching. That issue may or may not bite
someone later.
Comments about all this is very welcome! I'm basically at the point
where any approach I choose means investing a bunch of work, and it
would be useful with some feedback before I rush ahead and attempt any
of them. Perhaps someone here has an idea or hunch about which
approach might prove more fruitful.
Footnotes:
[1] There is a comment in the ImageMagick cache saying: "We have to
maintain a cache separate from the image cache, because the images
may be scaled before display." However, this was written before we
had native image scaling, and actually it seems to me either
incorrect, based on the above, or correct but only applicable to
ImageMagick, or, perhaps more likely, I am missing something
important.
[0001-gif-load-debug.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Sun, 31 Oct 2021 15:11:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 45224 <at> debbugs.gnu.org (full text, mbox):
Stefan Kangas <stefan <at> marxist.se> writes:
> The value of :animate-tardiness is updated in image.el on every
> iteration. When image.el updates :animate-tardiness, it will never be
> equal to something in the cache and we always get a cache miss!
Ahh! Yes, this has bit us before, I think (with image.el changing the
plist triggering recalculation unnecessarily. Uhm... 4aa4a8f9 for
instance.
> Option A) seems ugly to me: why would we be consing up Lisp lists on
> such a low level, which also makes me worry about creating a lot of
> unnecessary garbage. So I prefer Option B).
Yes, me too.
> The struct also seems more clean to me. Perhaps there are some
> performance implications I'm not thinking of? Or perhaps there is
> some even better way to do the cache check than a struct, such as
> using the "image struct" directly?
The problem is that it's not well-defined which elements in the plist
really affect display and which ones don't. If you change :max-width of
an image plist, then it should definitely affect display, but if you
change :gazonk, then it shouldn't.
So perhaps we should just document which elements that affect display,
and put those fields into the struct? (And then use the struct instead
of the plist for caching.) That way, if somebody adds a new field that
affects display, then they know to also update the struct.
> A third alternative is to somehow change image.el to put this
> information outside the image specifier, but that leaves unfixed a
> rather subtle issue with caching. That issue may or may not bite
> someone later.
That would be a simpler solution -- image.el could easily just use a
hash table to keep track of the data. But as you say, people will trip
over this elsewhere, too, because stashing data in the plist seems like
such an obvious thing to do.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Mon, 11 Apr 2022 12:39:02 GMT)
Full text and
rfc822 format available.
Message #31 received at 45224 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Yes, Emacs's animation code needs to cache the images better.
I've now fixed this in Emacs 29.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Mon, 11 Apr 2022 12:41:01 GMT)
Full text and
rfc822 format available.
Message #34 received at 45224 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> I've now fixed this in Emacs 29.
That is, using this test image:
https://c.tenor.com/V5YkL4e_LOIAAAAC/couple-cuddle.gif
Emacs 28.1 uses about 50% CPU when displaying that image on this laptop.
After the caching changes, Emacs uses about 5% CPU.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
bug marked as fixed in version 29.1, send any further explanations to
45224 <at> debbugs.gnu.org and Madhavan Krishnan <krishnanmadhavan000 <at> gmail.com>
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org.
(Mon, 11 Apr 2022 12:41:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Mon, 11 Apr 2022 16:50:02 GMT)
Full text and
rfc822 format available.
Message #39 received at 45224 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>> The struct also seems more clean to me. Perhaps there are some
>> performance implications I'm not thinking of? Or perhaps there is
>> some even better way to do the cache check than a struct, such as
>> using the "image struct" directly?
>
> The problem is that it's not well-defined which elements in the plist
> really affect display and which ones don't. If you change :max-width of
> an image plist, then it should definitely affect display, but if you
> change :gazonk, then it shouldn't.
And as I was looking at the image stuff anyway, I've now gone ahead and
added filtering for the animation elements from the image cache.
In my test GIF image, Emacs used to use 30% CPU when animating it.
Creating the GIF cache took that down to 3%. Fixing the first-level
image cache takes that down to 1%.
So finally Emacs should be usable when displaying a buffer with a bunch
of animated images. :-)
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#45224; Package
emacs.
(Mon, 11 Apr 2022 17:38:02 GMT)
Full text and
rfc822 format available.
Message #42 received at 45224 <at> debbugs.gnu.org (full text, mbox):
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Mon, 11 Apr 2022 18:49:46 +0200
> Cc: Madhavan Krishnan <krishnanmadhavan000 <at> gmail.com>, 45224 <at> debbugs.gnu.org
>
> And as I was looking at the image stuff anyway, I've now gone ahead and
> added filtering for the animation elements from the image cache.
>
> In my test GIF image, Emacs used to use 30% CPU when animating it.
> Creating the GIF cache took that down to 3%. Fixing the first-level
> image cache takes that down to 1%.
>
> So finally Emacs should be usable when displaying a buffer with a bunch
> of animated images. :-)
Great, thanks!
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Tue, 10 May 2022 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 3 years and 57 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.