[rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Richard Stallman]

This is what we're thinking of using instead of
buffer-chars-modified-tick.  It is a smaller change and more modular;
presuming it solves the problem well enough, it is definitely
preferable for installation now.

So, what do people think of this change?

------- Start of forwarded message -------
Date: Sun, 27 Aug 2006 12:14:22 +0200
From: martin rudalics <rudalics@gmx.at>
MIME-Version: 1.0
To:  rms@gnu.org
Subject: Re: jit lock sit-for provokes redisplay provokes imenu
In-Reply-To: <E1GF7hm-0001aP-NB@fencepost.gnu.org>
Content-Type: multipart/mixed;
 boundary="------------090602040009010004050308"
X-Spam-Status: No, score=1.0 required=5.0 tests=RCVD_IN_NJABL_PROXY 
	autolearn=no version=3.0.4

This is a multi-part message in MIME format.
- --------------090602040009010004050308
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit

Sorry for procrastinating.  Is the attached patch acceptable?

- --------------090602040009010004050308
Content-Type: text/plain;
 name="buffer-hash.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="buffer-hash.patch"

*** buffer.c.~1.506.~	Tue Aug 15 10:01:00 2006
- --- buffer.c	Sun Aug 27 11:41:52 2006
***************
*** 1148,1153 ****
- --- 1148,1187 ----

    return make_number (BUF_MODIFF (buf));
  }
+ 
+ DEFUN ("buffer-hash", Fbuffer_hash, Sbuffer_hash,
+        0, 1, 0,
+        doc: /* Return hash code for BUFFER.
+ No argument or nil as argument means use current buffer as BUFFER.  */)
+      (buffer)
+      register Lisp_Object buffer;
+ {
+   register struct buffer *buf;
+   register unsigned char *at, *to;
+   register unsigned int hash = 0;
+ 
+   if (NILP (buffer))
+     buf = current_buffer;
+   else
+     {
+       CHECK_BUFFER (buffer);
+       buf = XBUFFER (buffer);
+     }
+ 
+   at = BUF_BEG_ADDR (buf);
+   to = BUF_GPT_ADDR (buf);
+   while (at < to)
+     hash = ((((unsigned)(hash) << 4) + (((unsigned)(hash) >> 24) & 0x0fffffff))
+ 	    + (unsigned)(*at++));
+ 
+   at =  BUF_GAP_END_ADDR (buf);
+   to = BUF_Z_ADDR (buf);
+   while (at < to)
+     hash = ((((unsigned)(hash) << 4) + (((unsigned)(hash) >> 24) & 0x0fffffff))
+ 	    + (unsigned)(*at++));
+ 
+   return make_number (hash & INTMASK);
+ }
  \f
  DEFUN ("rename-buffer", Frename_buffer, Srename_buffer, 1, 2,
         "sRename buffer (to new name): \nP",
***************
*** 6024,6029 ****
- --- 6058,6064 ----
    defsubr (&Sbuffer_modified_p);
    defsubr (&Sset_buffer_modified_p);
    defsubr (&Sbuffer_modified_tick);
+   defsubr (&Sbuffer_hash);
    defsubr (&Srename_buffer);
    defsubr (&Sother_buffer);
    defsubr (&Sbuffer_enable_undo);


*** imenu.el.~1.118.~	Tue Aug 15 10:00:50 2006
- --- imenu.el	Sun Aug 27 11:32:36 2006
***************
*** 970,981 ****
    "The value of (buffer-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (not (eq (buffer-modified-tick)
! 		      imenu-menubar-modified-tick)))
!     (setq imenu-menubar-modified-tick (buffer-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.
- --- 970,988 ----
    "The value of (buffer-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

+ (defvar imenu-buffer-hash 0
+   "The value returned by `buffer-hash' as of last call to `imenu-update-menubar'.")
+ (make-variable-buffer-local 'imenu-buffer-hash)
+ 
  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
! 	     (let ((tick (buffer-modified-tick)))
! 	       (when (/= tick imenu-menubar-modified-tick)
! 		 (setq imenu-menubar-modified-tick tick)))
! 	     (let ((hash (buffer-hash)))
! 	       (when (/= hash imenu-buffer-hash)
! 		 (setq imenu-buffer-hash hash))))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
        ;; since the last time we did it.

- --------------090602040009010004050308--
------- End of forwarded message -------

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

> This is what we're thinking of using instead of
> buffer-chars-modified-tick.  It is a smaller change and more modular;
> presuming it solves the problem well enough, it is definitely
> preferable for installation now.
>
> So, what do people think of this change?

Nothing.  Hashing is not reliable and leads to non-reproducible
isolated cases of non-working.  It will also necessitate scanning
through the entire buffer whenever imenu-update-menubar is called.

I work with text buffers corresponding to books of a 1000 pages.
Those are several Megabytes large.  Every operation which scans the
whole buffer for some reason is going to be painful.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Kim F. Storm]

David Kastrup <dak@gnu.org> writes:

> Richard Stallman <rms@gnu.org> writes:

>> So, what do people think of this change?
>
>                                 It will also necessitate scanning
> through the entire buffer whenever imenu-update-menubar is called.
>
> I work with text buffers corresponding to books of a 1000 pages.
> Those are several Megabytes large.  Every operation which scans the
> whole buffer for some reason is going to be painful.

IIUC, this would be called after every modification to the buffer.

SO: VERY VERY VERY BAD IDEA.


Martin's change seems solid enough -- it's done, it's there, it fixes
the problem -- let's install it.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Richard Stallman]

    I work with text buffers corresponding to books of a 1000 pages.
    Those are several Megabytes large.  Every operation which scans the
    whole buffer for some reason is going to be painful.

How long does it take for that code to hash that buffer?

(Do you use imenu?)

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> This is what we're thinking of using instead of
> buffer-chars-modified-tick.  It is a smaller change and more modular;
> presuming it solves the problem well enough, it is definitely
> preferable for installation now.
>
> So, what do people think of this change?

Here is another idea which is even more modular: make imenu register
jit-lock functions to check buffer-modified-tick before the actual
jit-lock fontification takes place, and update
imenu-menubar-modified-tick afterwards if necessary.

I've checked that this works.

*** emacs/lisp/jit-lock.el.~1.55.~	2006-08-25 14:00:27.000000000 -0400
--- emacs/lisp/jit-lock.el	2006-08-28 12:05:32.000000000 -0400
***************
*** 279,290 ****
  	 (remove-hook 'after-change-functions 'jit-lock-after-change t)
  	 (remove-hook 'fontification-functions 'jit-lock-function))))
  
! (defun jit-lock-register (fun &optional contextual)
    "Register FUN as a fontification function to be called in this buffer.
  FUN will be called with two arguments START and END indicating the region
  that needs to be (re)fontified.
  If non-nil, CONTEXTUAL means that a contextual fontification would be useful."
!   (add-hook 'jit-lock-functions fun nil t)
    (when (and contextual jit-lock-contextually)
      (set (make-local-variable 'jit-lock-contextually) t))
    (jit-lock-mode t))
--- 279,292 ----
  	 (remove-hook 'after-change-functions 'jit-lock-after-change t)
  	 (remove-hook 'fontification-functions 'jit-lock-function))))
  
! (defun jit-lock-register (fun &optional contextual append)
    "Register FUN as a fontification function to be called in this buffer.
  FUN will be called with two arguments START and END indicating the region
  that needs to be (re)fontified.
+ FUN is called before all other previously-registered fontification functions
+ unless the optional argument APPEND is non-nil, in which case it is called last.
  If non-nil, CONTEXTUAL means that a contextual fontification would be useful."
!   (add-hook 'jit-lock-functions fun append t)
    (when (and contextual jit-lock-contextually)
      (set (make-local-variable 'jit-lock-contextually) t))
    (jit-lock-mode t))
*** emacs/lisp/imenu.el.~1.118.~	2006-07-09 16:52:14.000000000 -0400
--- emacs/lisp/imenu.el	2006-08-28 12:18:28.000000000 -0400
***************
*** 953,958 ****
--- 953,962 ----
  	(define-key newmap [menu-bar index]
  	  `(menu-item ,name ,(make-sparse-keymap "Imenu")))
  	(use-local-map newmap)
+ 	(when jit-lock-mode
+ 	  ;; Avoid updating the menubar after each stealth fontification.
+ 	  (jit-lock-register 'imenu-save-tick-before-jit-lock nil nil)
+ 	  (jit-lock-register 'imenu-update-tick-after-jit-lock nil t))
  	(add-hook 'menu-bar-update-hook 'imenu-update-menubar))
      (error "The mode `%s' does not support Imenu" mode-name)))
  
***************
*** 970,975 ****
--- 974,991 ----
    "The value of (buffer-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)
  
+ (defvar imenu-unmodified-before-jit nil)
+ (make-variable-buffer-local 'imenu-unmodified-before-jit)
+ 
+ (defun imenu-update-tick-for-jit-lock (start end)
+   (if (eq (buffer-modified-tick) imenu-menubar-modified-tick)
+       (setq imenu-unmodified-before-jit t)))
+ 
+ (defun imenu-update-tick-after-jit-lock (start end)
+   (when imenu-unmodified-before-jit
+     (setq imenu-menubar-modified-tick (buffer-modified-tick))
+     (setq imenu-unmodified-before-jit nil)))
+ 
  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Stefan Monnier]

> + 	(when jit-lock-mode
> + 	  ;; Avoid updating the menubar after each stealth fontification.
> + 	  (jit-lock-register 'imenu-save-tick-before-jit-lock nil nil)
> + 	  (jit-lock-register 'imenu-update-tick-after-jit-lock nil t))

Won't work if font-lock-mode is enabled after imenu is used.


        Stefan

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Richard Stallman]

    > + 	(when jit-lock-mode
    > + 	  ;; Avoid updating the menubar after each stealth fontification.
    > + 	  (jit-lock-register 'imenu-save-tick-before-jit-lock nil nil)
    > + 	  (jit-lock-register 'imenu-update-tick-after-jit-lock nil t))

    Won't work if font-lock-mode is enabled after imenu is used.

Could that be solved by making jit-lock-mode also enable these hooks?

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     > + 	(when jit-lock-mode
>     > + 	  ;; Avoid updating the menubar after each stealth fontification.
>     > + 	  (jit-lock-register 'imenu-save-tick-before-jit-lock nil nil)
>     > + 	  (jit-lock-register 'imenu-update-tick-after-jit-lock nil t))
>
>     Won't work if font-lock-mode is enabled after imenu is used.
>
> Could that be solved by making jit-lock-mode also enable these hooks?

How about this?  It adds jit-lock-pre-stealth-fontify-hook and
jit-lock-post-stealth-fontify-hook, which are normal hooks run before
and after each chunk of stealth fontification.  Then imenu can use
this to avoid menu bar updates resulting from stealth fontification.

The only disadvantage is adding two `run-hook' called to the
jit-lock-stealth-fontify timer function, but hopefully the performance
impact isn't too bad.

*** emacs/lisp/imenu.el.~1.118.~	2006-08-29 18:14:09.000000000 -0400
--- emacs/lisp/imenu.el	2006-08-29 22:38:21.000000000 -0400
***************
*** 953,958 ****
--- 953,962 ----
  	(define-key newmap [menu-bar index]
  	  `(menu-item ,name ,(make-sparse-keymap "Imenu")))
  	(use-local-map newmap)
+ 	(add-hook 'jit-lock-pre-stealth-fontify-hook
+ 		  'imenu-pre-stealth-fontify-fn nil t)
+ 	(add-hook 'jit-lock-post-stealth-fontify-hook
+ 		  'imenu-post-stealth-fontify-fn nil t)
  	(add-hook 'menu-bar-update-hook 'imenu-update-menubar))
      (error "The mode `%s' does not support Imenu" mode-name)))
  
***************
*** 970,980 ****
--- 974,1000 ----
    "The value of (buffer-modified-tick) as of last call to `imenu-update-menubar'.")
  (make-variable-buffer-local 'imenu-menubar-modified-tick)
  
+ (defvar imenu-unmodified-before-stealth-fontify nil)
+ (make-variable-buffer-local 'imenu-unmodified-before-stealth-fontify)
+ 
+ (defun imenu-pre-stealth-fontify-fn ()
+   (setq imenu-unmodified-before-stealth-fontify
+ 	(eq (buffer-modified-tick) imenu-menubar-modified-tick)))
+ 
+ (defun imenu-post-stealth-fontify-fn ()
+   (if imenu-unmodified-before-stealth-fontify
+       (setq imenu-menubar-modified-tick (buffer-modified-tick)
+ 	    imenu-unmodified-before-stealth-fontify nil)))
+ 
+ (setq foo 0)
+ 
  (defun imenu-update-menubar ()
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
  	     (not (eq (buffer-modified-tick)
  		      imenu-menubar-modified-tick)))
+     (setq foo (% (1+ foo) 10))
+     (message (format "Foo %d" foo))
      (setq imenu-menubar-modified-tick (buffer-modified-tick))
      (let ((index-alist (imenu--make-index-alist t)))
        ;; Don't bother updating if the index-alist has not changed
*** emacs/lisp/jit-lock.el.~1.55.~	2006-08-29 18:14:09.000000000 -0400
--- emacs/lisp/jit-lock.el	2006-08-29 22:33:28.000000000 -0400
***************
*** 182,187 ****
--- 182,192 ----
    "List of buffers with pending deferred fontification.")
  (defvar jit-lock-stealth-buffers nil
    "List of buffers that are being fontified stealthily.")
+ 
+ (defvar jit-lock-pre-stealth-fontify-hook nil
+   "Normal hook run before performing each chunk of stealth fontification.")
+ (defvar jit-lock-post-stealth-fontify-hook nil
+   "Normal hook run after performing each chunk of stealth fontification.")
  \f
  ;;; JIT lock mode
  
***************
*** 488,495 ****
--- 493,502 ----
  		  (with-temp-message (if jit-lock-stealth-verbose
  					 (concat "JIT stealth lock "
  						 (buffer-name)))
+ 		    (run-hooks 'jit-lock-pre-stealth-fontify-hook)
  		    (jit-lock-fontify-now start
  					  (+ start jit-lock-chunk-size))
+ 		    (run-hooks 'jit-lock-post-stealth-fontify-hook)
  		    ;; Run again after `jit-lock-stealth-nice' seconds.
  		    (setq delay (or jit-lock-stealth-nice 0)))
  		;; Nothing to fontify here.  Remove this buffer from
***************
*** 503,509 ****
  	(timer-set-idle-time jit-lock-stealth-repeat-timer (current-idle-time))
  	(timer-inc-time jit-lock-stealth-repeat-timer delay)
  	(timer-activate-when-idle jit-lock-stealth-repeat-timer t)))))
- 
  \f
  ;;; Deferred fontification.
  
--- 510,515 ----

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Stefan Monnier]

> How about this?  It adds jit-lock-pre-stealth-fontify-hook and
> jit-lock-post-stealth-fontify-hook, which are normal hooks run before
> and after each chunk of stealth fontification.  Then imenu can use
> this to avoid menu bar updates resulting from stealth fontification.

This only eliminates one particular source of unwanted updates.  I don't
think this code is worth the trouble.  At the very least, add it to
font-lock rather than jit-lock.
Better yet, introduce a macro `with-buffer-unmodified' like the one used in
jit-lock, and make it manage a `buffer-really-modified-tick', kind of like
the buffer-chars-modified-tick.  Then use it in jit-lock/font-lock
and friends.


        Stefan


PS: And make sure it also works when nested ;-)

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Kim F. Storm]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> How about this?  It adds jit-lock-pre-stealth-fontify-hook and
>> jit-lock-post-stealth-fontify-hook, which are normal hooks run before
>> and after each chunk of stealth fontification.  Then imenu can use
>> this to avoid menu bar updates resulting from stealth fontification.
>
> This only eliminates one particular source of unwanted updates.  I don't
> think this code is worth the trouble.  At the very least, add it to
> font-lock rather than jit-lock.
> Better yet, introduce a macro `with-buffer-unmodified' like the one used in
> jit-lock, and make it manage a `buffer-really-modified-tick', kind of like
> the buffer-chars-modified-tick.  Then use it in jit-lock/font-lock
> and friends.
>
>
>         Stefan
>
>
> PS: And make sure it also works when nested ;-)

I still vote for installing Martin's buffer-chars-modified-tick

 ... anything else seems to be "too much" or "too little" or "too slow" :-)

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Better yet, introduce a macro `with-buffer-unmodified' like the one used in
> jit-lock, and make it manage a `buffer-really-modified-tick', kind of like
> the buffer-chars-modified-tick.  Then use it in jit-lock/font-lock
> and friends.

I don't see how this could possibly work.  We can't update
buffer-really-modified-tick in Lisp code for arbitrary buffer changes
(that's the whole point of Martin's patch --- you have to do it at the
C level).  And you can't use it to manage a general-purpose variable
like imenu-menubar-modified-tick, because that keeps track of the
value of (buffer-modified-tick) as of the last call to
`imenu-update-menubar', so imenu must reset
imenu-menubar-modified-tick during each call to
`imenu-update-menubar'.

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Stefan Monnier]

>> Better yet, introduce a macro `with-buffer-unmodified' like the one used in
>> jit-lock, and make it manage a `buffer-really-modified-tick', kind of like
>> the buffer-chars-modified-tick.  Then use it in jit-lock/font-lock
>> and friends.

> I don't see how this could possibly work.  We can't update
> buffer-really-modified-tick in Lisp code for arbitrary buffer changes
> (that's the whole point of Martin's patch --- you have to do it at the
> C level).  And you can't use it to manage a general-purpose variable
> like imenu-menubar-modified-tick, because that keeps track of the
> value of (buffer-modified-tick) as of the last call to
> `imenu-update-menubar', so imenu must reset
> imenu-menubar-modified-tick during each call to
> `imenu-update-menubar'.

It can manage a variable that keeps track of a pair of numbers, that say
"any buffer-tick values between those two numbers can be considered as
equal".


        Stefan

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Richard Stallman]

    How about this?  It adds jit-lock-pre-stealth-fontify-hook and
    jit-lock-post-stealth-fontify-hook, which are normal hooks run before
    and after each chunk of stealth fontification.  Then imenu can use
    this to avoid menu bar updates resulting from stealth fontification.

It looks good to me.

Stefan wrote:

    This only eliminates one particular source of unwanted updates.  I don't
    think this code is worth the trouble.

That cause of unwanted updates is the only one we know of.  Nothing
else is likely to change the buffer in the background.  It's a fairly
simple and safe solution to the problem that we are working on, and I
think that's definitely worth the trouble.

      At the very least, add it to
    font-lock rather than jit-lock.

Why would that be better?

    Better yet, introduce a macro `with-buffer-unmodified' like the one used in
    jit-lock, and make it manage a `buffer-really-modified-tick', kind of like
    the buffer-chars-modified-tick.  Then use it in jit-lock/font-lock
    and friends.

That seems a lot more trouble, and I don't see that it is needed.

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[martin rudalics]

 > Stefan wrote:
 >
 >     This only eliminates one particular source of unwanted updates.  I don't
 >     think this code is worth the trouble.
 >
 > That cause of unwanted updates is the only one we know of.  Nothing
 > else is likely to change the buffer in the background.

`jit-lock-fontify-now' during scrolling, searching, window resizing.
Not mentioning the plethora of "hidden buffer changes" in c-mode.

 >     Better yet, introduce a macro `with-buffer-unmodified' like the one used in
 >     jit-lock, and make it manage a `buffer-really-modified-tick', kind of like
 >     the buffer-chars-modified-tick.  Then use it in jit-lock/font-lock
 >     and friends.
 >
 > That seems a lot more trouble, and I don't see that it is needed.

I think it is needed.

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Chong Yidong]

martin rudalics <rudalics@gmx.at> writes:

>> Stefan wrote:
>>
>>     This only eliminates one particular source of unwanted updates.  I don't
>>     think this code is worth the trouble.
>>
>> That cause of unwanted updates is the only one we know of.  Nothing
>> else is likely to change the buffer in the background.
>
> `jit-lock-fontify-now' during scrolling, searching, window resizing.
> Not mentioning the plethora of "hidden buffer changes" in c-mode.

AFAIU, those are much less problematic than the changes during stealth
fontification.  That's because stealth fontification takes place in a
timer, while Emacs is idle, and makes a change every 0.5 seconds (by
default).

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[martin rudalics]

 > martin rudalics <rudalics@gmx.at> writes:
 >
 >
 >>>Stefan wrote:
 >>>
 >>>    This only eliminates one particular source of unwanted updates.  I don't
 >>>    think this code is worth the trouble.
 >>>
 >>>That cause of unwanted updates is the only one we know of.  Nothing
 >>>else is likely to change the buffer in the background.
 >>
 >>`jit-lock-fontify-now' during scrolling, searching, window resizing.
 >>Not mentioning the plethora of "hidden buffer changes" in c-mode.
 >
 >
 > AFAIU, those are much less problematic than the changes during stealth
 > fontification.  That's because stealth fontification takes place in a
 > timer, while Emacs is idle, and makes a change every 0.5 seconds (by
 > default).

Not since we changed `jit-lock-stealth-fontify'.  Remember: The old call
chain was `jit-lock-stealth-fontify' -> `sit-for' -> `sit_for' ->
`redisplay_preserve_echo_area' -> `redisplay_internal' ->
`prepare_menu_bars' -> `update_menu_bar' with the notorious
`windows_or_buffers_changed' check and the subsequent run of
`Qmenu_bar_update_hook' which contains `imenu-update-menubar'.

But we _removed_ `sit-for' from `jit-lock-stealth-fontify' and hence
stealth fontification doesn't redisplay anymore.  That's been my entire
motivation for patching this.  I think, most people complaining about
stealth fontification overhead in fact complained about those senseless
_redisplays_ stealth fontification's `sit-for' incurred every 0.5 secs.
(Compare the current "silent PC vs. emacs" thread on emacs-pretest-bug.)

Hence the only overhead incurred by stealth fontification nowadays
should be due to the fact that it "modified" the buffer, the user (or
process output) interrupted stealth fontifcation, and as a consequence
`imenu-update-menubar' is run.  We can ignore that.

The buffer changes during scrolling and window resizing, however, are
considerably more serious since they affect responsiveness of Emacs.  My
Emacs considerably lags behind other applications during scrolling and
flickers noticeably whenever I drag a modeline.  Now consider that for
every _single_ line dragged or scrolled I call `imenu-update-menubar'.
The only buffer "change" here is the fontification of one single line.
That doesn't make any sense.  And it makes the speed of scrolling depend
(1) on the size of the buffer and (2) the quality of a thing like
`imenu-create-index-function'.  Wouldn't it make more sense to spend
that time to filter out, for example, all those definitions Imenu finds
in C comments or strings?

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Kim F. Storm]

martin rudalics <rudalics@gmx.at> writes:

> The buffer changes during scrolling and window resizing, however, are
> considerably more serious since they affect responsiveness of Emacs.  My
> Emacs considerably lags behind other applications during scrolling and
> flickers noticeably whenever I drag a modeline.  Now consider that for
> every _single_ line dragged or scrolled I call `imenu-update-menubar'.
> The only buffer "change" here is the fontification of one single line.
> That doesn't make any sense.  And it makes the speed of scrolling depend
> (1) on the size of the buffer and (2) the quality of a thing like
> `imenu-create-index-function'.  Wouldn't it make more sense to spend
> that time to filter out, for example, all those definitions Imenu finds
> in C comments or strings?

And you changes (at the C level + imenu) fixes all these problems, right?

If, so I think we are wasting our time discussing sub-optimal ways to
fix a specific problem where we already have a generic solution.

Can't we just install Martin's patch and get closer to the pretest??

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Chong Yidong]

storm@cua.dk (Kim F. Storm) writes:

> And you changes (at the C level + imenu) fixes all these problems, right?
>
> If, so I think we are wasting our time discussing sub-optimal ways to
> fix a specific problem where we already have a generic solution.
>
> Can't we just install Martin's patch and get closer to the pretest??

If RMS is fine with it.

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Richard Stallman]

    > Can't we just install Martin's patch and get closer to the pretest??

    If RMS is fine with it.

I was hoping to install a change that would be smaller and more
isolated to reduce the risk.  If people agree Martin's patch is
correct, then it is ok to install that.  I don't remember if he wrote
changes for the manual; if not, then would someone please do that?

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     > Can't we just install Martin's patch and get closer to the pretest??
>
>     If RMS is fine with it.
>
> I was hoping to install a change that would be smaller and more
> isolated to reduce the risk.  If people agree Martin's patch is
> correct, then it is ok to install that.

Done.

> I don't remember if he wrote changes for the manual; if not, then
> would someone please do that?

Done.  (Updated etc/NEWS too).

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Richard Stallman]

    `jit-lock-fontify-now' during scrolling, searching, window resizing.

I don't understand.  Could you be a bit less terse?

    Not mentioning the plethora of "hidden buffer changes" in c-mode.

What do you mean by "hidden buffer changes"?

     >     Better yet, introduce a macro `with-buffer-unmodified' like the one used in
     >     jit-lock, and make it manage a `buffer-really-modified-tick', kind of like
     >     the buffer-chars-modified-tick.  Then use it in jit-lock/font-lock
     >     and friends.
     >
     > That seems a lot more trouble, and I don't see that it is needed.

    I think it is needed.

You can try to demonstrate it.

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[martin rudalics]

 > What do you mean by "hidden buffer changes"?

 From `cc-engine.el':

;; Hidden buffer changes
;;
;; Various functions in CC Mode use text properties for caching and
;; syntactic markup purposes, and those of them that might modify such
;; properties but still don't modify the buffer in a visible way are
;; said to do "hidden buffer changes".  They should be used within
;; `c-save-buffer-state' or a similar function that saves and restores
;; buffer modifiedness, disables buffer change hooks, etc.

For example, `c-beginning-of-defun' and `c-end-of-defun' may trigger
imenu to scan the buffer.  (On my Emacs, they do so sometimes twice in
one and the same call).  Maybe Alan could elaborate on this.  Meanwhile,
grepping for "hidden buffer change" in the progmodes directory should
get you some 150 hits.

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Richard Stallman]

    For example, `c-beginning-of-defun' and `c-end-of-defun' may trigger
    imenu to scan the buffer.

These changes are made by editing commands, so they won't happen
repeatedly while Emacs is idle.  Thus, they are not the same sort of
grave problem as stealth fontification.

Howver, I agree it would be good to handle them for imenu.  Modifying
c-save-buffer-state could be an easy way to do that, but I agree that
a general solution such as buffer-chars-modified-tick would be nice.

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Stefan Monnier]

>     Better yet, introduce a macro `with-buffer-unmodified' like the one
>     used in jit-lock, and make it manage a `buffer-really-modified-tick',
>     kind of like the buffer-chars-modified-tick.  Then use it in
>     jit-lock/font-lock and friends.

> That seems a lot more trouble, and I don't see that it is needed.

I suspect it's actually going to be less trouble ;-)
Or at least no worse: the code would simply be moved from jit-lock to this
new macro, and then the macro can be used anywhere, e.g. in jit-lock (where
we can remove the corresponding macro), font-lock (idem), and probably many
other places where we use restore-buffer-modified-p.

It'd end up providing something similar to Martin's
buffer-chars-modified-tick, except implemented in Lisp rather than in C.


        Stefan

Re: [rudalics@gmx.at: Re: jit lock sit-for provokes redisplay provokes imenu]

[Richard Stallman]

    Or at least no worse: the code would simply be moved from jit-lock to this
    new macro, and then the macro can be used anywhere, e.g. in jit-lock (where
    we can remove the corresponding macro), font-lock (idem), and probably many
    other places where we use restore-buffer-modified-p.

    It'd end up providing something similar to Martin's
    buffer-chars-modified-tick, except implemented in Lisp rather than in C.

Please give it a try if you want to; we can see what it looks like.