[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#41200: Displaying a tooltip with x-show-tip gets very slow as more f
From: |
Stefan Monnier |
Subject: |
bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined |
Date: |
Fri, 15 May 2020 10:03:44 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
> Indeed, you're completely right; thanks! Replacing face_alist and
> Vface_new_frame_defaults with hash tables makes the worst example
> about 10 times faster, and with that change tooltips now take 30 to
> 50ms to display instead of 500-600ms in my real-life use case (my
> usual config). I have attached a patch.
Oh, this is great, makes a very noticeable difference.
I'm hoping it can make it into Emacs-27.1, tho it is probably rather
late for such a significant change.
> * I removed the function frame-face-alist and changed the type of the
> variable face-new-frame-defaults. Both were documented as internal.
> Should I add an ELisp implementation of frame-face-alist for
> compatibility? (It wouldn't be a perfect shim, since modifying its
> return value wouldn't do the same). For face-new-frame-defaults it's
> a bit trickier, since the variable now holds a hash table.
> Should I change its name to make the change obvious, at least?
The variable's name did not say "alist", so I don't see a need to change
it from that point of view. But I think it deserves a "--" since it's
supposed to be internal.
A quick grep revealed:
elpa/packages/context-coloring/fixtures/benchmark/faces.el: (mapcar #'car
face-new-frame-defaults))
That doesn't seem very serious, but I haven't grep'd the MELPA packages.
I saw no such comparable use of `frame-face-alist` in the wild.
> * The name face_hash isn't ideal, since there's already a distinct
> notion of face hashes (hash codes). Can you think of a better name?
Yes: it's not a hash, it's a table.
I think the better names refer to the "conceptual" type rather than the
specific implementation type, so I prefer "map" or "table" to "hash",
"alist", "obarray", and whatnot.
> - command-execute 454 47%
[...]
> - face-set-after-frame-default 387 40%
> - face-spec-recalc 374 39%
> - make-face-x-resource-internal 296 30%
> - set-face-attributes-from-resources 273 28%
> - set-face-attribute-from-resource 219 22%
[...]
> - command-execute 768 80%
[...]
> - face-set-after-frame-default 674 70%
> - face-spec-recalc 660 69%
> - face-spec-set-2 350 36%
> - apply 348 36%
> - set-face-attribute 342 35%
> - internal-set-lisp-face-attribute 342 35%
> - frame-set-background-mode 331 34%
> - face-spec-recalc 284 29%
> - make-face-x-resource-internal 235 24%
Both of those profiles suggest that most of the time is still spent in
`face-spec-recalc`, so it would be worth trying harder to avoid calling
it or to speed it up somehow (presumably with some better memozing/caching).
The first profile also suggests that we spend too much time in
`make-face-x-resource-internal`.
What caught my eye in the second profile is the fact that we call
`face-spec-recalc` recursively. I suspect that recursion is not desired.
Stefan
PS: Maybe another way to speed this up is to do the `face-spec-recalc`
lazily (i.e. only when we encounter the face during redisplay), but
that's probably harder to do and not as good (since it will still be
slow in the case where the frame happens to display hundreds of faces).
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, (continued)
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Clément Pit-Claudel, 2020/05/15
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Juri Linkov, 2020/05/16
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Clément Pit-Claudel, 2020/05/16
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Juri Linkov, 2020/05/17
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Clément Pit-Claudel, 2020/05/17
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Juri Linkov, 2020/05/19
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Eli Zaretskii, 2020/05/23
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Juri Linkov, 2020/05/23
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Eli Zaretskii, 2020/05/23
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Juri Linkov, 2020/05/24
bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined,
Stefan Monnier <=