unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#45248: 28.0.50; Emacs freezes with big C functions
@ 2020-12-15  3:59 Ravine Var
  2020-12-15 16:05 ` Eli Zaretskii
  2020-12-17 15:08 ` Alan Mackenzie
  0 siblings, 2 replies; 9+ messages in thread
From: Ravine Var @ 2020-12-15  3:59 UTC (permalink / raw)
  To: 45248

Scrolling through large functions freezes Emacs. For example,
the function proto_register_rrc(void) extracted from Wireshark
shows the issue.

https://gitlab.com/wireshark/wireshark/-/raw/master/epan/dissectors/packet-rrc.c

Profile report with emacs -Q:

https://gist.github.com/ravine-var/9136ed2608a08f65111bdc2fc531ccf5

In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, cairo version 1.17.4)
 of 2020-12-15 built on ryzen-mach
Repository revision: fd4297b25a61b33340ef312355748512e702bc2c
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Arch Linux

Configured using:
 'configure --prefix=/usr/local --without-rsvg --with-x-toolkit=no
 --without-lcms2 --without-libsystemd --without-dbus --without-gsettings
 --without-selinux --with-sound=no --enable-link-time-optimization
 'CFLAGS=-O2 -march=native''

Configured features:
XPM JPEG TIFF GIF PNG CAIRO NOTIFY INOTIFY ACL GNUTLS LIBXML2 FREETYPE
HARFBUZZ ZLIB OLDXMENU X11 XDBE XIM MODULES THREADS PDUMPER

Important settings:
  value of $LANG: en_IN.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Dired by name

Minor modes in effect:
  save-place-mode: t
  savehist-mode: t
  dired-omit-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: 1
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
~/packages/xcscope/xcscope hides /usr/share/emacs/site-lisp/xcscope
~/packages/dictionary-el/dictionary hides /usr/local/share/emacs/28.0.50/lisp/net/dictionary

Features:
(shadow face-remap emacsbug dired-aux cl-extra help-mode edmacro kmacro
emms-browser sort emms-playlist-sort emms-last-played emms-cache
emms-volume emms-volume-mixerctl emms-volume-pulse emms-volume-amixer
emms-info-libtag emms-info emms-later-do emms-player-simple
emms-playlist-mode emms-source-playlist emms-source-file locate emms
emms-compat saveplace savehist visual-fill-column mu4e-contrib eshell
esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups
esh-util bookmark pp mu4e mu4e-org mu4e-main mu4e-view thingatpt
mu4e-headers mu4e-compose mu4e-context mu4e-draft mu4e-actions ido
rfc2368 smtpmail sendmail mu4e-mark mu4e-proc mu4e-utils doc-view
jka-compr image-mode exif mu4e-lists mu4e-message shr kinsoku svg xml
dom browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util url-parse url-vars mailcap
flow-fill org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro
org-footnote org-src ob-comint org-pcomplete pcomplete comint ansi-color
org-list org-faces org-entities noutline outline easy-mmode org-version
ob-emacs-lisp ob-core ob-eval org-table ol org-keys org-compat advice
org-macs org-loaddefs format-spec find-func cal-menu calendar
cal-loaddefs mule-util hl-line mu4e-vars message rmc puny rfc822 mml
mml-sec epa derived epg epg-config gnus-util rmail rmail-loaddefs
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json map text-property-search time-date subr-x seq
byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies mm-encode
mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr
mailabbrev mail-utils gmm-utils mailheader cl-loaddefs cl-lib mu4e-meta
dictionary link connection cycbuf dired-x dired dired-loaddefs xcscope
ring easymenu iso-transl tooltip eldoc electric uniquify ediff-hook
vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core term/tty-colors frame
minibuffer cl-generic cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite charscript charprop case-table epa-hook jka-cmpr-hook help
simple abbrev obarray cl-preloaded nadvice button loaddefs faces
cus-face macroexp files window text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote threads inotify dynamic-setting font-render-setting cairo x
multi-tty make-network-process emacs)

Memory information:
((conses 16 145029 10616)
 (symbols 48 16107 1)
 (strings 32 54243 2983)
 (string-bytes 1 1799965)
 (vectors 16 28914)
 (vector-slots 8 327029 18380)
 (floats 8 151 158)
 (intervals 56 538 134)
 (buffers 984 12))





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#45248: 28.0.50; Emacs freezes with big C functions
  2020-12-15  3:59 bug#45248: 28.0.50; Emacs freezes with big C functions Ravine Var
@ 2020-12-15 16:05 ` Eli Zaretskii
  2020-12-17 15:08 ` Alan Mackenzie
  1 sibling, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2020-12-15 16:05 UTC (permalink / raw)
  To: Ravine Var; +Cc: 45248

> From: Ravine Var <ravine.var@gmail.com>
> Date: Tue, 15 Dec 2020 09:29:12 +0530
> 
> Scrolling through large functions freezes Emacs. For example,
> the function proto_register_rrc(void) extracted from Wireshark
> shows the issue.
> 
> https://gitlab.com/wireshark/wireshark/-/raw/master/epan/dissectors/packet-rrc.c

AFAICT, it's the 40K-line static data structure that starts around
line 160000.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#45248: 28.0.50; Emacs freezes with big C functions
  2020-12-15  3:59 bug#45248: 28.0.50; Emacs freezes with big C functions Ravine Var
  2020-12-15 16:05 ` Eli Zaretskii
@ 2020-12-17 15:08 ` Alan Mackenzie
  2020-12-18 11:17   ` Ravine Var
  1 sibling, 1 reply; 9+ messages in thread
From: Alan Mackenzie @ 2020-12-17 15:08 UTC (permalink / raw)
  To: Ravine Var; +Cc: acm, 45248

Hello, Ravine.

Thanks for reporting this bug, and for opening the bug report.

On Tue, Dec 15, 2020 at 09:29:12 +0530, Ravine Var wrote:
> Scrolling through large functions freezes Emacs. For example,
> the function proto_register_rrc(void) extracted from Wireshark
> shows the issue.

> https://gitlab.com/wireshark/wireshark/-/raw/master/epan/dissectors/packet-rrc.c

> Profile report with emacs -Q:

> https://gist.github.com/ravine-var/9136ed2608a08f65111bdc2fc531ccf5

> In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, cairo version 1.17.4)
>  of 2020-12-15 built on ryzen-mach
> Repository revision: fd4297b25a61b33340ef312355748512e702bc2c
> Repository branch: master
> Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
> System Description: Arch Linux

[ .... ]

As Eli surmised, the problem is the very large static struct in the
function.

The solution appears to be to cache positions within structs, so that
the function searching back over brace lists needn't do so repeatedly in
the same piece of buffer.

I'm hoping that the following patch which implements this cache will
solve the problem.  Would you please apply it to your copy of the master
branch, byte compile it, and try it out on various CC Mode buffers.
Then please let me know how well it has fixed the bug.  Thanks!




diff -r e43499573c2d cc-engine.el
--- a/cc-engine.el	Tue Dec 15 11:12:15 2020 +0000
+++ b/cc-engine.el	Thu Dec 17 14:57:35 2020 +0000
@@ -3574,8 +3574,9 @@
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Defuns which analyze the buffer, yet don't change `c-state-cache'.
 (defun c-get-fallback-scan-pos (here)
-  ;; Return a start position for building `c-state-cache' from
-  ;; scratch.  This will be at the top level, 2 defuns back.
+  ;; Return a start position for building `c-state-cache' from scratch.  This
+  ;; will be at the top level, 2 defuns back.  Return nil if we don't find
+  ;; these defun starts a reasonable way back.
   (save-excursion
     (save-restriction
       (when (> here (* 10 c-state-cache-too-far))
@@ -11627,6 +11628,195 @@
     (or (looking-at c-brace-list-key)
 	(progn (goto-char here) nil))))
 
+(defun c-laomib-loop (lim)
+  ;; The "expensive" loop from `c-looking-at-or-maybe-in-bracelist'.  Move
+  ;; backwards over comma separated sexps as far as possible, but no further
+  ;; than LIM, which may be nil, meaning no limit.  Return the final value of
+  ;; `braceassignp', which is t if we encountered "= {", usually nil
+  ;; otherwise.
+  (let ((braceassignp 'dontknow)
+	  (class-key
+	   ;; Pike can have class definitions anywhere, so we must
+	   ;; check for the class key here.
+	   (and (c-major-mode-is 'pike-mode)
+		c-decl-block-key)))
+    (while (eq braceassignp 'dontknow)
+      (cond ((eq (char-after) ?\;)
+	     (setq braceassignp nil))
+	    ((and class-key
+		  (looking-at class-key))
+	     (setq braceassignp nil))
+	    ((and c-has-compound-literals
+		  (looking-at c-return-key))
+	     (setq braceassignp t)
+	     nil)
+	    ((eq (char-after) ?=)
+	     ;; We've seen a =, but must check earlier tokens so
+	     ;; that it isn't something that should be ignored.
+	     (setq braceassignp 'maybe)
+	     (while (and (eq braceassignp 'maybe)
+			 (zerop (c-backward-token-2 1 t lim)))
+	       (setq braceassignp
+		     (cond
+		      ;; Check for operator =
+		      ((and c-opt-op-identifier-prefix
+			    (looking-at c-opt-op-identifier-prefix))
+		       nil)
+		      ;; Check for `<opchar>= in Pike.
+		      ((and (c-major-mode-is 'pike-mode)
+			    (or (eq (char-after) ?`)
+				;; Special case for Pikes
+				;; `[]=, since '[' is not in
+				;; the punctuation class.
+				(and (eq (char-after) ?\[)
+				     (eq (char-before) ?`))))
+		       nil)
+		      ((looking-at "\\s.") 'maybe)
+		      ;; make sure we're not in a C++ template
+		      ;; argument assignment
+		      ((and
+			(c-major-mode-is 'c++-mode)
+			(save-excursion
+			  (let ((here (point))
+				(pos< (progn
+					(skip-chars-backward "^<>")
+					(point))))
+			    (and (eq (char-before) ?<)
+				 (not (c-crosses-statement-barrier-p
+				       pos< here))
+				 (not (c-in-literal))
+				 ))))
+		       nil)
+		      (t t)))))
+	    ((and
+	      (c-major-mode-is 'c++-mode)
+	      (eq (char-after) ?\[)
+	      ;; Be careful of "operator []"
+	      (not (save-excursion
+		     (c-backward-token-2 1 nil lim)
+		     (looking-at c-opt-op-identifier-prefix))))
+	     (setq braceassignp t)
+	     nil))
+      (when (eq braceassignp 'dontknow)
+	(cond ((and
+		(not (eq (char-after) ?,))
+		(save-excursion
+		  (c-backward-syntactic-ws)
+		  (eq (char-before) ?})))
+	       (setq braceassignp nil))
+	      ((/= (c-backward-token-2 1 t lim) 0)
+	       (if (save-excursion
+		     (and c-has-compound-literals
+			  (eq (c-backward-token-2 1 nil lim) 0)
+			  (eq (char-after) ?\()))
+		   (setq braceassignp t)
+		 (setq braceassignp nil))))))
+    braceassignp))
+
+;; The following variable is a cache of up to four entries, each entry of
+;; which is a list representing a call to c-laomib-loop.  It contains the
+;; following elements:
+;; 0: `lim' argument - used as an alist key, never nil.
+;; 1: Position in buffer where the scan started.
+;; 2: Position in buffer where the scan ended.
+;; 3: Result of the call to `c-laomib-loop'.
+(defvar c-laomib-cache nil)
+(make-variable-buffer-local 'c-laomib-cache)
+
+(defun c-laomib-get-cache (containing-sexp)
+  ;; Get an element from `c-laomib-cache' matching CONTAINING-SEXP.
+  ;; Return that element or nil if one wasn't found.
+  (let ((elt (assq containing-sexp c-laomib-cache)))
+    (when elt
+      ;; Move the fetched `elt' to the front of the cache.
+      (setq c-laomib-cache (delq elt c-laomib-cache))
+      (push elt c-laomib-cache)
+      elt)))
+
+(defun c-laomib-put-cache (lim start end result)
+  ;; Insert a new element into `c-laomib-cache', removing another element to
+  ;; make room, if necessary.  The four parameters LIM, START, END, RESULT are
+  ;; the components of the new element (see comment for `c-laomib-cache').
+  ;; The return value is of no significance.
+  (when lim
+    (let ((old-elt (assq lim c-laomib-cache))
+	  ;; (elt (cons containing-sexp (cons start nil)))
+	  (new-elt (list lim start end result))
+	  big-ptr
+	  (cur-ptr c-laomib-cache)
+	  togo togo-ptr (size 0) cur-size
+	  )
+      (if old-elt (setq c-laomib-cache (delq old-elt c-laomib-cache)))
+
+      (while (>= (length c-laomib-cache) 4)
+	;; We delete the least recently used elt which doesn't enclose START,
+	;; or..
+	(dolist (elt c-laomib-cache)
+	  (if (or (<= start (cadr elt))
+		  (> start (car (cddr elt))))
+	      (setq togo elt)))
+
+	;; ... delete the least recently used elt which isn't the biggest.
+	(when (not togo)
+	  (while (cdr cur-ptr)
+	    (setq cur-size (- (nth 2 (cadr cur-ptr)) (car (cadr cur-ptr))))
+	    (when (> cur-size size)
+	      (setq size cur-size
+		    big-ptr cur-ptr))
+	    (setq cur-ptr (cdr cur-ptr)))
+	  (setq togo (if (cddr big-ptr)
+			 (car (last big-ptr))
+		       (car big-ptr))))
+
+	(setq c-laomib-cache (delq togo c-laomib-cache)))
+
+      (push new-elt c-laomib-cache))))
+
+(defun c-laomib-fix-elt (lwm elt paren-state)
+  ;; Correct a c-laomib-cache entry ELT with respect to buffer changes, either
+  ;; doing nothing, signalling it is to be deleted, or replacing its start
+  ;; point with one lower in the buffer than LWM.  PAREN-STATE is the paren
+  ;; state at LWM.  Return the corrected entry, or nil (if it needs deleting).
+  ;; Note that corrections are made by `setcar'ing the original structure,
+  ;; which thus remains intact.
+  (cond
+   ((or (not lwm) (> lwm (cadr elt)))
+    elt)
+   ((<= lwm (nth 2 elt))
+    nil)
+   (t
+    (let (cur-brace)
+      ;; Search for the last brace in `paren-state' before (car `lim').  This
+      ;; brace will become our new 2nd element of `elt'.
+      (while
+	  ;; Search one brace level per iteration.
+	  (and paren-state
+	       (progn
+		 ;; (setq cur-brace (c-laomib-next-BRACE paren-state))
+		 (while
+		     ;; Go past non-brace levels, one per iteration.
+		     (and paren-state
+			  (not (eq (char-after
+				    (c-state-cache-top-lparen paren-state))
+				   ?{)))
+		   (setq paren-state (cdr paren-state)))
+		 (cadr paren-state))
+	       (> (c-state-cache-top-lparen (cdr paren-state)) (car elt)))
+	(setq paren-state (cdr paren-state)))
+      (when (cadr paren-state)
+	(setcar (cdr elt) (c-state-cache-top-lparen paren-state))
+	elt)))))
+
+(defun c-laomib-invalidate-cache (beg _end)
+  ;; Called from late in c-before-change.  Amend `c-laomib-cache' to remove
+  ;; details pertaining to the buffer after position BEG.
+  (save-excursion
+    (goto-char beg)
+    (let ((paren-state (c-parse-state)))
+      (dolist (elt c-laomib-cache)
+	(when (not (c-laomib-fix-elt beg elt paren-state))
+	  (setq c-laomib-cache (delq elt c-laomib-cache)))))))
+
 (defun c-looking-at-or-maybe-in-bracelist (&optional containing-sexp lim)
   ;; Point is at an open brace.  If this starts a brace list, return a list
   ;; whose car is the buffer position of the start of the construct which
@@ -11647,11 +11837,6 @@
   ;; Here, "brace list" does not include the body of an enum.
   (save-excursion
     (let ((start (point))
-	  (class-key
-	   ;; Pike can have class definitions anywhere, so we must
-	   ;; check for the class key here.
-	   (and (c-major-mode-is 'pike-mode)
-		c-decl-block-key))
 	  (braceassignp 'dontknow)
 	  inexpr-brace-list bufpos macro-start res pos after-type-id-pos
 	  in-paren parens-before-brace)
@@ -11736,79 +11921,29 @@
 
        (t
 	(goto-char pos)
-	;; Checks to do on all sexps before the brace, up to the
-	;; beginning of the statement.
-	(while (eq braceassignp 'dontknow)
-	  (cond ((eq (char-after) ?\;)
-		 (setq braceassignp nil))
-		((and class-key
-		      (looking-at class-key))
-		 (setq braceassignp nil))
-		((and c-has-compound-literals
-		      (looking-at c-return-key))
-		 (setq braceassignp t)
-		 nil)
-		((eq (char-after) ?=)
-		 ;; We've seen a =, but must check earlier tokens so
-		 ;; that it isn't something that should be ignored.
-		 (setq braceassignp 'maybe)
-		 (while (and (eq braceassignp 'maybe)
-			     (zerop (c-backward-token-2 1 t lim)))
-		   (setq braceassignp
-			 (cond
-			  ;; Check for operator =
-			  ((and c-opt-op-identifier-prefix
-				(looking-at c-opt-op-identifier-prefix))
-			   nil)
-			  ;; Check for `<opchar>= in Pike.
-			  ((and (c-major-mode-is 'pike-mode)
-				(or (eq (char-after) ?`)
-				    ;; Special case for Pikes
-				    ;; `[]=, since '[' is not in
-				    ;; the punctuation class.
-				    (and (eq (char-after) ?\[)
-					 (eq (char-before) ?`))))
-			   nil)
-			  ((looking-at "\\s.") 'maybe)
-			  ;; make sure we're not in a C++ template
-			  ;; argument assignment
-			  ((and
-			    (c-major-mode-is 'c++-mode)
-			    (save-excursion
-			      (let ((here (point))
-				    (pos< (progn
-					    (skip-chars-backward "^<>")
-					    (point))))
-				(and (eq (char-before) ?<)
-				     (not (c-crosses-statement-barrier-p
-					   pos< here))
-				     (not (c-in-literal))
-				     ))))
-			   nil)
-			  (t t)))))
-		((and
-		  (c-major-mode-is 'c++-mode)
-		  (eq (char-after) ?\[)
-		 ;; Be careful of "operator []"
-		  (not (save-excursion
-			 (c-backward-token-2 1 nil lim)
-			 (looking-at c-opt-op-identifier-prefix))))
-		 (setq braceassignp t)
-		 nil))
-	  (when (eq braceassignp 'dontknow)
-	    (cond ((and
-		    (not (eq (char-after) ?,))
-		    (save-excursion
-		      (c-backward-syntactic-ws)
-		      (eq (char-before) ?})))
-		   (setq braceassignp nil))
-		  ((/= (c-backward-token-2 1 t lim) 0)
-		   (if (save-excursion
-			 (and c-has-compound-literals
-			      (eq (c-backward-token-2 1 nil lim) 0)
-			      (eq (char-after) ?\()))
-		       (setq braceassignp t)
-		     (setq braceassignp nil))))))
+	(when (eq braceassignp 'dontknow)
+	  (let* ((cache-entry (c-laomib-get-cache containing-sexp))
+		 (lim2 (or (cadr cache-entry) lim))
+		 sub-bassign-p)
+	    (if cache-entry
+		(cond
+		 ((<= (point) (cadr cache-entry))
+		  (goto-char (nth 2 cache-entry))
+		  (setq braceassignp (nth 3 cache-entry)))
+		 ((> (point) (cadr cache-entry))
+		  (setq sub-bassign-p (c-laomib-loop lim2))
+		  (if (<= (point) (cadr cache-entry))
+		      (progn
+			(c-laomib-put-cache containing-sexp
+					    start (nth 2 cache-entry)
+					    sub-bassign-p)
+			(setq braceassignp (nth 3 cache-entry))
+			(goto-char (nth 2 cache-entry)))
+		    (setq braceassignp sub-bassign-p)))
+		 (t))
+
+	      (setq braceassignp (c-laomib-loop lim))
+	      (c-laomib-put-cache lim start (point) braceassignp))))
 
 	(cond
 	 (braceassignp
diff -r e43499573c2d cc-mode.el
--- a/cc-mode.el	Tue Dec 15 11:12:15 2020 +0000
+++ b/cc-mode.el	Thu Dec 17 14:57:35 2020 +0000
@@ -627,6 +627,8 @@
   (make-local-variable 'fill-paragraph-function)
   (setq fill-paragraph-function 'c-fill-paragraph)
 
+  ;; Initialize the cache for `c-looking-at-or-maybe-in-bracelist'.
+  (setq c-laomib-cache nil)
   ;; Initialize the three literal sub-caches.
   (c-truncate-lit-pos-cache 1)
   ;; Initialize the cache of brace pairs, and opening braces/brackets/parens.
@@ -2034,7 +2036,9 @@
 		(if c-get-state-before-change-functions
 		    (mapc (lambda (fn)
 			    (funcall fn beg end))
-			  c-get-state-before-change-functions))))
+			  c-get-state-before-change-functions))
+
+		(c-laomib-invalidate-cache beg end)))
 	  (c-clear-string-fences))))
     (c-truncate-lit-pos-cache beg)
     ;; The following must be done here rather than in `c-after-change'
@@ -2183,7 +2187,8 @@
 	old-pos
 	(new-pos pos)
 	capture-opener
-	bod-lim bo-decl)
+	bod-lim bo-decl
+	paren-state containing-brace)
     (goto-char (c-point 'bol new-pos))
     (unless lit-start
       (setq bod-lim (c-determine-limit 500))
@@ -2202,12 +2207,16 @@
 	   (setq old-pos (point))
 	   (let (pseudo)
 	     (while
-		 (progn
-		   (c-syntactic-skip-backward "^;{}" bod-lim t)
-		   (and (eq (char-before) ?})
-			(save-excursion
-			  (backward-char)
-			  (setq pseudo (c-cheap-inside-bracelist-p (c-parse-state))))))
+		 (and
+		  ;; N.B. `c-syntactic-skip-backward' doesn't check (> (point)
+		  ;; lim) and can loop if that's not the case.
+		  (> (point) bod-lim)
+		  (progn
+		    (c-syntactic-skip-backward "^;{}" bod-lim t)
+		    (and (eq (char-before) ?})
+			 (save-excursion
+			   (backward-char)
+			   (setq pseudo (c-cheap-inside-bracelist-p (c-parse-state)))))))
 	       (goto-char pseudo))
 	     t)
 	   (> (point) bod-lim)
@@ -2240,7 +2249,14 @@
 		      (and (eq (char-before) ?{)
 			   (save-excursion
 			     (backward-char)
-			     (consp (c-looking-at-or-maybe-in-bracelist))))
+			     (setq paren-state (c-parse-state))
+			     (while
+				 (and
+				  (setq containing-brace
+					(c-pull-open-brace paren-state))
+				  (not (eq (char-after containing-brace) ?{))))
+			     (consp (c-looking-at-or-maybe-in-bracelist
+				     containing-brace containing-brace))))
 		      )))
 	   (not (bobp)))
 	(backward-char))		; back over (, [, <.


-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#45248: 28.0.50; Emacs freezes with big C functions
  2020-12-17 15:08 ` Alan Mackenzie
@ 2020-12-18 11:17   ` Ravine Var
  2020-12-18 17:29     ` Alan Mackenzie
  0 siblings, 1 reply; 9+ messages in thread
From: Ravine Var @ 2020-12-18 11:17 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 45248

Alan Mackenzie <acm@muc.de> writes:
> As Eli surmised, the problem is the very large static struct in the
> function.
>
> The solution appears to be to cache positions within structs, so that
> the function searching back over brace lists needn't do so repeatedly in
> the same piece of buffer.
>
> I'm hoping that the following patch which implements this cache will
> solve the problem.  Would you please apply it to your copy of the master
> branch, byte compile it, and try it out on various CC Mode buffers.
> Then please let me know how well it has fixed the bug.  Thanks!

The patch didn't apply cleanly on master, but it still went through.

progmodes $ patch -p1 < ~/Downloads/patches/bug_45248_message_11.mbox
patching file cc-engine.el
Hunk #1 succeeded at 3567 (offset -7 lines).
Hunk #2 succeeded at 11616 (offset -12 lines).
Hunk #3 succeeded at 11825 (offset -12 lines).
Hunk #4 succeeded at 11909 (offset -12 lines).
patching file cc-mode.el
Hunk #1 succeeded at 639 with fuzz 2 (offset 12 lines).
Hunk #2 succeeded at 2056 (offset 20 lines).
Hunk #3 succeeded at 2209 (offset 22 lines).
Hunk #4 succeeded at 2229 (offset 22 lines).
Hunk #5 succeeded at 2271 (offset 22 lines).


Testing with the large structure copied into a file showed
some initial improvement, but the screen still locked up
once scrolling reached 15% or so.

Enabling fast-but-imprecise-scrolling allowed scrolling
to happen, but there was lots of stuttering.

Profile report with emacs -Q on a Ryzen-based machine
(no fast-but-imprecise-scrolling):

https://gist.github.com/ravine-var/0116c20477dce725b02543a85e91cbab





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#45248: 28.0.50; Emacs freezes with big C functions
  2020-12-18 11:17   ` Ravine Var
@ 2020-12-18 17:29     ` Alan Mackenzie
  2020-12-19  4:28       ` Ravine Var
  0 siblings, 1 reply; 9+ messages in thread
From: Alan Mackenzie @ 2020-12-18 17:29 UTC (permalink / raw)
  To: Ravine Var; +Cc: 45248

Hello, Ravine.

Thanks again for the fast testing and reply.

On Fri, Dec 18, 2020 at 16:47:36 +0530, Ravine Var wrote:
> Alan Mackenzie <acm@muc.de> writes:
> > As Eli surmised, the problem is the very large static struct in the
> > function.

> > The solution appears to be to cache positions within structs, so that
> > the function searching back over brace lists needn't do so repeatedly in
> > the same piece of buffer.

> > I'm hoping that the following patch which implements this cache will
> > solve the problem.  Would you please apply it to your copy of the master
> > branch, byte compile it, and try it out on various CC Mode buffers.
> > Then please let me know how well it has fixed the bug.  Thanks!

> The patch didn't apply cleanly on master, but it still went through.

Sorry, I should have asked you to do $ cd lisp/progmodes first.  But I'm
glad you got it applied anyway.

> progmodes $ patch -p1 < ~/Downloads/patches/bug_45248_message_11.mbox

[ .... ]

> Testing with the large structure copied into a file showed
> some initial improvement, but the screen still locked up
> once scrolling reached 15% or so.

> Enabling fast-but-imprecise-scrolling allowed scrolling
> to happen, but there was lots of stuttering.

> Profile report with emacs -Q on a Ryzen-based machine
> (no fast-but-imprecise-scrolling):

> https://gist.github.com/ravine-var/0116c20477dce725b02543a85e91cbab

Thanks for the profile.  Noticeable in it is that
c-looking-at-or-maybe-in-bracelist isn't taking up masses of runtime any
more.  The profile is virtually identical to one I generated myself (on
my own Ryzen machine).

Let's be clear about the situation: CC Mode does just too much
processing in refontification for current hardware to be able to scroll
smoothly through a buffer at normal auto-repeat speeds.  Setting
fast-but-imprecise-scrolling to non-nil is a handy workaround, but it
isn't good enough to make the scrolling appear to be smooth when one
holds down C-v.  I think (and hope) this is what you mean by
"stuttering".

The bug which I'm hoping to have fixed was that in the large struct in
Wireshark's function proto_register_rrc, the scrolling got slower and
slower as one got further into the struct.  On my machine, 80% of the
way through that struct, scrolling a single screen (62 lines) was taking
between four and five seconds, which was unacceptably slow.  With the
bug fix, this scrolling takes about 75 milliseconds, regardless of the
position in the struct.  This is apart from the first screen drawing in
the neighbourhood, which takes a little time to fill the new cache
entry.

Could you please confirm that you see the same uniformity in scrolling
speed anywhere inside a large struct, and that the speed is acceptably
close to instantaneous for a single scrolling operation.  Or do you see
something different?  Thanks!

Maybe we can close this bug relatively soon.

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#45248: 28.0.50; Emacs freezes with big C functions
  2020-12-18 17:29     ` Alan Mackenzie
@ 2020-12-19  4:28       ` Ravine Var
  2020-12-22 20:40         ` Alan Mackenzie
  0 siblings, 1 reply; 9+ messages in thread
From: Ravine Var @ 2020-12-19  4:28 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 45248

Alan Mackenzie <acm@muc.de> writes:
> Let's be clear about the situation: CC Mode does just too much
> processing in refontification for current hardware to be able to scroll
> smoothly through a buffer at normal auto-repeat speeds.  Setting
> fast-but-imprecise-scrolling to non-nil is a handy workaround, but it
> isn't good enough to make the scrolling appear to be smooth when one
> holds down C-v.  I think (and hope) this is what you mean by
> "stuttering".

Yes, that's what I meant. These are my keyboard settings:

$ xset q | grep "repeat rate"
  auto repeat delay:  200    repeat rate:  30

> The bug which I'm hoping to have fixed was that in the large struct in
> Wireshark's function proto_register_rrc, the scrolling got slower and
> slower as one got further into the struct.  On my machine, 80% of the
> way through that struct, scrolling a single screen (62 lines) was taking
> between four and five seconds, which was unacceptably slow.  With the
> bug fix, this scrolling takes about 75 milliseconds, regardless of the
> position in the struct.  This is apart from the first screen drawing in
> the neighbourhood, which takes a little time to fill the new cache
> entry.
>
> Could you please confirm that you see the same uniformity in scrolling
> speed anywhere inside a large struct, and that the speed is acceptably
> close to instantaneous for a single scrolling operation.  Or do you see
> something different?  Thanks!

I still see the behavior that you have described. With the patch,
scrolling becomes slower and slower rapidly and the screen locks up
(using emacs -Q). I tested with a single C file containing just
the function proto_register_rrc().





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#45248: 28.0.50; Emacs freezes with big C functions
  2020-12-19  4:28       ` Ravine Var
@ 2020-12-22 20:40         ` Alan Mackenzie
  2020-12-23  3:18           ` Ravine Var
  0 siblings, 1 reply; 9+ messages in thread
From: Alan Mackenzie @ 2020-12-22 20:40 UTC (permalink / raw)
  To: Ravine Var; +Cc: 45248

Hello again, Ravine.

Sorry it's taken a little while to reply.  I've been preoccupied with
another bug in the meantime.

On Sat, Dec 19, 2020 at 09:58:48 +0530, Ravine Var wrote:
> Alan Mackenzie <acm@muc.de> writes:
> > Let's be clear about the situation: CC Mode does just too much
> > processing in refontification for current hardware to be able to scroll
> > smoothly through a buffer at normal auto-repeat speeds.  Setting
> > fast-but-imprecise-scrolling to non-nil is a handy workaround, but it
> > isn't good enough to make the scrolling appear to be smooth when one
> > holds down C-v.  I think (and hope) this is what you mean by
> > "stuttering".

> Yes, that's what I meant. These are my keyboard settings:

> $ xset q | grep "repeat rate"
>   auto repeat delay:  200    repeat rate:  30

> > The bug which I'm hoping to have fixed was that in the large struct in
> > Wireshark's function proto_register_rrc, the scrolling got slower and
> > slower as one got further into the struct.  On my machine, 80% of the
> > way through that struct, scrolling a single screen (62 lines) was taking
> > between four and five seconds, which was unacceptably slow.  With the
> > bug fix, this scrolling takes about 75 milliseconds, regardless of the
> > position in the struct.  This is apart from the first screen drawing in
> > the neighbourhood, which takes a little time to fill the new cache
> > entry.

> > Could you please confirm that you see the same uniformity in scrolling
> > speed anywhere inside a large struct, and that the speed is acceptably
> > close to instantaneous for a single scrolling operation.  Or do you see
> > something different?  Thanks!

> I still see the behavior that you have described.

Just to be sure what you're seeing, I take it you see this:
(i) Move point to a long way through the big struct in
proto_register_rrc, e.g. with C-u 2  M->;
(ii) Perform a single C-v, to ensure the caches are filled, and wait for
redisplay to complete;
(iii) Perform one or more C-v.  These each take several seconds before
redisplay is complete.

I do not see this happening on my machine.  Except that's not quite
true.  If I switch to M-x c++-mode in the file (e.g. by mistake), I see
the above slowdown.  I have a fix prepared for that, too.

> With the patch, scrolling becomes slower and slower rapidly and the
> screen locks up (using emacs -Q).

By "scrolling becomes slower", do you mean (1) that the time for an
isolated single full screen scroll becomes longer, the further you are
through that struct?  Or do you mean (2) that on auto-repeat of the C-v
or PageDown key, the scrolling becomes slower?

If you mean (1), I would ask you, yet again, to produce a profile,
following steps (i), (ii), and (iii) above, doing M-x profiler-start
after step (ii), then M-x profiler-report after step (iii).

If you mean (2), from emacs -Q without enabling
fast-but-imprecise-scrolling, then you are just seeing the normal
expected, but unfortunate behaviour of Emacs: Emacs will not redisplay a
screen as long as there is keyboard input unprocessed, but Emacs is
spending its time fontifying the intermediate screens (which are not
about to be displayed) as part of processing that input.  30 characters
per second is faster than CC Mode can paint a single screen.

For problem (2) I recommend, again, enabling
fast-but-imprecise-scrolling.

> I tested with a single C file containing just the function
> proto_register_rrc().

I have been testing with this, too.  :-)

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#45248: 28.0.50; Emacs freezes with big C functions
  2020-12-22 20:40         ` Alan Mackenzie
@ 2020-12-23  3:18           ` Ravine Var
  2020-12-24 11:50             ` Alan Mackenzie
  0 siblings, 1 reply; 9+ messages in thread
From: Ravine Var @ 2020-12-23  3:18 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 45248

Alan Mackenzie <acm@muc.de> writes:
> Sorry it's taken a little while to reply.  I've been preoccupied with
> another bug in the meantime.

No problem.

> By "scrolling becomes slower", do you mean (1) that the time for an
> isolated single full screen scroll becomes longer, the further you are
> through that struct?  Or do you mean (2) that on auto-repeat of the C-v
> or PageDown key, the scrolling becomes slower?

The second behavior. Opening the file and scrolling from the
the beginning with C-v becomes progressively slower.

> If you mean (2), from emacs -Q without enabling
> fast-but-imprecise-scrolling, then you are just seeing the normal
> expected, but unfortunate behaviour of Emacs: Emacs will not redisplay a
> screen as long as there is keyboard input unprocessed, but Emacs is
> spending its time fontifying the intermediate screens (which are not
> about to be displayed) as part of processing that input.  30 characters
> per second is faster than CC Mode can paint a single screen.

Ok.

> For problem (2) I recommend, again, enabling
> fast-but-imprecise-scrolling.

Without the patch in message 11, scrolling is bad
even when fast-but-imprecise-scrolling is enabled.

With the patch, things are much better and there
is no lockup, once fast-but-imprecise-scrolling is enabled.

I think this is enough for this bug - thanks for
looking into this issue.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#45248: 28.0.50; Emacs freezes with big C functions
  2020-12-23  3:18           ` Ravine Var
@ 2020-12-24 11:50             ` Alan Mackenzie
  0 siblings, 0 replies; 9+ messages in thread
From: Alan Mackenzie @ 2020-12-24 11:50 UTC (permalink / raw)
  To: Ravine Var; +Cc: 45248-done

Hello, Ravine.

On Wed, Dec 23, 2020 at 08:48:26 +0530, Ravine Var wrote:
> Alan Mackenzie <acm@muc.de> writes:
> > Sorry it's taken a little while to reply.  I've been preoccupied with
> > another bug in the meantime.

> No problem.

> > By "scrolling becomes slower", do you mean (1) that the time for an
> > isolated single full screen scroll becomes longer, the further you are
> > through that struct?  Or do you mean (2) that on auto-repeat of the C-v
> > or PageDown key, the scrolling becomes slower?

> The second behavior. Opening the file and scrolling from the
> the beginning with C-v becomes progressively slower.

> > If you mean (2), from emacs -Q without enabling
> > fast-but-imprecise-scrolling, then you are just seeing the normal
> > expected, but unfortunate behaviour of Emacs: Emacs will not redisplay a
> > screen as long as there is keyboard input unprocessed, but Emacs is
> > spending its time fontifying the intermediate screens (which are not
> > about to be displayed) as part of processing that input.  30 characters
> > per second is faster than CC Mode can paint a single screen.

> Ok.

> > For problem (2) I recommend, again, enabling
> > fast-but-imprecise-scrolling.

> Without the patch in message 11, scrolling is bad
> even when fast-but-imprecise-scrolling is enabled.

> With the patch, things are much better and there
> is no lockup, once fast-but-imprecise-scrolling is enabled.

> I think this is enough for this bug - thanks for
> looking into this issue.

Thanks!  I've found some infinite loops, which I've also corrected.  And
I've committed the patch to the savannah master branch.

So, I'm closing the bug with this post.  Thanks for all the testing and
profiling in sorting out this bug.

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-12-24 11:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-15  3:59 bug#45248: 28.0.50; Emacs freezes with big C functions Ravine Var
2020-12-15 16:05 ` Eli Zaretskii
2020-12-17 15:08 ` Alan Mackenzie
2020-12-18 11:17   ` Ravine Var
2020-12-18 17:29     ` Alan Mackenzie
2020-12-19  4:28       ` Ravine Var
2020-12-22 20:40         ` Alan Mackenzie
2020-12-23  3:18           ` Ravine Var
2020-12-24 11:50             ` Alan Mackenzie

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).