Re: jit-lock refontifies too much

[Stefan Monnier]

>> My gut feeling is that this is way past the point of diminishing returns.
>> Already his optimization is rarely noticeable, but breaks a couple
>> (rare) special cases.

> In the patch below I provide three variables recording the behavior of
> contextual fontification

> jit-lock-fail-change records the number of modifications the patch
> couldn't handle because a buffer change did not occur within the bounds
> of a previous change

> jit-lock-fail-context records the number of buffer modifications where
> syntactic context changed, including modifications of elisp doc-strings

> jit-lock-succ-context records the number of buffer modifications where
> contextual refontification was avoided

> I believe that the patch would be useful if and only if (1) it does not
> break existing code, (2) redisplay doesn't suffer noticeable delay due
> to before-change-functions, (3) the value of jit-lock-succ-context
> exceeds the sum of jit-lock-fail-change and jit-lock-fail-context
> significantly, that is by a factor of two or three at least.

I expect number 2 won't be a problem (or can be made into a nonproblem).
I also expect number 3 won't be a problem; what's your experience?
Number 1 is what concerns me.

> +             ;; Refontify contextually if
> +             ;; 1. paren depth equals 1 before or after change(s) in Lisp
> +             ;;    modes - needed to handle doc-strings,
> +             ;; 2. character that terminates containing string changed,
> +             ;; 3. comment status changed,
> +             ;; 4. comment type changed.

This is ugly: problems specific to emacs-lisp-mode should not be fixed in
jit-lock but in lisp-mode.el.  I'm not sure how to fix it, tho.  Maybe you
could (in lisp-font-lock-syntactic-face-function) place
a jit-lock-defer-multiline property.

> +             (if (or (and (memq major-mode '(emacs-lisp-mode lisp-mode))
> +                          (or (= (nth 0 ppss) 1)
> +                              (= (nth 0 jit-lock-context-ppss) 1)))

Here I'd just compare (not (equal (nth 0 ppss) (nth 0 jit-lock-context-ppss))).

> +(defun jit-lock-before-change (start end)
> +  "Calculate ppss at beginning of first line following END.
> +Installed on `before-change-functions' when contextual fontification is
> +enabled.  START and END are start and end of the changed text."
> +  (when (and jit-lock-mode jit-lock-context-unfontify-pos
> +          ;; Quit unless `jit-lock-context-unfontify-pos' is below START.
> +          (> jit-lock-context-unfontify-pos start)
> +          ;; Do this once for a sequence of modifications only, that is, iff
> +          ;; `jit-lock-context-start' does not point into current buffer.
> +          (not (eq (marker-buffer jit-lock-context-start)
> +                   (current-buffer))))
> +    (when (marker-buffer jit-lock-context-start)
> +      ;; `jit-lock-context-start' points into another buffer.  Set
> +      ;; `jit-lock-context-unfontify-pos' in that buffer.
> +      (with-current-buffer (marker-buffer jit-lock-context-start)
> +     (setq jit-lock-context-unfontify-pos
> +           (min jit-lock-context-unfontify-pos
> +                jit-lock-context-start))))
> +    (save-excursion
> +      ;; Install markers.
> +      (set-marker jit-lock-context-start
> +               (progn (goto-char start) (line-beginning-position)))
> +      (set-marker jit-lock-context-end
> +               (progn (goto-char end) (line-beginning-position 2)))
> +      ;; Record ppss at `jit-lock-context-end'.
> +      (setq jit-lock-context-ppss (syntax-ppss jit-lock-context-end)))))

It's not trivial to convince oneself that your code is correct.  I'd rather
you arrange somehow to leave as much of the code unchanged.  Among other
things, jit-lock-context-unfontify-pos should be set unconditionally in
jit-lock-after-change, as before.  Only if your optimization applies, then
you should reset jit-lock-context-unfontify-pos.  Basically make it obvious
that as long as your optimization does not succeed, the behavior is
unchanged.  The code that does (when (marker-buffer jit-lock-context-start)
...) above is the kind of "oops let's fix things back" kind of thing I'd
rather not see.


        Stefan