Subject: Re: bug#72692: Emacs 31.05 (40eecd594ac) get SIGSEGV on Linux (Linux 6.6.45 Kde Wayland)

From: Pip Cet <[email-masked]>
Date: Wed, 28 Aug 2024 11:58:48 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Tue, 27 Aug 2024 19:26:21 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked], [email-masked]
>>
>> "Eli Zaretskii" <[email-masked]> writes:
>>
>> > That was the main idea of the patch I proposed, except that avoiding
>> > to set the face_change flag when all we have in the cache are the
>> > basic faces is a bit stronger, no?
>>
>> Indeed.  I think your proposed patch is the way to go, with the
>> off-by-one fixed.  I think there will be some slowdown for people using
>> non-ASCII faces in the modeline, in particular, but it should still
>> result in acceptable performance.
>
> Displaying the mode line calls init_iterator exactly once, AFAICT, so
> I don't expect any special performance issues due to that.

I think you're right, and that I was confused.

>> >> Is that case sufficiently common to result in acceptable
>> >> performance?
>> >
>> > Which case?
>>
>> That only basic faces are realized, and we can therefore avoid freeing
>> any cached faces.
>
> If you mean "only basic faces are realized" as opposed to "face cache
> is empty", then it's quite possible that they are the same situation,
> at least now.

I think I saw a backtrace in which the basic faces were realized, but
I'm not entirely sure and I don't think it matters; your patch should
work and is the way to go.

> (But in that case we don't lose anything, since the
> test of cache->used = 0 is not cheaper than cache->used > SOMETHING.)
> But if you mean "face cache is empty", then I think this situation is
> quite common, yes.  People nowadays write Lisp code that creates and
> modifies a lot of faces, and every change in the faces on the Lisp
> level causes us to set the face_change flag, which then causes us to
> empty the cache.

Correct, but as we don't lose anything by performing your optimization,
let's go with that, assuming it survives testing and you don't come up
with something even better?

Pip