bug#16526: 24.3.50; scroll-conservatively & c-mode regression

[Alan Mackenzie <acm@muc.de>]

Hello, Eli, hello, Martin.

On Sun, Jan 26, 2014 at 08:43:10PM +0000, Alan Mackenzie wrote:
> On Sun, Jan 26, 2014 at 07:20:27PM +0200, Eli Zaretskii wrote:

> > You are right, sorry.  (I wasn't wrong, either: recenter does call the
> > same find_defun_start around EOB, which is what I saw.  But those
> > calls are very few and aren't responsible for the slowdown.  I also
> > wasn't wrong about point being at EOB, see below.  But I described
> > what happens incorrectly.)

> > Here's what I see in the debugger:

> > After beginning-of-buffer jumps to point-min, redisplay kicks in.
> > Since scroll-conservatively is set to a large value, redisplay first
> > tries to see whether it can bring point into view by scrolling the
> > window as little as possible.  It calls try_scrolling, which at some
> > point (around line 15000) tries to see whether the new location of
> > point is close enough to the current window start.  It does so by
> > calling move_it_to, which simulates the display.  While doing so,
> > move_it_to hits a portion of text with font-lock properties, and calls
> > JIT Lock to fontify them.

> > And here's where things go awry: For some reason, the CC Mode
> > fontification code decides it needs to scan the buffer backwards,
> > starting from EOB.

> The @dfn{state cache}, a list of certain brace/paren/bracket positions
> around some position, is set for a position near EOB.  With the switch to
> a different position, CC Mode tweaks the state cache rather than
> calculating it anew starting at BOB.  When the new position is nearer
> BOB, the code searches backwards for the appropriate braces.  However, it
> shouldn't be scanning the entire buffer backwards.  There is clearly a
> bug here.

> > So it goes temporarily to EOB (this is why I saw point being there),
> > and scans all the way back, I think in this loop from
> > c-append-lower-brace-pair-to-state-cache, which is called with its
> > first argument FROM set to EOB:

> > 	  ;; In the next pair of nested loops, the inner one moves back past a
> > 	  ;; pair of (mis-)matching parens or brackets; the outer one moves
> > 	  ;; back over a sequence of unmatched close brace/paren/bracket each
> > 	  ;; time round.
> > 	  (while
> > 	      (progn
> > 		(c-safe
> > 		  (while
> > 		      (and (setq ce (scan-lists bra -1 -1)) ; back past )/]/}; might signal
> > 			   (setq bra (scan-lists ce -1 1)) ; back past (/[/{; might signal
> > 			   (or (> bra here) ;(> ce here)
> > 			       (and
> > 				(< ce here)
> > 				(or (not (eq (char-after bra) ?\{))
> > 				    (and (goto-char bra)
> > 					 (c-beginning-of-macro)
> > 					 (< (point) macro-start-or-from))))))))
> > 		(and ce (< ce bra)))
> > 	    (setq bra ce))	; If we just backed over an unbalanced closing
> > 					; brace, ignore it.

> The backward scan-lists calls will be causing continual forward searches
> from BOB in syntax.c, every time the backward scan hits a comment ender.

> > This loop takes a lot of time, of course, and is a waste of time,
> > since eventually try_scrolling comes to the correct conclusion that
> > scrolling is impossible, and instead recenters at BOB.

> > Why does CC Mode decide to go from EOB backwards, I don't know;
> > presumably, this is decided by c-parse-state-get-strategy as part of
> > c-parse-state-1.

> Yes.  There is a bug here.  I have a strong suspicion where.

> [ .... ]

> > I hope this information will allow Alan to find the culprit and solve
> > the problem.

> Yes indeed, thanks.  But I'm not going to be able to resolve it in a
> scale of hours.  It's going to be days.  Sorry!

OK, here is a rough patch (smooth version to follow if it's any good),
which attempts to solve the problem by not calling
c-append-lower-brace-pair-to-state-cache in the pertinent circumstances.
Please try it out and let me know if it solves the problem (I still can't
reproduce the massive slowdown myself).



diff -r d6064f65b0a1 cc-engine.el
--- a/cc-engine.el	Sun Jan 19 12:09:59 2014 +0000
+++ b/cc-engine.el	Wed Jan 29 21:17:50 2014 +0000
@@ -2556,6 +2556,8 @@
   ;; o - ('forward START-POINT) - scan forward from START-POINT,
   ;;	 which is not less than the highest position in `c-state-cache' below here.
   ;; o - ('backward nil) - scan backwards (from HERE).
+  ;; o - ('back-and-forward START-POINT) - like 'forward, but when HERE is earlier
+  ;;     than GOOD-POS.
   ;; o - ('IN-LIT nil) - point is inside the literal containing point-min.
   (let ((cache-pos (c-get-cache-scan-pos here))	; highest position below HERE in cache (or 1)
 	strategy	    ; 'forward, 'backward, or 'IN-LIT.
@@ -2570,9 +2572,10 @@
      ((< (- good-pos here) (- here cache-pos)) ; FIXME!!! ; apply some sort of weighting.
       (setq strategy 'backward))
      (t
-      (setq strategy 'forward
+      (setq strategy 'back-and-forward
 	    start-point cache-pos)))
-    (list strategy (and (eq strategy 'forward) start-point))))
+    (list strategy ;(and (eq strategy 'forward)
+	  start-point)));)
 
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -2857,7 +2860,7 @@
   ;; adjust it to get outside a string/comment.	 (Sorry about this!  The code
   ;; needs to be FAST).
   ;;
-  ;; Return a list (GOOD-POS SCAN-BACK-POS PPS-STATE), where
+  ;; Return a list (GOOD-POS SCAN-BACK-POS PPS-STATE), where  FIXME!!!
   ;; o - GOOD-POS is a position where the new value `c-state-cache' is known
   ;;   to be good (we aim for this to be as high as possible);
   ;; o - SCAN-BACK-POS, if not nil, indicates there may be a brace pair
@@ -2892,6 +2895,7 @@
 	     pos
 	     upper-lim	   ; ,beyond which `c-state-cache' entries are removed
 	     scan-back-pos
+	     cons-separated
 	     pair-beg pps-point-state target-depth)
 
 	;; Remove entries beyond HERE.  Also remove any entries inside
@@ -2913,7 +2917,8 @@
 		   (consp (car c-state-cache))
 		   (> (cdar c-state-cache) upper-lim))
 	  (setcar c-state-cache (caar c-state-cache))
-	  (setq scan-back-pos (car c-state-cache)))
+	  (setq scan-back-pos (car c-state-cache)
+		cons-separated t))
 
 	;; The next loop jumps forward out of a nested level of parens each
 	;; time round; the corresponding elements in `c-state-cache' are
@@ -2986,7 +2991,8 @@
 	  (setq c-state-cache (cons (cons pair-beg pos)
 				    c-state-cache)))
 
-	(list pos scan-back-pos pps-state)))))
+	;(when (< pos scan-back-pos) (setq scan-back-pos nil)) ; 2014-01-26  FIXME!!!
+	(list pos scan-back-pos pps-state cons-separated)))))
 
 (defun c-remove-stale-state-cache-backwards (here)
   ;; Strip stale elements of `c-state-cache' by moving backwards through the
@@ -3271,6 +3277,7 @@
 		     ; are no open parens/braces between it and HERE.
 	 bopl-state
 	 res
+	 cons-separated
 	 scan-backward-pos scan-forward-p) ; used for 'backward.
     ;; If POINT-MIN has changed, adjust the cache
     (unless (= (point-min) c-state-point-min)
@@ -3283,13 +3290,15 @@
 
     ;; SCAN!
     (cond
-     ((eq strategy 'forward)
+     ((memq strategy '(forward back-and-forward)) ;(eq strategy 'forward)
       (setq res (c-remove-stale-state-cache start-point here here-bopl))
       (setq cache-pos (car res)
 	    scan-backward-pos (cadr res)
-	    bopl-state (car (cddr res))) ; will be nil if (< here-bopl
+	    bopl-state (car (cddr res)) ; will be nil if (< here-bopl
 					; start-point)
-      (if scan-backward-pos
+	    cons-separated (car (cdddr res)))
+      (if (and scan-backward-pos
+	       (or cons-separated (eq strategy 'forward))) ;scan-backward-pos
 	  (c-append-lower-brace-pair-to-state-cache scan-backward-pos here))
       (setq good-pos
 	    (c-append-to-state-cache cache-pos here))


-- 
Alan Mackenzie (Nuremberg, Germany).