[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: |
Clément Pit-Claudel |
Subject: |
bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined |
Date: |
Fri, 15 May 2020 12:22:53 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
On 15/05/2020 11.17, Eli Zaretskii wrote:
>>>> * 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?
>>>
>>> face_hash_table?
>>
>> Thanks, good idea. I also liked Stefan's frame_face_map.
>
> "Map" is too general, IMO, it doesn't tell enough about the kind of
> object it is.
Got it. Is face_table better? (that was Stefan's other suggestion)
>>> And I'm not sure we should have it only in Lisp: perhaps we should
>>> maintain the alist as well, and add/remove to/from it when a face is
>>> added or removed in the hash table. Otherwise this change of
>>> internals will have painful effect on packages that use the current
>>> APIs.
>>
>> frame-face-alist is likely less crucial than face-new-frame-defaults,
>> because it was already a function, so the return value has to be mutated in
>> place to modify it (it couldn't be directly assigned). For both of these,
>> however, how would we ensure that the alist remains in sync with the hashmap
>> (that is, how do we catch modifications?)
>
> I thought about updating the alist when the hash-table is modified.
> Since you always know whether the face is already in the hash-table,
> you don't need to scan the alist looking for it.
Would that be done in C, or in any place where frame-new-face-defaults is
modified? (for example, edebug changes face-new-frame-defaults in one place;
if we keep the alist in addition to the hash table, would it modify both or
would there be a C mechanism that mirrors hash-table modifications to the
alist?)
> I'd really like to know that no one is using these before we make the
> final decision about this.
That's a fair point. While I couldn't find uses of frame-face-alist, there are
a few uses of face-new-frame-defaults-alist:
https://github.com/pestctrl/emacs-config/commit/31d6bff97dacf60f71066902a23d37e59b4c1288
is from someone who uses that to speed up temporary frame creation(!),
https://github.com/Lindydancer/face-explorer/blob/13bd4553bc4b09215a04d0267be1cb4ed834775c/test/face-explorer-test-faces.el
is from Anders Lindgren (in CC; hi Andres! We're considering replacing
face-new-frame-defaults-alist with a hash table, would that be an issue?), who
uses it to remove a face, and
https://github.com/Wilfred/elisp-fu/blob/92c491584f466ee729ea1b328234636e65e2c665/elisp-fu.el
includes code that's the same as in eval-defun.
I have a variant of the patch that keeps the alist variable, but I fear that
it's worse than removing it: since changes to the alist won't be propagated to
the hash table, it might be better to error out with a compilation error?
Btw, I have one more question: some callers of face-list seems to rely on the
order of faces added to it, so it would be better to preserve that order.
Since hash-tables are not necessarily ordered, should I sort faces by face-id
before returning them?
>> If so, where from?
>
> I think the problem is that tab-line is declared a basic face, but its
> defface form is not in faces.el.
Ah, good catch. Current there's a defface for tab-bar in lisp/tab-bar, and
since that's preloaded it works, but the defface for tab-line is in
lisp/tab-line.el and so isn't preloaded.
Should I move both to faces.el?
>>>> + if (!FIXNATP (face_id))
>>>> + // FIXME: I'm not sure what to do in this case
>>>
>>> I'm not sure I understand why do you need to look at the existing
>>> face's 'face' property? The original code didn't.
>>
>> The original code iterated over face-frame-alist in the order in which
>> entries were added to it. If I understand correctly, iteration order isn't
>> guaranteed on hash tables (is it?), so I had to find a different source of
>> truth for these.
>
> Maybe we should store the ID with the face? I think we wanted to get
> rid of the 'face' property of the faces at some point.
Sounds reasonable; but where would we store it? Right now faces are just
symbols, right?
> No, I was asking why not start with it as nil and actually make a
> hash-table when we first need it.
Oh, I thought it would be simpler to always have a hash table instead of having
to sanity check every time.
Thanks a lot,
Clément.
- 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, Eli Zaretskii, 2020/05/12
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Eli Zaretskii, 2020/05/15
- 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, Eli Zaretskii, 2020/05/15
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Noam Postavsky, 2020/05/15
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined,
Clément Pit-Claudel <=
- bug#41200: Displaying a tooltip with x-show-tip gets very slow as more faces are defined, Eli Zaretskii, 2020/05/15
- 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, Eli Zaretskii, 2020/05/15
- 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, Eli Zaretskii, 2020/05/15
- 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