bug#45224: 28.0.50; eww and GIFS (cpu usage shoots through the roof)

[Lars Ingebrigtsen]

Stefan Kangas <stefan@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