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

[Alan Mackenzie]

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