jit-lock refontifies too much

jit-lock refontifies too much

[martin rudalics]

`jit-lock-fontify-now' contains the line

     (setq next (line-beginning-position 2))

`font-lock-default-fontify-region' contains the line

     (setq end (line-beginning-position 2))

In practice this means that when jit-lock is turned on, every time you
type a character jit-lock refontifies the current _and_ the following
line.  Chunks fontified stealthily always overlap by one line.

One might argue that `font-lock-after-change-function' contains

	 (progn (goto-char end) (forward-line 1) (point)))))))

but there the adjustment might be useful - albeit undocumented - since
it allows to correct the syntactic context for the following line.
jit-lock mode has `jit-lock-context-fontify' handle syntactic context
and unconditionally refontifying the second line is plain overkill.

In `font-lock-default-fontify-region' I'd therefore replace the lines:

	  (when font-lock-multiline
	    (setq end (or (text-property-any end (point-max)
					     'font-lock-multiline nil)
			  (point-max))))
	  (goto-char end)
	  (setq end (line-beginning-position 2))
	  ;; Now do the fontification.

either by

	  (cond
            (font-lock-multiline
	    (setq end (or (text-property-any end (point-max)
					     'font-lock-multiline nil)
			  (point-max)))
             (goto-char end) (setq end (line-beginning-position 2)))
            ;; don't modify end in jit-lock-mode
            ((and (boundp 'jit-lock-mode) jit-lock-mode))
            (t (goto-char end) (setq end (line-beginning-position 2))))
	  ;; Now do the fontification.

or - but this might disturb people accustomed to the current behavior of
plain font-lock -

	  (when font-lock-multiline
	    (setq end (or (text-property-any end (point-max)
					     'font-lock-multiline nil)
			  (point-max)))
             (goto-char end)
             (setq end (line-beginning-position 2)))
	  ;; Now do the fontification.

Finally one could introduce a variable `font-lock-lines-after'

(defcustom font-lock-lines-after 0
   "*Number of lines after the changed text to include in refontification."
   :type 'integer
   :group 'font-lock
   :version "22.1")

and use

           (goto-char end)
           (setq end (line-beginning-position (1+ font-lock-lines-after)))
	  (when font-lock-multiline
	    (setq end (or (text-property-any end (point-max)
					     'font-lock-multiline nil)
			  (point-max)))
             (goto-char end)
             (setq end (line-beginning-position 2)))
	  ;; Now do the fontification.

Re: jit-lock refontifies too much

[Stefan Monnier]

> or - but this might disturb people accustomed to the current behavior of
> plain font-lock -

> 	  (when font-lock-multiline
> 	    (setq end (or (text-property-any end (point-max)
> 					     'font-lock-multiline nil)
> 			  (point-max)))
>              (goto-char end)
>              (setq end (line-beginning-position 2)))
> 	  ;; Now do the fontification.

I'd rather change jit-lock, since font-lock may be used without jit-lock.


        Stefan

Re: jit-lock refontifies too much

[martin rudalics]

 >>	  (when font-lock-multiline
 >>	    (setq end (or (text-property-any end (point-max)
 >>					     'font-lock-multiline nil)
 >>			  (point-max)))
 >>             (goto-char end)
 >>             (setq end (line-beginning-position 2)))
 >>	  ;; Now do the fontification.
 >
 >
 > I'd rather change jit-lock, since font-lock may be used without jit-lock.

It would be unsafe to change jit-lock.  Currently it is not mandatory
for a "font-lock-fontify-region-function" to adjust the end of the
region.  delphi-fontify-region, for example, doesn't.  It does rely on
jit-lock-fontify-now to provide correct endpoints.  Put this the other
way round.  If font-lock-default-fontify-region's present approach to
fontify an additional line were indeed mandatory, most current
font-lock-fontify-region-functions would be invalid since they do not
adjust the end of the region.

Anyway, my first solution should emulate your proposal.  It would
suppress fontification of the second line iff jit-lock is turned on.
Using font-lock without jit-lock would not change the current behavior:

	  (cond
            (font-lock-multiline
	    (setq end (or (text-property-any end (point-max)
					     'font-lock-multiline nil)
			  (point-max)))
             (goto-char end) (setq end (line-beginning-position 2)))
            ;; don't modify end in jit-lock-mode
            ((and (boundp 'jit-lock-mode) jit-lock-mode))
            (t (goto-char end) (setq end (line-beginning-position 2))))
	  ;; Now do the fontification.

Re: jit-lock refontifies too much

[Stefan Monnier]

>>>>> "martin" == martin rudalics <rudalics@gmx.at> writes:

>>> (when font-lock-multiline
>>> (setq end (or (text-property-any end (point-max)
>>> 'font-lock-multiline nil)
>>> (point-max)))
>>> (goto-char end)
>>> (setq end (line-beginning-position 2)))
>>> ;; Now do the fontification.
>> 
>> 
>> I'd rather change jit-lock, since font-lock may be used without jit-lock.

> It would be unsafe to change jit-lock.  Currently it is not mandatory
> for a "font-lock-fontify-region-function" to adjust the end of the
> region.  delphi-fontify-region, for example, doesn't.  It does rely on
> jit-lock-fontify-now to provide correct endpoints.  Put this the other
> way round.  If font-lock-default-fontify-region's present approach to
> fontify an additional line were indeed mandatory, most current
> font-lock-fontify-region-functions would be invalid since they do not
> adjust the end of the region.

Indeed, you're right.  The problem has nothing to do with jit-lock:
font-lock does the same double-rounding even when jit-lock is turned off,
because font-lock-after-change-function also does the rounding.

> Anyway, my first solution should emulate your proposal.  It would
> suppress fontification of the second line iff jit-lock is turned on.
> Using font-lock without jit-lock would not change the current behavior:

In light of the above, I think that checking jit-lock-mode is not the
right approach.

How 'bout the patch below instead?


        Stefan


--- orig/lisp/font-lock.el
+++ mod/lisp/font-lock.el
@@ -1058,7 +1058,8 @@
 					   'font-lock-multiline nil)
 			(point-max)))
 	  (goto-char end)
-	  (setq end (line-beginning-position 2))
+          ;; Round up to a whole line.
+          (unless (bolp) (setq end (line-beginning-position 2)))
 	  ;; Now do the fontification.
 	  (font-lock-unfontify-region beg end)
 	  (when font-lock-syntactic-keywords

Re: jit-lock refontifies too much

[martin rudalics]

 > How 'bout the patch below instead?
 >
 >
 >         Stefan
 >
 >
 > --- orig/lisp/font-lock.el
 > +++ mod/lisp/font-lock.el
 > @@ -1058,7 +1058,8 @@
 >  					   'font-lock-multiline nil)
 >  			(point-max)))
 >  	  (goto-char end)
 > -	  (setq end (line-beginning-position 2))
 > +          ;; Round up to a whole line.
 > +          (unless (bolp) (setq end (line-beginning-position 2)))
 >  	  ;; Now do the fontification.
 >  	  (font-lock-unfontify-region beg end)
 >  	  (when font-lock-syntactic-keywords
 >
 >

... splendide

Re: jit-lock refontifies too much

[Richard M. Stallman]

Are you going to install this patch?

--- orig/lisp/font-lock.el
+++ mod/lisp/font-lock.el
@@ -1058,7 +1058,8 @@
 					   'font-lock-multiline nil)
 			(point-max)))
 	  (goto-char end)
-	  (setq end (line-beginning-position 2))
+          ;; Round up to a whole line.
+          (unless (bolp) (setq end (line-beginning-position 2)))
 	  ;; Now do the fontification.
 	  (font-lock-unfontify-region beg end)
 	  (when font-lock-syntactic-keywords

Re: jit-lock refontifies too much

[Stefan Monnier]

> Are you going to install this patch?

It's installed, AFAIK.


        Stefan

Re: jit-lock refontifies too much

[martin rudalics]

With contextual fontification turned on jit-lock exposes yet another
inefficiency.  When changing text you trigger eventual execution of
`jit-lock-context-fontify' which removes the `fontified' text property
from the current and all subsequent lines in the buffer.  This has two
unpleasant consequences:

- After jit-lock-context-time seconds of Emacs idle time, redisplay will
   refontify all lines below the current one in any window showing the
   current buffer.

- After jit-lock-stealth-time seconds of Emacs idle time stealth
   fontification refontifies all subsequent lines in the current buffer.

I suppose that the latter was the main cause for complaints about
"stealth fontification GCing a lot" some months ago.  The ensuing
discussion lead to changing the default value of jit-lock-stealth-time
to 16 seconds.

Contextual fontification is useful since it reminds me whenever I forget
to close a string or comment.  But the context of subsequent lines
changes at most every 100th character I type.  Hence in at least 99 out
of 100 cases contextual fontification just slows down editing and wastes
processor cycles.

The problem could be resolved by using the following reasoning (please
correct me if I'm wrong):

- Syntactic context is correctly established via the face property
   assigned to a character by font-lock.  As an example, font-lock
   assigns a character the font-lock-string-face property iff that
   character is within a string - according to the best knowledge of
   syntax-ppss.

- The syntactic context of a character may change iff preceding text is
   modified (I ignore syntax table switching here).

- When text is modified, the syntactical context of subsequent
   characters may change iff the context of the first character
   following the modified text changes.

Hence, it seems sufficient to have jit-lock-fontify-now compare the face
property of the first character following modified text before and after
refontification.  If the property did not change, the modification could
not possibly change the syntactic context of subsequent text.

There is one complication: When text has not been fontified yet, the
comparison described above would always indicate a context change.  This
would be inefficient in some cases, for example, when scrolling backward
into unfontified text.  The patch of jit-lock below tries to avoid this
by using an additional text property called "jit-lock-context".


--- jit-lock.el	2005-08-18 09:58:40.000000000 +0200
+++ jit-lock.el	2005-09-21 10:34:32.000000000 +0200
@@ -159,10 +159,14 @@
  They are called with two arguments: the START and END of the region to fontify.")
  (make-variable-buffer-local 'jit-lock-functions)

-(defvar jit-lock-context-unfontify-pos nil
-  "Consider text after this position as contextually unfontified.
+(defvar jit-lock-context-min-pos nil
+  "Minimum buffer position to consider for contextual fontification.
  If nil, contextual fontification is disabled.")
-(make-variable-buffer-local 'jit-lock-context-unfontify-pos)
+(make-variable-buffer-local 'jit-lock-context-min-pos)
+
+(defvar jit-lock-context-max-pos nil
+  "Maximum buffer position to consider for contextual fontification.")
+(make-variable-buffer-local 'jit-lock-context-max-pos)


  (defvar jit-lock-stealth-timer nil
@@ -233,12 +237,15 @@
  	     (setq jit-lock-context-timer
  		   (run-with-idle-timer jit-lock-context-time t
  					'jit-lock-context-fontify)))
-	   (setq jit-lock-context-unfontify-pos
-		 (or jit-lock-context-unfontify-pos (point-max))))
+	   (unless jit-lock-context-min-pos
+	     (setq jit-lock-context-min-pos (point-max)))
+	   (unless jit-lock-context-max-pos
+	     (setq jit-lock-context-max-pos 1)))

  	 ;; Setup our hooks.
  	 (add-hook 'after-change-functions 'jit-lock-after-change nil t)
-	 (add-hook 'fontification-functions 'jit-lock-function))
+	 (add-hook 'fontification-functions 'jit-lock-function)
+	 (add-hook 'after-revert-hook 'jit-lock-after-revert nil t))

  	;; Turn Just-in-time Lock mode off.
  	(t
@@ -262,7 +269,8 @@

  	 ;; Remove hooks.
  	 (remove-hook 'after-change-functions 'jit-lock-after-change t)
-	 (remove-hook 'fontification-functions 'jit-lock-function))))
+	 (remove-hook 'fontification-functions 'jit-lock-function)
+	 (remove-hook 'after-revert-hook 'jit-lock-after-revert t))))

  ;;;###autoload
  (defun jit-lock-register (fun &optional contextual)
@@ -330,7 +338,7 @@
       ;; from the end of a buffer to its start, can do repeated
       ;; `parse-partial-sexp' starting from `point-min', which can
       ;; take a long time in a large buffer.
-     (let (next)
+     (let (next prop prop-at from to)
         (save-match-data
  	 ;; Fontify chunks beginning at START.  The end of a
  	 ;; chunk is either `end', or the start of a region
@@ -349,6 +357,22 @@
  	   (goto-char next)  (setq next (line-beginning-position 2))
  	   (goto-char start) (setq start (line-beginning-position))

+	   ;; Examine `jit-lock-context' text property within chunk.
+	   (when (and jit-lock-context-min-pos
+		      (< jit-lock-context-min-pos next)
+		      (text-property-any
+		       (setq from (max start jit-lock-context-min-pos))
+		       (setq to (min next jit-lock-context-max-pos))
+		       'jit-lock-context t))
+	     ;; Syntactic context may change after current chunk.  Have `prop'
+	     ;; record the face property of the last character to be fontified.
+	     ;; Usually that will be a newline character.  If text was inserted
+	     ;; and/or deleted at eob that may be any character but then there
+	     ;; is no following text anyway.  In any case let `prop-at' denote
+	     ;; the position before that character.
+	     (setq prop-at (1- next))
+	     (setq prop (get-text-property prop-at 'face)))
+
  	   ;; Fontify the chunk, and mark it as fontified.
  	   ;; We mark it first, to make sure that we don't indefinitely
  	   ;; re-execute this fontification if an error occurs.
@@ -359,8 +383,28 @@
  	     ;; jit-locking), make sure the fontification will be performed
  	     ;; before displaying the block again.
  	     (quit (put-text-property start next 'fontified nil)
+		   (setq prop-at nil) ; reset prop-at
  		   (funcall 'signal (car err) (cdr err))))

+	   ;; Examine face property of last character fontified.
+	   (when prop-at
+	     (if (equal (get-text-property prop-at 'face) prop)
+		 ;; Syntactic context on subsequent lines should not be affected
+		 ;; by change within fontified chunk, remove `jit-lock-context'
+		 ;; property from fontified chunk.
+		 (remove-text-properties from to '(jit-lock-context nil))
+	       ;; Syntactic context on subsequent lines might be affected by
+	       ;; change within fontified chunk.  Assign `jit-lock-context' text
+	       ;; property to character following chunk and correspondingly
+	       ;; update `jit-lock-context-max-pos'.
+	       (save-restriction
+		 (widen) ; we need access to the character following next
+		 (when (< next (point-max))
+		   (setq jit-lock-context-max-pos
+			 (max (1+ next) jit-lock-context-max-pos))
+		   (put-text-property next (1+ next) 'jit-lock-context t))))
+	     (setq prop-at nil))
+
  	   ;; Find the start of the next chunk, if any.
  	   (setq start (text-property-any next end 'fontified nil))))))))

@@ -507,40 +551,44 @@
    "Refresh fontification to take new context into account."
    (dolist (buffer (buffer-list))
      (with-current-buffer buffer
-      (when jit-lock-context-unfontify-pos
+      (when (and jit-lock-context-min-pos
+		 (< jit-lock-context-min-pos jit-lock-context-max-pos))
  	;; (message "Jit-Context %s" (buffer-name))
  	(save-restriction
  	  (widen)
-	  (when (and (>= jit-lock-context-unfontify-pos (point-min))
-		     (< jit-lock-context-unfontify-pos (point-max)))
+	  (when (setq jit-lock-context-min-pos
+		      (text-property-any
+		       (min jit-lock-context-min-pos (point-max))
+		       (min jit-lock-context-max-pos (point-max))
+		       'jit-lock-context t))
  	    ;; If we're in text that matches a complex multi-line
  	    ;; font-lock pattern, make sure the whole text will be
  	    ;; redisplayed eventually.
  	    ;; Despite its name, we treat jit-lock-defer-multiline here
  	    ;; rather than in jit-lock-defer since it has to do with multiple
  	    ;; lines, i.e. with context.
-	    (when (get-text-property jit-lock-context-unfontify-pos
-				     'jit-lock-defer-multiline)
-	      (setq jit-lock-context-unfontify-pos
+	    (when (get-text-property
+		   jit-lock-context-min-pos 'jit-lock-defer-multiline)
+	      (setq jit-lock-context-min-pos
  		    (or (previous-single-property-change
-			 jit-lock-context-unfontify-pos
-			 'jit-lock-defer-multiline)
+			 jit-lock-context-min-pos 'jit-lock-defer-multiline)
  			(point-min))))
  	    (with-buffer-prepared-for-jit-lock
  	     ;; Force contextual refontification.
  	     (remove-text-properties
-	      jit-lock-context-unfontify-pos (point-max)
-	      '(fontified nil jit-lock-defer-multiline nil)))
-	    (setq jit-lock-context-unfontify-pos (point-max))))))))
+	      jit-lock-context-min-pos (point-max)
+	      '(fontified nil jit-lock-defer-multiline nil jit-lock-context nil))))
+	  (setq jit-lock-context-min-pos (point-max))
+	  (setq jit-lock-context-max-pos 1))))))

  (defun jit-lock-after-change (start end old-len)
-  "Mark the rest of the buffer as not fontified after a change.
-Installed on `after-change-functions'.
-START and END are the start and end of the changed text.  OLD-LEN
-is the pre-change length.
-This function ensures that lines following the change will be refontified
-in case the syntax of those lines has changed.  Refontification
-will take place when text is fontified stealthily."
+  "Reset `fontified' text property after a change.
+Installed on `after-change-functions'.	START and END are the start and
+end of the changed text.  OLD-LEN is the pre-change length.  This
+function ensures that lines following the change will be refontified
+provided `jit-lock-contextually' is non-nil and the syntactic context
+may change.  Refontification will take place during redisplay or when
+text is fontified stealthily."
    (when jit-lock-mode
      (save-excursion
        (with-buffer-prepared-for-jit-lock
@@ -561,12 +609,37 @@

         ;; Make sure we change at least one char (in case of deletions).
         (setq end (min (max end (1+ start)) (point-max)))
-       ;; Request refontification.
-       (put-text-property start end 'fontified nil))
-      ;; Mark the change for deferred contextual refontification.
-      (when jit-lock-context-unfontify-pos
-	(setq jit-lock-context-unfontify-pos
-	      (min jit-lock-context-unfontify-pos start))))))
+       (if jit-lock-context-min-pos
+	   ;; Install jit-lock-context text property on modified text and set
+	   ;; `jit-lock-context-min-pos' and `jit-lock-context-max-pos'
+	   ;; accordingly.
+	   (progn
+	     (add-text-properties
+	      start end '(fontified nil jit-lock-context t))
+	     (setq jit-lock-context-min-pos
+		   (min start jit-lock-context-min-pos))
+	     (setq jit-lock-context-max-pos
+		   (max (+ jit-lock-context-max-pos (- end start)) end)))
+	 (put-text-property start end 'fontified nil))))))
+
+(defun jit-lock-after-revert ()
+  "Reset `jit-lock' related properties after reverting a buffer.
+Installed on `after-revert-hook'.  In order to preserve markers and
+other things `revert-buffer' attempts to reuse text at the beginning and
+end of the buffer.  This may leave spurious remnants of text with the
+`jit-lock-context' text property in the reverted buffer.  For sanity
+reasons we remove such properties from the entire buffer, reset
+`jit-lock-context-min-pos' and `jit-lock-context-max-pos', and also
+remove the `fontified' and `jit-lock-defer-multiline' properties."
+  (when jit-lock-context-min-pos
+    (with-buffer-prepared-for-jit-lock
+     (save-restriction
+       (widen)
+       (remove-text-properties
+	(point-min) (point-max)
+	'(fontified nil jit-lock-defer-multiline nil jit-lock-context nil))
+       (setq jit-lock-context-min-pos (point-max))
+       (setq jit-lock-context-max-pos 1)))))

  (provide 'jit-lock)

Re: jit-lock refontifies too much

[Stefan Monnier]

>>>>> "martin" == martin rudalics <rudalics@gmx.at> writes:

> With contextual fontification turned on jit-lock exposes yet another
> inefficiency.  When changing text you trigger eventual execution of
> `jit-lock-context-fontify' which removes the `fontified' text property
> from the current and all subsequent lines in the buffer.  This has two
> unpleasant consequences:

> - After jit-lock-context-time seconds of Emacs idle time, redisplay will
>    refontify all lines below the current one in any window showing the
>    current buffer.

> - After jit-lock-stealth-time seconds of Emacs idle time stealth
>    fontification refontifies all subsequent lines in the current buffer.

This second part predates jit-lock-context-*.  The introduction of
jit-lock-context-* was meant mainly to distinguish the above two cases so
that the idle time that triggers one can be changed without affecting
the other.

> I suppose that the latter was the main cause for complaints about
> "stealth fontification GCing a lot" some months ago.  The ensuing
> discussion lead to changing the default value of jit-lock-stealth-time
> to 16 seconds.

The jit-lock-stealth-* is indeed the part of the code that has caused some
problems for some people (I myself have it disabled).  The introduction of
jit-lock-context-* is what has allowed jit-lock-stealth-time to be pushed
from 3s to 16s (while jit-lock-context-time is itself set to 0.5s instead of
3s).

> Contextual fontification is useful since it reminds me whenever I forget
> to close a string or comment.  But the context of subsequent lines
> changes at most every 100th character I type.

We agree.

> Hence in at least 99 out of 100 cases contextual fontification just slows
> down editing and wastes processor cycles.

In theory, that's true.  In practice, I've yet to come up with a case where
it's noticeable.  Have you actually perceived this slow down, or are
talking hypothetically?  What do you compare it against?  Remember that in
Emacs-21, the exact same wasteful redisplay took place, except that it took
place after 3s of idle-time rather than 0.5s.

> The problem could be resolved by using the following reasoning (please
> correct me if I'm wrong):

> - Syntactic context is correctly established via the face property
>    assigned to a character by font-lock.  As an example, font-lock
>    assigns a character the font-lock-string-face property iff that
>    character is within a string - according to the best knowledge of
>    syntax-ppss.

That's not quite true since the font-lock-string face may be used for
elements not recognized by the syntax-tables.  But the exceptions are very
rare anyway and probably don't affect your proposed solution.

> - The syntactic context of a character may change iff preceding text is
>    modified (I ignore syntax table switching here).

Actually, jit-lock-multiline ws introduced specifically because this is not
necessarily the case.  In Perl, if you change

            s{a}{b}ex;
into
            s{a}{b};

the syntactic context of `b' is changed.  Yes: Perl's syntax sucks.
But again, this shouldn't affect your proposed solution.

> - When text is modified, the syntactical context of subsequent
>    characters may change iff the context of the first character
>    following the modified text changes.

Now, this is the core reasoning behind your proposed solution.

> Hence, it seems sufficient to have jit-lock-fontify-now compare the face
> property of the first character following modified text before and after
> refontification.  If the property did not change, the modification could
> not possibly change the syntactic context of subsequent text.

I like your idea.  But I know it'll fail in a case that I use on
a regular basis: nested comments.

If you change

    (* (* this is a nested comment
          blabla *)
       end of comment *)

to

    (* this is a nested comment
          blabla *)
       end of comment *)

the char after your modification will still be on font-lock-comment face,
but the text "end of comment *)" needs to be refontified.  Wait, I don't
even need nested comments, just take C mode:

     /* sfsgf
        tryjjy */
to
     //* sfsgf
        tryjjy */

Although in your case jit-lock will round up the "change" to a whole line,
so the problem shouldn't bite you in the case of C.

The above problems can be "easily" addressed by changing your algorithm to
not look at the face property, but instead to look at the return value of
syntax-ppss.  If it hasn't changed, then we know the subsequent text doesn't
need refontification.

Hmm... except of course for things like sh-mode where

       cat - <<EOF
       blabla
       EOF
to
       cat - <<EO
       blabla
       EOF

won't work right.  But that case can be handled specially (the problem in
that case is that the font-lock-syntactic-* setting of sh-mode will
actually cause the syntax-table property on the final EOF to change, but
it's done inside font-lock so it's a buffer modification which font-lock
won't notice because inhibit-modification-hooks is non-nil: we can require
sh-mode to call a special function to notify font-lock that it's modified
some part of the buffer).

> There is one complication: When text has not been fontified yet, the
> comparison described above would always indicate a context change.  This
> would be inefficient in some cases, for example, when scrolling backward
> into unfontified text.  The patch of jit-lock below tries to avoid this
> by using an additional text property called "jit-lock-context".

Adding yet-another-text-property is a waste of precious CPU and
memory resources.  It makes your suggestion pretty dubious.  Luckily, using
syntax-ppss instead of faces should not suffer from this same initialization
problem.


        Stefan

Re: jit-lock refontifies too much

[martin rudalics]

 >>Hence in at least 99 out of 100 cases contextual fontification just slows
 >>down editing and wastes processor cycles.
 >
 >
 > In theory, that's true.  In practice, I've yet to come up with a case where
 > it's noticeable.  Have you actually perceived this slow down, or are
 > talking hypothetically?  What do you compare it against?  Remember that in
 > Emacs-21, the exact same wasteful redisplay took place, except that it took
 > place after 3s of idle-time rather than 0.5s.
 >

I'm only comparing Emacs-22.1 jit-lock with my patch.  On a 1GHz machine
setting jit-lock-context-time to zero seconds makes Emacs stutter when I
use auto-repeat to insert a sequence of characters.  Patched, I can set
this to zero seconds and won't suffer any delays as long as I don't
insert, for example, a sequence of string delimiters.  With faster
hardware the slowdown might go away.  GC overhead stays.

 >
 >>The problem could be resolved by using the following reasoning (please
 >>correct me if I'm wrong):
 >
 >
 >>- Syntactic context is correctly established via the face property
 >>   assigned to a character by font-lock.  As an example, font-lock
 >>   assigns a character the font-lock-string-face property iff that
 >>   character is within a string - according to the best knowledge of
 >>   syntax-ppss.
 >
 >
 > That's not quite true since the font-lock-string face may be used for
 > elements not recognized by the syntax-tables.  But the exceptions are very
 > rare anyway and probably don't affect your proposed solution.

Anyway, the "iff" is wrong.

 >
 >
 >>- The syntactic context of a character may change iff preceding text is
 >>   modified (I ignore syntax table switching here).
 >
 >
 > Actually, jit-lock-multiline ws introduced specifically because this is not
 > necessarily the case.  In Perl, if you change
 >
 >             s{a}{b}ex;
 > into
 >             s{a}{b};
 >
 > the syntactic context of `b' is changed.  Yes: Perl's syntax sucks.
 > But again, this shouldn't affect your proposed solution.
 >

But this is already beyond syntactic fontification, that is, current
font-lock-fontify-syntactically-region using syntax-ppss only can't
handle this.  And my solution should handle jit-lock-multiline much the
same way original jit-lock does.  Anyway, you're right.  I would have to
say something like "syntax-pps recognizes changes in the syntactic
context of a character iff text preceding the character is modified".

 > If you change
 >
 >     (* (* this is a nested comment
 >           blabla *)
 >        end of comment *)
 >
 > to
 >
 >     (* this is a nested comment
 >           blabla *)
 >        end of comment *)
 >
 > the char after your modification will still be on font-lock-comment face,
 > but the text "end of comment *)" needs to be refontified.

Elementary.  It's not the first time I'm stumbling over nested comments.

 > Wait, I don't
 > even need nested comments, just take C mode:
 >
 >      /* sfsgf
 >         tryjjy */
 > to
 >      //* sfsgf
 >         tryjjy */
 >
 > Although in your case jit-lock will round up the "change" to a whole line,
 > so the problem shouldn't bite you in the case of C.

It does, however.  I would have to syntax-ppss the start of the
following line in order to handle this.  Your examples reveal the major
weakness of my approach.  I'm not able to handle changes in the type of
the first delimiter of a delimited expression as long as the face of an
enclosed character may stay the same.

 >
 > The above problems can be "easily" addressed by changing your algorithm to
 > not look at the face property, but instead to look at the return value of
 > syntax-ppss.  If it hasn't changed, then we know the subsequent text doesn't
 > need refontification.
 >

I don't remember any previous return value when refontifying.  Calling
syntax-ppss twice for the same position in one and the same invocation
of jit-lock-fontify-now always yields the same value.

 >>There is one complication: When text has not been fontified yet, the
 >>comparison described above would always indicate a context change.  This
 >>would be inefficient in some cases, for example, when scrolling backward
 >>into unfontified text.  The patch of jit-lock below tries to avoid this
 >>by using an additional text property called "jit-lock-context".
 >
 >
 > Adding yet-another-text-property is a waste of precious CPU and
 > memory resources.  It makes your suggestion pretty dubious.  Luckily, using
 > syntax-ppss instead of faces should not suffer from this same initialization
 > problem.
 >

The property would last only as long as there are unhandled buffer
changes.  However, it does waste resources.  Too bad that syntax-ppss
won't work.

Re: jit-lock refontifies too much

[Stefan Monnier]

> I'm only comparing Emacs-22.1 jit-lock with my patch.  On a 1GHz machine
> setting jit-lock-context-time to zero seconds makes Emacs stutter when I
> use auto-repeat to insert a sequence of characters.  Patched, I can set
> this to zero seconds and won't suffer any delays as long as I don't
> insert, for example, a sequence of string delimiters.  With faster
> hardware the slowdown might go away.  GC overhead stays.

Oh, yes, if you set it to 0s the slowdown will be noticeable, especially in
buffers where the font-lock patterns are complex, such as in C mode or
Perl mode.  That's partly why the default is only 0.5s.

>> Wait, I don't even need nested comments, just take C mode:
>> 
>> /* sfsgf
>> tryjjy */
>> to
>> //* sfsgf
>> tryjjy */
>> 
>> Although in your case jit-lock will round up the "change" to a whole line,
>> so the problem shouldn't bite you in the case of C.

> It does, however.  I would have to syntax-ppss the start of the
> following line in order to handle this.  Your examples reveal the major
> weakness of my approach.  I'm not able to handle changes in the type of
> the first delimiter of a delimited expression as long as the face of an
> enclosed character may stay the same.

Exactly.  There may be some other cases, tho.  E.g. in elisp-mode, if you
change

        (defvar foo 1
          "hello")
to
        (defvar foo '(1
          "hello")

the "hello" needs refontification (from font-lock-doc to font-lock-string).
In the above case, the syntax-ppss state changes, so using that would solve
this problem.  But changing

        (defvar foo 1
          "hello")
to
        (defvar foo 0 1
          "hello")

also requires a similar refontification, but the syntax-ppss state doesn't
really change, so even using syntax-ppss wouldn't solve this problem.

>> The above problems can be "easily" addressed by changing your algorithm to
>> not look at the face property, but instead to look at the return value of
>> syntax-ppss.  If it hasn't changed, then we know the subsequent text doesn't
>> need refontification.

> I don't remember any previous return value when refontifying.  Calling
> syntax-ppss twice for the same position in one and the same invocation
> of jit-lock-fontify-now always yields the same value.

Oh, right, when jit-lock-fontify-now gets called, the buffer is already
changed, so you'd have to use a before-change-functions hook to remember the
syntax-ppss state before the change.  Hmmm...


        Stefan

Re: jit-lock refontifies too much

[Richard M. Stallman]

    - Syntactic context is correctly established via the face property
       assigned to a character by font-lock.  As an example, font-lock
       assigns a character the font-lock-string-face property iff that
       character is within a string - according to the best knowledge of
       syntax-ppss.

It might be worth doing this for the first character of a line, only.
That would bound the extent of refontification, but without wasting]
lots of space for text properties.

In the code that you wrote, are these properties put on characters at
the start of a line, or could they appear anywhere?

    There is one complication: When text has not been fontified yet, the
    comparison described above would always indicate a context change.

I do not understand that statement.  When the subsequent text has
not been fontified yet, it certainly needs to be fontified.
So what is it that you perceive as a problem?

Stefan wrote:

    Adding yet-another-text-property is a waste of precious CPU and
    memory resources.  It makes your suggestion pretty dubious.  Luckily, using
    syntax-ppss instead of faces should not suffer from this same initialization
    problem.

How does syntax-ppss avoid the need to scan too far?
(I know you told me before, but I don't remember.)
I wonder if use of syntax-ppss will be faster than
the refontifications it saves.

You wrote:

    I'm only comparing Emacs-22.1 jit-lock with my patch.  On a 1GHz machine
    setting jit-lock-context-time to zero seconds makes Emacs stutter when I
    use auto-repeat to insert a sequence of characters.

jit-lock-context-time is not normally zero.  So what exactly is the
goal of your proposal?  Is the goal to make jit-lock-context-time zero
without loss of performance?

     > The above problems can be "easily" addressed by changing your algorithm to
     > not look at the face property, but instead to look at the return value of
     > syntax-ppss.  If it hasn't changed, then we know the subsequent text doesn't
     > need refontification.
     >

    I don't remember any previous return value when refontifying.  Calling
    syntax-ppss twice for the same position in one and the same invocation
    of jit-lock-fontify-now always yields the same value.

    The property would last only as long as there are unhandled buffer
    changes.  However, it does waste resources.  Too bad that syntax-ppss
    won't work.

I don't understand that response.

Stefan wrote:

    Oh, right, when jit-lock-fontify-now gets called, the buffer is already
    changed, so you'd have to use a before-change-functions hook to remember the
    syntax-ppss state before the change.  Hmmm...

You'd have to cache the syntax-ppss value in a text property, perhaps
for the first char on a line.

Re: jit-lock refontifies too much

[martin rudalics]

Stefan wrote:
 > In the above case, the syntax-ppss state changes, so using that would solve
 > this problem.  But changing
 >
 >         (defvar foo 1
 >           "hello")
 > to
 >         (defvar foo 0 1
 >           "hello")
 >
 > also requires a similar refontification, but the syntax-ppss state doesn't
 > really change, so even using syntax-ppss wouldn't solve this problem.
 >

And that would be a fairly common case too, for example, when mistyping
something on the first line of a definition.  Maybe one day someone
writes a parse-partial-sexp with a return value that holds the number of
elements in the innermost containing list before TO - useful to speed up
lisp-font-lock-syntactic-face-function anyway.  Till then I must remove
any optimizations when paren depth equals/equaled 1 in Lisp modes.

Richard wrote:
 >     - Syntactic context is correctly established via the face property
 >        assigned to a character by font-lock.  As an example, font-lock
 >        assigns a character the font-lock-string-face property iff that
 >        character is within a string - according to the best knowledge of
 >        syntax-ppss.
 >
 > It might be worth doing this for the first character of a line, only.
 > That would bound the extent of refontification, but without wasting]
 > lots of space for text properties.
 >

I don't waste any text properties on this.  All I do is comparing the
face property of one specific character as assigned by refontification
against the face property of that character before refontification.  If
these two differ I have to refontify subsequent text.  Unfortunately, as
Stefan pointed out, comparing face properties is not sufficient to
detect all possible changes of syntactic context.

 > In the code that you wrote, are these properties put on characters at
 > the start of a line, or could they appear anywhere?
 >
 >     There is one complication: When text has not been fontified yet, the
 >     comparison described above would always indicate a context change.
 >
 > I do not understand that statement.  When the subsequent text has
 > not been fontified yet, it certainly needs to be fontified.
 > So what is it that you perceive as a problem?

False positives: When, for example, text within a string was never
fontified before it (usually) does not have a font-lock face property.
After first fontification it does.  Hence my comparison above would
indicate a change although there never was a buffer modification.  Each
time I scrolled backwards into yet unfontified text I would have to
refontify the rest of the buffer.  That's what I needed my text property
for.  The property would have been added together with the fontified nil
property on the entire modified text.  Hence the property could have
appeared anywhere.

 > Stefan wrote:
 >
 >     Adding yet-another-text-property is a waste of precious CPU and
 >     memory resources.  It makes your suggestion pretty dubious.  Luckily, using
 >     syntax-ppss instead of faces should not suffer from this same initialization
 >     problem.
 >
 > How does syntax-ppss avoid the need to scan too far?
 > (I know you told me before, but I don't remember.)
 > I wonder if use of syntax-ppss will be faster than
 > the refontifications it saves.
 >

syntax-ppss is called in font-lock-fontify-syntactically-region -
usually every time I insert/delete one single character.  Hence it
should be very fast.  In practice it runs parse-partial-sexp from the
beginning of the first line where a buffer modification occurred
recently.  It might be interesting to cache additional values when
typing or inserting longer stretches of text by temporarily binding
syntax-ppss-max-span to something less than the default value.  The
current strategy attempts to distribute cache positions evenly in a
buffer.  However, buffers don't change evenly.

 > You wrote:
 >
 >     I'm only comparing Emacs-22.1 jit-lock with my patch.  On a 1GHz machine
 >     setting jit-lock-context-time to zero seconds makes Emacs stutter when I
 >     use auto-repeat to insert a sequence of characters.
 >
 > jit-lock-context-time is not normally zero.  So what exactly is the
 > goal of your proposal?  Is the goal to make jit-lock-context-time zero
 > without loss of performance?
 >

My goal is to save CPU overhead due to contextual refontification.  But
Stefan asked me to provide an example where contextual refontification
could have visible impact.

In practice I wouldn't ever set jit-lock-context-time to zero.  When I
insert, for example, a single string delimiter, I don't want to have all
subsequent lines change their face instantaneously since I usually close
the string immediately afterwards.  Contextual fontification is useful
to remind me when I forget to insert a delimiter or, to tell me that I
have corrected that mistake.  The default value accomplishes this in an
ideal way - to my taste.  In this context it's also important that the
value is larger than the auto-repeat delay of the keyboard.

 >      > The above problems can be "easily" addressed by changing your algorithm to
 >      > not look at the face property, but instead to look at the return value of
 >      > syntax-ppss.  If it hasn't changed, then we know the subsequent text doesn't
 >      > need refontification.
 >      >
 >
 >     I don't remember any previous return value when refontifying.  Calling
 >     syntax-ppss twice for the same position in one and the same invocation
 >     of jit-lock-fontify-now always yields the same value.
 >
 >     The property would last only as long as there are unhandled buffer
 >     changes.  However, it does waste resources.  Too bad that syntax-ppss
 >     won't work.
 >
 > I don't understand that response.
 >

As sketched above I try to detect whether syntactic context changes by
comparing two values.  When I compare face properties the old property
is still there before I refontify, maybe some time after the buffer
modification.  I remember the old value, refontify, and compare the old
value against the new value after refontification.  With ppss things are
different since any "old value" gets lost together with the buffer
modification.  Hence, I can't use ppss to establish the old value when
refontifying.

"The property would last ..." sentence refers to the text property I
needed to discriminate regions that were never fontified before from
regions that were refontified due to a buffer change.  The property
would have got removed as soon as all yet unresolved positions had been
either fontified or their fontified property set to nil.  However, till
then it would have wasted resources.

 > Stefan wrote:
 >
 >     Oh, right, when jit-lock-fontify-now gets called, the buffer is already
 >     changed, so you'd have to use a before-change-functions hook to remember the
 >     syntax-ppss state before the change.  Hmmm...
 >
 > You'd have to cache the syntax-ppss value in a text property, perhaps
 > for the first char on a line.
 >

That would be overly expensive.  I rewrote all this with ppss, two
markers, a before-change-functions hook and without any text properties.
It seems to work but needs some further testing.

Re: jit-lock refontifies too much

[Richard M. Stallman]

    False positives: When, for example, text within a string was never
    fontified before it (usually) does not have a font-lock face property.
    After first fontification it does.  Hence my comparison above would
    indicate a change although there never was a buffer modification.

It would be better just to keep comparing until you reach a line that
previously was fontified.  After all, the line that was not fontified
before certainly should get fontified, if it is on the screen.  But a
subsequent line that was previously fontified may not need to be
changed.

     > jit-lock-context-time is not normally zero.  So what exactly is the
     > goal of your proposal?  Is the goal to make jit-lock-context-time zero
     > without loss of performance?

    My goal is to save CPU overhead due to contextual refontification.  But
    Stefan asked me to provide an example where contextual refontification
    could have visible impact.

Since the question is whether there is a realistic case where it would
have visible impact.  If nobody really wants to set
jit-lock-context-time to zero, that's not a realistic example.
So we still don't know whether there is a realistic case where it
would have visible impact.

      With ppss things are
    different since any "old value" gets lost together with the buffer
    modification.  Hence, I can't use ppss to establish the old value when
    refontifying.

Yes.  That is why I suggested recording these ppss values in text
properties of the first character on a line--so that they would stay
around for comparison later.

    That would be overly expensive.  I rewrote all this with ppss, two
    markers, a before-change-functions hook and without any text properties.
    It seems to work but needs some further testing.

That is interesting.  I would not have expected it to work.
However, this would require doing that computation for each change,
and that could be rather expensive, right?

One advantage for the idea of saving it in a text property for the
first character on each line is that it only has to be checked
when it is time to refontify.

Another possible advantage is: if things are not in sync for the first
line after the end of the changed text, it might be in sync on a
subsequent line, and that could avoid refontifying most of the lines
on the screen.

Re: jit-lock refontifies too much

[martin rudalics]

 > It would be better just to keep comparing until you reach a line that
 > previously was fontified.  After all, the line that was not fontified
 > before certainly should get fontified, if it is on the screen.  But a
 > subsequent line that was previously fontified may not need to be
 > changed.
 >

This might work for a single window on one and the same buffer.  With
two windows there might remain some unfontified stretch between the
portions shown in the windows.  When I modify text in the first window
and that would affect the context of the second window, the information
might not get through the unfontified area.  On the other hand comparing
until I reach fontified text (or point-max) regardless of whether text
is on screen would be expensive.

 > Since the question is whether there is a realistic case where it would
 > have visible impact.  If nobody really wants to set
 > jit-lock-context-time to zero, that's not a realistic example.
 > So we still don't know whether there is a realistic case where it
 > would have visible impact.

Disciplined users insert/delete both delimiters of comments or strings
simultaneously.  They could profitably set `jit-lock-context-time' to
zero.  I'm too undisciplined.

 >
 > That is why I suggested recording these ppss values in text
 > properties of the first character on a line--so that they would stay
 > around for comparison later.

That would be fine.  But recording these properties for each and every
line fontified would introduce too much overhead.  It would be enough
recording only those things really needed (fields 0, 3, 4, and 7 of
ppss).  In any case this would have to be done for every buffer even if
it isn't modified ever.  A lazy strategy would require some clairvoyance
about which lines would get modified or a before-change-hook anyway.

Saving ppss in text properties at bol would be certainly useful for
font-locking purposes.  But these ppss would usually be invalid and
therefore not useful for other purposes.  And they would have to be
recycled and recalculated after a context change has been detected.

 >
 >     That would be overly expensive.  I rewrote all this with ppss, two
 >     markers, a before-change-functions hook and without any text properties.
 >     It seems to work but needs some further testing.
 >
 > That is interesting.  I would not have expected it to work.

It will "work" until you or Stefan have a closer look at it.

 > However, this would require doing that computation for each change,
 > and that could be rather expensive, right?

For the first in a sequence of changes.  That is, when you enter/delete
a stretch of text I would initially record the ppss for the line begin
following the first character you entered/deleted.  Usually, you then
modify text (a) within 0.5 secs and (b) adjacent to the position of the
last text change.  In such cases I would not record anything for
subsequent modifications.  If you now sit for 0.5 secs, refontification
will be needed iff the syntactic context really changed.

If, within 0.5 secs, you change text on another line I give up.  More
precisely, "change text on another line" means "change text outside the
region enclosed by the markers".  Initially this region ranges from the
begin of the line where a modification starts to the begin of the line
following the end of the modification.  In particular, I give up for
backward-deletions of newlines preceding the start of the region,
indent-region, filling, or replacing all occurrences of some text in a
buffer.  Although, setting `jit-lock-context-time' to zero could avoid
refontification in some of these cases.

 >
 > One advantage for the idea of saving it in a text property for the
 > first character on each line is that it only has to be checked
 > when it is time to refontify.
 >

If "time to refontifiy" means "during `jit-lock-context-fontify'" that's
what I try to do presently.

 > Another possible advantage is: if things are not in sync for the first
 > line after the end of the changed text, it might be in sync on a
 > subsequent line, and that could avoid refontifying most of the lines
 > on the screen.
 >

I can think of two interpretations for "things are not in sync":

1. Buffer text contains something like an unclosed string/comment.  For
    an unclosed comment a subsequent line may "compensate" the error if
    it contains a comment terminator and comments don't nest.  In this
    case one could avoid refontification.  A paren in column zero will
    compensate such errors too.  But in that case correcting an error
    would require that subsequent lines are refontified in order to clear
    the warnings installed by font-lock.  Hence it might be difficult to
    derive a general method.

2. Text on screen has not been yet fontified correctly.  In this case
    "things are not in sync" for `jit-lock-context-time' seconds.  With
    the default settings I would have to get things out of sync and back
    into sync within 0.5 secs.  Doing this within the region enclosed by
    the markers should be handled by my approach.  Changes outside the
    region would indicacte the presence of a skilled user - someone who'd
    probably profit from setting `jit-lock-context-time' to a value less
    than the default.

Re: jit-lock refontifies too much

[Stefan Monnier]

>     That would be overly expensive.  I rewrote all this with ppss, two
>     markers, a before-change-functions hook and without any text properties.
>     It seems to work but needs some further testing.

> That is interesting.  I would not have expected it to work.
> However, this would require doing that computation for each change,
> and that could be rather expensive, right?

Shouldn't be particularly expensive: syntax-ppss is called by font-lock
anyway, and since it uses caching, calling it a couple more times around the
same spot is pretty cheap.

> One advantage for the idea of saving it in a text property for the
> first character on each line is that it only has to be checked
> when it is time to refontify.

What his patch does is pretty much the same except he uses
before-change-functions in order to lazily only store the syntax-ppss of the
line after the change, whereas you'd eagerly store it for every line in
the buffer.

> Another possible advantage is: if things are not in sync for the first
> line after the end of the changed text, it might be in sync on a
> subsequent line, and that could avoid refontifying most of the lines
> on the screen.

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.


        Stefan

Re: jit-lock refontifies too much

[martin rudalics]

 > 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.


--- jit-lock.el	2005-08-18 09:58:40.000000000 +0200
+++ jit-lock.el	2005-09-28 14:17:28.000000000 +0200
@@ -164,6 +164,19 @@
  If nil, contextual fontification is disabled.")
  (make-variable-buffer-local 'jit-lock-context-unfontify-pos)

+(defvar jit-lock-context-start (make-marker)
+  "Position preceding text affected by latest sequence of buffer changes.
+A marker that stays behind when text is inserted there.")
+
+(defvar jit-lock-context-end (make-marker)
+  "Position following text affected by latest sequence of buffer changes.
+A marker that advances when text is inserted there.")
+(set-marker-insertion-type jit-lock-context-end t)
+
+(defvar jit-lock-context-ppss nil
+ "ppss recorded before the latest sequence of buffer changes.
+This state is recorded by `jit-lock-before-change' at position
+`jit-lock-context-end' before the first of these changes.")

  (defvar jit-lock-stealth-timer nil
    "Timer for stealth fontification in Just-in-time Lock mode.")
@@ -229,6 +242,7 @@

  	 ;; Initialize contextual fontification if requested.
  	 (when (eq jit-lock-contextually t)
+	   (add-hook 'before-change-functions 'jit-lock-before-change nil t)
  	   (unless jit-lock-context-timer
  	     (setq jit-lock-context-timer
  		   (run-with-idle-timer jit-lock-context-time t
@@ -258,9 +272,13 @@
  	     (setq jit-lock-context-timer nil))
  	   (when jit-lock-defer-timer
  	     (cancel-timer jit-lock-defer-timer)
-	     (setq jit-lock-defer-timer nil)))
+	     (setq jit-lock-defer-timer nil))
+	   ;; Reset markers.
+	   (set-marker jit-lock-context-start nil)
+	   (set-marker jit-lock-context-end nil))

  	 ;; Remove hooks.
+	 (remove-hook 'before-change-functions 'jit-lock-before-change t)
  	 (remove-hook 'after-change-functions 'jit-lock-after-change t)
  	 (remove-hook 'fontification-functions 'jit-lock-function))))

@@ -509,38 +527,97 @@
      (with-current-buffer buffer
        (when jit-lock-context-unfontify-pos
  	;; (message "Jit-Context %s" (buffer-name))
-	(save-restriction
-	  (widen)
-	  (when (and (>= jit-lock-context-unfontify-pos (point-min))
-		     (< jit-lock-context-unfontify-pos (point-max)))
-	    ;; If we're in text that matches a complex multi-line
-	    ;; font-lock pattern, make sure the whole text will be
-	    ;; redisplayed eventually.
-	    ;; Despite its name, we treat jit-lock-defer-multiline here
-	    ;; rather than in jit-lock-defer since it has to do with multiple
-	    ;; lines, i.e. with context.
-	    (when (get-text-property jit-lock-context-unfontify-pos
-				     'jit-lock-defer-multiline)
-	      (setq jit-lock-context-unfontify-pos
-		    (or (previous-single-property-change
-			 jit-lock-context-unfontify-pos
-			 'jit-lock-defer-multiline)
-			(point-min))))
-	    (with-buffer-prepared-for-jit-lock
-	     ;; Force contextual refontification.
-	     (remove-text-properties
-	      jit-lock-context-unfontify-pos (point-max)
-	      '(fontified nil jit-lock-defer-multiline nil)))
-	    (setq jit-lock-context-unfontify-pos (point-max))))))))
+	(save-excursion
+	  (save-restriction
+	    (widen)
+
+	    ;; If `jit-lock-context-start' points into current buffer
+	    ;; investigate latest sequence of buffer modifications.
+	    (when (eq (marker-buffer jit-lock-context-start) (current-buffer))
+	      ;; Record ppss for `jit-lock-context-end' - a position following
+	      ;; the latest sequence of buffer changes - and compare it with the
+	      ;; value before these changes recorded in `jit-lock-context-ppss'.
+	      (let ((ppss (syntax-ppss jit-lock-context-end)))
+		;; 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.
+		(if (or (and (memq major-mode '(emacs-lisp-mode lisp-mode))
+			     (or (= (nth 0 ppss) 1)
+				 (= (nth 0 jit-lock-context-ppss) 1)))
+			(not (equal (nth 3 ppss) (nth 3 jit-lock-context-ppss)))
+			(not (equal (nth 4 ppss) (nth 4 jit-lock-context-ppss)))
+			(not (equal (nth 7 ppss) (nth 7 jit-lock-context-ppss))))
+		    ;; Assign `jit-lock-context-unfontify-pos'.
+		    (progn
+		      (setq jit-lock-fail-context (1+ jit-lock-fail-context))
+		      (setq jit-lock-context-unfontify-pos
+			    (min jit-lock-context-unfontify-pos
+				 jit-lock-context-start)))
+		  (setq jit-lock-succ-context (1+ jit-lock-succ-context))))
+	      ;; Reset markers.
+	      (set-marker jit-lock-context-start nil)
+	      (set-marker jit-lock-context-end nil))
+
+	    (when (and (>= jit-lock-context-unfontify-pos (point-min))
+		       (< jit-lock-context-unfontify-pos (point-max)))
+	      ;; If we're in text that matches a complex multi-line
+	      ;; font-lock pattern, make sure the whole text will be
+	      ;; redisplayed eventually.
+	      ;; Despite its name, we treat jit-lock-defer-multiline here
+	      ;; rather than in jit-lock-defer since it has to do with multiple
+	      ;; lines, i.e. with context.
+	      (when (get-text-property jit-lock-context-unfontify-pos
+				       'jit-lock-defer-multiline)
+		(setq jit-lock-context-unfontify-pos
+		      (or (previous-single-property-change
+			   jit-lock-context-unfontify-pos
+			   'jit-lock-defer-multiline)
+			  (point-min))))
+	      (with-buffer-prepared-for-jit-lock
+	       ;; Force contextual refontification.
+	       (remove-text-properties
+		jit-lock-context-unfontify-pos (point-max)
+		'(fontified nil jit-lock-defer-multiline nil)))
+	      (setq jit-lock-context-unfontify-pos (point-max)))))))))
+
+(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)))))

  (defun jit-lock-after-change (start end old-len)
    "Mark the rest of the buffer as not fontified after a change.
-Installed on `after-change-functions'.
-START and END are the start and end of the changed text.  OLD-LEN
-is the pre-change length.
-This function ensures that lines following the change will be refontified
-in case the syntax of those lines has changed.  Refontification
-will take place when text is fontified stealthily."
+Installed on `after-change-functions'.	START and END are the start and
+end of the changed text.  OLD-LEN is the pre-change length.  When
+contextual fontification is enabled, this function ensures that lines
+following the change will be refontified in case the syntax of those
+lines has changed.  Refontification will take place during redisplay or
+when text is fontified stealthily."
    (when jit-lock-mode
      (save-excursion
        (with-buffer-prepared-for-jit-lock
@@ -562,13 +639,50 @@
         ;; Make sure we change at least one char (in case of deletions).
         (setq end (min (max end (1+ start)) (point-max)))
         ;; Request refontification.
-       (put-text-property start end 'fontified nil))
-      ;; Mark the change for deferred contextual refontification.
-      (when jit-lock-context-unfontify-pos
-	(setq jit-lock-context-unfontify-pos
-	      (min jit-lock-context-unfontify-pos start))))))
+       (put-text-property start end 'fontified nil)
+
+       ;; Contextual refontification.
+       (cond
+	((not jit-lock-context-unfontify-pos))
+	;; Handle case where `jit-lock-context-start' was not set properly for
+	;; some reason, for example, because `before-change-functions' has been
+	;; temporarily let-bound to nil.
+	((not (eq (marker-buffer jit-lock-context-start) (current-buffer)))
+	 ;; Adjust `jit-lock-context-unfontify-pos'.
+	 (setq jit-lock-context-unfontify-pos
+	       (min jit-lock-context-unfontify-pos start))
+	 (when (marker-buffer jit-lock-context-start)
+	   ;; `jit-lock-context-start' points into some other 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)))
+	   ;; Reset markers.
+	   (setq jit-lock-fail-change (1+ jit-lock-fail-change))
+	   (set-marker jit-lock-context-start nil)
+	   (set-marker jit-lock-context-end nil)))
+	;; Quit if `jit-lock-context-unfontify-pos' is before START.  Also sort
+	;; out buffer modifications that precede `jit-lock-context-start' or
+	;; follow `jit-lock-context-end'.  We could handle backward-deletions of
+	;; newlines here but in that case we would have to re-parse anyway.
+	((or (<= jit-lock-context-unfontify-pos start)
+	     (< start jit-lock-context-start)
+	     (> end jit-lock-context-end))
+	 ;; Adjust `jit-lock-context-unfontify-pos'.
+	 (setq jit-lock-context-unfontify-pos
+	       (min jit-lock-context-unfontify-pos
+		    jit-lock-context-start start))
+	 ;; Reset markers.
+	 (setq jit-lock-fail-change (1+ jit-lock-fail-change))
+	 (set-marker jit-lock-context-start nil)
+	 (set-marker jit-lock-context-end nil)))))))

  (provide 'jit-lock)

  ;;; arch-tag: 56b5de6e-f581-453b-bb97-49c39372ff9e
  ;;; jit-lock.el ends here
+
+(defvar jit-lock-fail-change 0)
+(defvar jit-lock-fail-context 0)
+(defvar jit-lock-succ-context 0)

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

Re: jit-lock refontifies too much

[Richard M. Stallman]

     > It would be better just to keep comparing until you reach a line that
     > previously was fontified.  After all, the line that was not fontified
     > before certainly should get fontified, if it is on the screen.  But a
     > subsequent line that was previously fontified may not need to be
     > changed.
     >

    This might work for a single window on one and the same buffer.  With
    two windows there might remain some unfontified stretch between the
    portions shown in the windows.

So what?

     > That is why I suggested recording these ppss values in text
     > properties of the first character on a line--so that they would stay
     > around for comparison later.

    That would be fine.  But recording these properties for each and every
    line fontified would introduce too much overhead.

I suspect think it is comparable to the amount of space used by
font-lock mode now.  Maybe less.  If so, why is it too much?

     > Another possible advantage is: if things are not in sync for the first
     > line after the end of the changed text, it might be in sync on a
     > subsequent line, and that could avoid refontifying most of the lines
     > on the screen.
     >

    I can think of two interpretations for "things are not in sync":

It means "the before and after ppss values do not match".

You're arguing this can't happen very easily.  Maybe that is true.

Re: jit-lock refontifies too much

[martin rudalics]

 > 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.
 >

It's ugly.  I'll defvar a jit-lock-context-refontify-function and set
that locally in lisp-mode.

 >
 >>+		(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))).

That would be wrong - I must refontify when both values equal 1 - else I
wouldn't handle changing

(defvar foo 1 0
   "bar")

to

(defvar foo 10
   "bar")

 >
 > 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.

You're right.  I'll try to follow that advice.

 > 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.
 >

That code is needed if someone has set `jit-lock-context-time' to a
large value, changes something in buffer A, switches to buffer B, and
changes something in buffer B _before_ jit-lock-context-fontify is able
to handle the modification in buffer A.  In that case, I can't apply my
optimization in buffer A but have to set jit-lock-context-unfontify-pos
to jit-lock-context-start.  If I implement your proposal and retain the
assignment in after-change-functions I shan't need that.

Re: jit-lock refontifies too much

[Stefan Monnier]

>>> +		(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))).

> That would be wrong - I must refontify when both values equal 1 - else I
> wouldn't handle changing

You mean that it wouldn't help for emacs-lisp-mode.  You're right.
The problem with emacs-lisp-mode should be fixed elsewhere.

I suggested to compare (not (equal (nth 0 ppss) (nth
0 jit-lock-context-ppss))) for "completeness".


        Stefan

Re: jit-lock refontifies too much

[martin rudalics]

Richard wrote:
 >      > That is why I suggested recording these ppss values in text
 >      > properties of the first character on a line--so that they would stay
 >      > around for comparison later.
 >
 >     That would be fine.  But recording these properties for each and every
 >     line fontified would introduce too much overhead.
 >
 > I suspect think it is comparable to the amount of space used by
 > font-lock mode now.  Maybe less.  If so, why is it too much?
 >

I somewhat fell between two stools here.  With respect to my first
proposal Stefan judged ".. yet-another-text-property is a waste of
precious CPU and memory resources."

 >      > Another possible advantage is: if things are not in sync for the first
 >      > line after the end of the changed text, it might be in sync on a
 >      > subsequent line, and that could avoid refontifying most of the lines
 >      > on the screen.
 >      >
 >
 >     I can think of two interpretations for "things are not in sync":
 >
 > It means "the before and after ppss values do not match".
 >
 > You're arguing this can't happen very easily.  Maybe that is true.
 >

I didn't argue that.  Complete ppss are hardly ever "in sync" after a
modification of preceding text.  The values of ppss I need to determine
whether context changed might be in sync quite often.



Stefan wrote:
 > I suggested to compare (not (equal (nth 0 ppss) (nth
 > 0 jit-lock-context-ppss))) for "completeness".
 >

That's reasonable.  I'll do that.

Re: jit-lock refontifies too much

[Richard M. Stallman]

    I somewhat fell between two stools here.  With respect to my first
    proposal Stefan judged ".. yet-another-text-property is a waste of
    precious CPU and memory resources."

I do not necessarily agree with him; I think it would depend on how
much good the feature does.

I am not sure how much good it would do, so I don't know whether this
idea is worth while.

     > It means "the before and after ppss values do not match".
     >
     > You're arguing this can't happen very easily.  Maybe that is true.
     >

    I didn't argue that.  Complete ppss are hardly ever "in sync" after a
    modification of preceding text.  The values of ppss I need to determine
    whether context changed might be in sync quite often.

"In sync" doesn't mean entirely equal, it means that they show
there was no relevant change.

By the way, in principle we could add more info to the ppss value so
that font lock gets whatever info it needs so as to work correctly.

Re: jit-lock refontifies too much

[Stefan Monnier]

>     I somewhat fell between two stools here.  With respect to my first
>     proposal Stefan judged ".. yet-another-text-property is a waste of
>     precious CPU and memory resources."

> I do not necessarily agree with him; I think it would depend on how
> much good the feature does.

Agreed.  If it brings a significant benefit, adding yet-another-property is
worthwhile: it only makes things worse by a (presumably smallish) constant
factor.  In considered this cost high because the feature seems rather
minor: unless you set your jit-lock-context-time to a very small value
you'll rarely if ever notice the difference (except maybe in those few rare
cases where the new feature gets things wrong ;-( ).


        Stefan