* jit-lock refontifies too much @ 2005-09-12 6:25 martin rudalics 2005-09-12 13:37 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: martin rudalics @ 2005-09-12 6:25 UTC (permalink / raw) `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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-12 6:25 jit-lock refontifies too much martin rudalics @ 2005-09-12 13:37 ` Stefan Monnier 2005-09-14 5:48 ` martin rudalics 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2005-09-12 13:37 UTC (permalink / raw) Cc: emacs-devel > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-12 13:37 ` Stefan Monnier @ 2005-09-14 5:48 ` martin rudalics 2005-09-14 11:53 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: martin rudalics @ 2005-09-14 5:48 UTC (permalink / raw) Cc: emacs-devel >> (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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-14 5:48 ` martin rudalics @ 2005-09-14 11:53 ` Stefan Monnier 2005-09-15 5:31 ` martin rudalics 2005-09-20 0:03 ` Richard M. Stallman 0 siblings, 2 replies; 24+ messages in thread From: Stefan Monnier @ 2005-09-14 11:53 UTC (permalink / raw) Cc: emacs-devel >>>>> "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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-14 11:53 ` Stefan Monnier @ 2005-09-15 5:31 ` martin rudalics 2005-09-20 0:03 ` Richard M. Stallman 1 sibling, 0 replies; 24+ messages in thread From: martin rudalics @ 2005-09-15 5:31 UTC (permalink / raw) Cc: emacs-devel > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-14 11:53 ` Stefan Monnier 2005-09-15 5:31 ` martin rudalics @ 2005-09-20 0:03 ` Richard M. Stallman 2005-09-20 13:53 ` Stefan Monnier 1 sibling, 1 reply; 24+ messages in thread From: Richard M. Stallman @ 2005-09-20 0:03 UTC (permalink / raw) Cc: rudalics, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-20 0:03 ` Richard M. Stallman @ 2005-09-20 13:53 ` Stefan Monnier 2005-09-21 9:24 ` martin rudalics 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2005-09-20 13:53 UTC (permalink / raw) Cc: rudalics, emacs-devel > Are you going to install this patch? It's installed, AFAIK. Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-20 13:53 ` Stefan Monnier @ 2005-09-21 9:24 ` martin rudalics 2005-09-21 13:34 ` Stefan Monnier 2005-09-25 2:40 ` Richard M. Stallman 0 siblings, 2 replies; 24+ messages in thread From: martin rudalics @ 2005-09-21 9:24 UTC (permalink / raw) Cc: rms, emacs-devel 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) ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-21 9:24 ` martin rudalics @ 2005-09-21 13:34 ` Stefan Monnier 2005-09-22 6:52 ` martin rudalics 2005-09-25 2:40 ` Richard M. Stallman 1 sibling, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2005-09-21 13:34 UTC (permalink / raw) Cc: rms, emacs-devel >>>>> "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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-21 13:34 ` Stefan Monnier @ 2005-09-22 6:52 ` martin rudalics 2005-09-22 17:37 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: martin rudalics @ 2005-09-22 6:52 UTC (permalink / raw) Cc: rms, emacs-devel >>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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-22 6:52 ` martin rudalics @ 2005-09-22 17:37 ` Stefan Monnier 2005-09-25 13:17 ` martin rudalics 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2005-09-22 17:37 UTC (permalink / raw) Cc: rms, emacs-devel > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-22 17:37 ` Stefan Monnier @ 2005-09-25 13:17 ` martin rudalics 2005-09-26 23:56 ` Richard M. Stallman 0 siblings, 1 reply; 24+ messages in thread From: martin rudalics @ 2005-09-25 13:17 UTC (permalink / raw) Cc: rms, emacs-devel 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-25 13:17 ` martin rudalics @ 2005-09-26 23:56 ` Richard M. Stallman 2005-09-27 13:09 ` martin rudalics 2005-09-27 15:54 ` Stefan Monnier 0 siblings, 2 replies; 24+ messages in thread From: Richard M. Stallman @ 2005-09-26 23:56 UTC (permalink / raw) Cc: monnier, emacs-devel 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-26 23:56 ` Richard M. Stallman @ 2005-09-27 13:09 ` martin rudalics 2005-09-28 17:10 ` Richard M. Stallman 2005-09-27 15:54 ` Stefan Monnier 1 sibling, 1 reply; 24+ messages in thread From: martin rudalics @ 2005-09-27 13:09 UTC (permalink / raw) Cc: monnier, emacs-devel > 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-27 13:09 ` martin rudalics @ 2005-09-28 17:10 ` Richard M. Stallman 2005-10-01 7:34 ` martin rudalics 0 siblings, 1 reply; 24+ messages in thread From: Richard M. Stallman @ 2005-09-28 17:10 UTC (permalink / raw) Cc: monnier, emacs-devel > 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-28 17:10 ` Richard M. Stallman @ 2005-10-01 7:34 ` martin rudalics 2005-10-01 23:25 ` Richard M. Stallman 0 siblings, 1 reply; 24+ messages in thread From: martin rudalics @ 2005-10-01 7:34 UTC (permalink / raw) Cc: monnier, emacs-devel 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-10-01 7:34 ` martin rudalics @ 2005-10-01 23:25 ` Richard M. Stallman 2005-10-02 14:53 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: Richard M. Stallman @ 2005-10-01 23:25 UTC (permalink / raw) Cc: monnier, emacs-devel 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-10-01 23:25 ` Richard M. Stallman @ 2005-10-02 14:53 ` Stefan Monnier 0 siblings, 0 replies; 24+ messages in thread From: Stefan Monnier @ 2005-10-02 14:53 UTC (permalink / raw) Cc: martin rudalics, emacs-devel > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-26 23:56 ` Richard M. Stallman 2005-09-27 13:09 ` martin rudalics @ 2005-09-27 15:54 ` Stefan Monnier 2005-09-28 12:26 ` martin rudalics 1 sibling, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2005-09-27 15:54 UTC (permalink / raw) Cc: martin rudalics, emacs-devel > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-27 15:54 ` Stefan Monnier @ 2005-09-28 12:26 ` martin rudalics 2005-09-28 14:16 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: martin rudalics @ 2005-09-28 12:26 UTC (permalink / raw) Cc: rms, emacs-devel > 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) ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-28 12:26 ` martin rudalics @ 2005-09-28 14:16 ` Stefan Monnier 2005-09-29 6:29 ` martin rudalics 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2005-09-28 14:16 UTC (permalink / raw) Cc: rms, emacs-devel >> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-28 14:16 ` Stefan Monnier @ 2005-09-29 6:29 ` martin rudalics 2005-09-29 20:25 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: martin rudalics @ 2005-09-29 6:29 UTC (permalink / raw) Cc: rms, emacs-devel > 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-29 6:29 ` martin rudalics @ 2005-09-29 20:25 ` Stefan Monnier 0 siblings, 0 replies; 24+ messages in thread From: Stefan Monnier @ 2005-09-29 20:25 UTC (permalink / raw) Cc: rms, emacs-devel >>> + (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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: jit-lock refontifies too much 2005-09-21 9:24 ` martin rudalics 2005-09-21 13:34 ` Stefan Monnier @ 2005-09-25 2:40 ` Richard M. Stallman 1 sibling, 0 replies; 24+ messages in thread From: Richard M. Stallman @ 2005-09-25 2:40 UTC (permalink / raw) Cc: monnier, emacs-devel - 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2005-10-02 14:53 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2005-09-12 6:25 jit-lock refontifies too much martin rudalics 2005-09-12 13:37 ` Stefan Monnier 2005-09-14 5:48 ` martin rudalics 2005-09-14 11:53 ` Stefan Monnier 2005-09-15 5:31 ` martin rudalics 2005-09-20 0:03 ` Richard M. Stallman 2005-09-20 13:53 ` Stefan Monnier 2005-09-21 9:24 ` martin rudalics 2005-09-21 13:34 ` Stefan Monnier 2005-09-22 6:52 ` martin rudalics 2005-09-22 17:37 ` Stefan Monnier 2005-09-25 13:17 ` martin rudalics 2005-09-26 23:56 ` Richard M. Stallman 2005-09-27 13:09 ` martin rudalics 2005-09-28 17:10 ` Richard M. Stallman 2005-10-01 7:34 ` martin rudalics 2005-10-01 23:25 ` Richard M. Stallman 2005-10-02 14:53 ` Stefan Monnier 2005-09-27 15:54 ` Stefan Monnier 2005-09-28 12:26 ` martin rudalics 2005-09-28 14:16 ` Stefan Monnier 2005-09-29 6:29 ` martin rudalics 2005-09-29 20:25 ` Stefan Monnier 2005-09-25 2:40 ` Richard M. Stallman
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.