emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: master 7362554: Widen around c-font-lock-fontify-region. This fixes


From: Stefan Monnier
Subject: Re: master 7362554: Widen around c-font-lock-fontify-region. This fixes bug #38049.
Date: Thu, 14 Nov 2019 16:29:38 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux)

>> I think the problem is wider than CC-mode.  Maybe CC-mode is more
>> susceptible to it or maybe it's just an accident that this was reported
>> for CC-mode, but running font-lock (and syntax-propertize) within
>> narrowing tends to be fiddly.
> I'm also okay with fixing it in font-lock or jit-lock.  But doing that
> inside reposition.el makes no sense to me.

IIUC this can't be fixed in font-lock or jit-lock.  It can only be fixed
"upstream" (e.g. in repos-count-screen-lines) or "downstream" (in the
particular major mode's rules).

I think neither "upstream" nor "downstream" is satisfactory, but
until/unless we introduce some "structure" on the narrowings (so that
font-lock can know for sure whether and how it can widen) that's what we
have to live with.

> So we are going to do such changes in every application that calls
> vertical-motion, directly or indirectly?

I'd phrase it as "this would need to be done in any narrowing
which changes point-min and internally can trigger jit-lock".

Whether we're going to do it or not I don't know.
I think "don't change point-min" is generally the better fix (which
partly means weaning ourselves from narrowing).

> And what about posn-at-point, or pos-visible-in-window-p, or any other
> API that uses the display code internally?

If these can trigger jit-lock then that applies as well, yes.
`sit-for` as well, BTW.

> are we going to fix their callers as well?

The fix should not be "where we can do vertical-motion (and friends)"
but "where we narrow (and later trigger jit-lock)".  Whether that means
"fix their callers as well" depends on what it is that owns the "their
callers": yes it would be in the functions that call (directly or not)
`vertical-motion` (and friends) but in the functions that call
those functions.

>> > In particular, what if the POINT-MIN..END chunk is still too large to
>> > fontify in one go?
>> AFAIK all uses of jit-lock are more efficient if they get fewer larger
>> chunks than more smaller chunks.
> So you are saying that we should enlarge jit-lock-chunk-size to
> most-positive-fixnum?

That would cause jit-lock being applied not just to "fewer larger
chunks" but also to more total text: when the redisplay calls jit-lock,
we know POS will be displayed and needs to be jit-locked but we don't
how far further we will keep looking, and that what jit-lock-chunk-size
is for (it's supposed to be large enough that the per-chunk overhead
doesn't kill it while being small enough that the amount of text
we needlessly jit-lock isn't too large either).

In the presently discussed patch we know beforehand that all the text
between start and end will be considered and will need to be jit-locked,
which is why we can do it more efficiently in a single explicit call
than what the redisplay code would do otherwise.

In any case, changing the narrow-to-region so it doesn't change
point-min is the right fix.  Adding a call to `jit-lock-fontify-now` is
another but worse way to fix it.

Adding that call should be harmless and could be beneficial for
performance, but I personally wouldn't do it (it can also be harmful
for performance in the case where the text has already been jit-locked
in which case it won't do anything but will still take a bit of time to
do it).


        Stfan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]