GNU bug report logs - #25122
24.5; function describe-variable hangs on large variables

Previous Next

Package: emacs;

Reported by: Boruch Baum <boruch_baum <at> gmx.com>

Date: Tue, 6 Dec 2016 02:21:02 UTC

Severity: minor

Tags: fixed, patch

Merged with 13439, 21717

Found in versions 24.4.50, 24.5

Fixed in version 26.1

Done: npostavs <at> users.sourceforge.net

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25122 in the body.
You can then email your comments to 25122 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Tue, 06 Dec 2016 02:21:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Boruch Baum <boruch_baum <at> gmx.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 06 Dec 2016 02:21:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Boruch Baum <boruch_baum <at> gmx.com>
To: Emacs Bug Reporting <bug-gnu-emacs <at> gnu.org>
Subject: 24.5; function describe-variable hangs on large variables
Date: Mon, 5 Dec 2016 21:21:12 -0500
Subject: 24.5; function describe-variable hangs on large variables

1) When evaluating function describe-variable for variable
package-archive-conteqnts, emacs hangs for minutes before I gave up.

2) Aborting vua C-g works.

3) Viewing the buffer list revealed that a *Help* buffer had begun to be
created. Its content was "package-archive-contents is a variable defined
in `package.el'. Its value is " (new-lines removed).

4) If emacs is trying to stuff into that variable (and into that *Help*
buffer) all the archive information from the archive files of my
~/.emacs.d/elpa/archives/ tree, that would be about 730kb of elisp.

In GNU Emacs 24.5.1 (x86_64-pc-linux-gnu)
 of 2016-03-19 on trouble, modified by Debian
System Description:	Devuan GNU/Linux 1.0 (jessie)

Configured using:
 `configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.5/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.5/site-lisp:/usr/share/emacs/site-lisp
 --build x86_64-linux-gnu --prefix=/usr --sharedstatedir=/var/lib
 --libexecdir=/usr/lib --localstatedir=/var/lib
 --infodir=/usr/share/info --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.5/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.5/site-lisp:/usr/share/emacs/site-lisp
 --with-x=no --without-gconf --without-gsettings 'CFLAGS=-g -O2
 -fstack-protector-strong -Wformat -Werror=format-security -Wall'
 CPPFLAGS=-D_FORTIFY_SOURCE=2 LDFLAGS=-Wl,-z,relro'

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

Major mode: Emacs-Lisp

Minor modes in effect:
  global-anzu-mode: t
  anzu-mode: t
  ws-butler-mode: t
  dtrt-indent-mode: t
  clean-aindent-mode: t
  yas-minor-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  volatile-highlights-mode: t
  global-ede-mode: t
  ede-minor-mode: t
  global-semantic-idle-scheduler-mode: t
  global-semanticdb-minor-mode: t
  async-bytecomp-package-mode: t
  global-semantic-stickyfunc-mode: t
  semantic-mode: t
  helm-mode: t
  shell-dirtrack-mode: t
  projectile-mode: t
  global-company-mode: t
  company-mode: t
  override-global-mode: t
  winner-mode: t
  show-paren-mode: t
  savehist-mode: t
  desktop-save-mode: t
  delete-selection-mode: t
  tooltip-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t


Features:
(shadow sort mail-extr eieio-opt emacsbug helm-command tramp-cache
conf-mode org-element org-rmail org-mhe org-irc org-info org-gnus
org-docview doc-view image-mode org-bibtex bibtex org-bbdb org-w3m
misearch multi-isearch zygospore sh-script smie executable setup-editing
help-macro sgml-mode iedit-lib rect anzu mule-util ws-butler benchmark
dtrt-indent clean-aindent-mode yasnippet undo-tree diff
volatile-highlights ede/cpp-root ede/emacs setup-cedet ede/speedbar
ede/files ede ede/base ede/auto ede/source eieio-speedbar speedbar
sb-image dframe eieio-custom wid-edit semantic/idle semantic/format
ezimage semantic/tag-ls semantic/find semantic/ctxt semantic/db-mode
semantic/db eieio-base cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs setup-helm-gtags helm-gtags
subr-x pulse which-func setup-helm helm-projectile helm-config
async-bytecomp helm-imenu semantic/util-modes semantic/util semantic
semantic/tag semantic/lex semantic/fw mode-local cedet org org-macro
org-footnote org-pcomplete org-list org-faces org-entities noutline
outline org-version ob-sh ob-awk ob-latex ob-emacs-lisp ob ob-tangle
ob-ref ob-lob ob-table ob-exp org-src ob-keys ob-comint ob-core ob-eval
org-compat org-macs org-loaddefs cal-menu calendar cal-loaddefs imenu
helm-easymenu helm-mode helm-elisp helm-files tramp tramp-compat
tramp-loaddefs trampver shell pcomplete ffap helm-buffers helm-tags
helm-bookmark helm-locate helm-eval edebug eldoc helm-grep helm-regexp
helm-elscreen helm-adaptive helm-info info helm-types helm-external
helm-net browse-url xml helm-utils helm-help helm helm-source
helm-multi-match helm-lib smtpmail sendmail async setup-general windmove
projectile skeleton grep ibuf-ext thingatpt json epl rx company-oddmuse
company-keywords company-etags company-gtags company-dabbrev-code
company-files company-capf company-cmake company-xcode company-clang
company-semantic company-eclim company-css company-nxml company-bbdb
tempo ispell etags find-func company-dabbrev company-template company
tar-mode use-package cl diminish bind-key compile comint tool-bar
autoload lisp-mnt finder-inf mm-archive message rfc822 mml mml-sec
mailabbrev gmm-utils mailheader mm-decode mm-bodies mm-encode mail-utils
gnutls network-stream starttls url-http tls mail-parse rfc2231 rfc2047
rfc2045 ietf-drums url-gw url-cache url-auth url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
mailcap url-handlers url-parse auth-source eieio byte-opt bytecomp
byte-compile cl-extra cconv eieio-core gnus-util mm-util mail-prsvr
password-cache url-vars epg xterm server warnings dired-details+
dired-details help-mode advice help-fns dired+ image-dired image-file
image dired-x dired-aux dired winner ring pcase git-blame format-spec
package epg-config bookmark cl-macs gv derived pp jka-compr ibuf-macs
ibuffer paren woman man easymenu regexp-opt ansi-color edmacro kmacro
time-date savehist desktop frameset cl-loaddefs cl-lib elec-pair delsel
tango-dark-theme debian-el debian-el-loaddefs w3m-load emacs-goodies-el
emacs-goodies-custom emacs-goodies-loaddefs easy-mmode devhelp tooltip
electric uniquify ediff-hook vc-hooks lisp-float-type tabulated-list
newcomment lisp-mode prog-mode register page menu-bar rfn-eshadow timer
select mouse jit-lock font-lock syntax facemenu font-core frame cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple
abbrev minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
dbusbind gfilenotify multi-tty emacs)

Memory information:
((conses 16 582332 490741)
 (symbols 48 55652 20)
 (miscs 40 421 1843)
 (strings 32 146045 231132)
 (string-bytes 1 4264772)
 (vectors 16 55051)
 (vector-slots 8 1598839 288778)
 (floats 8 286 3294)
 (intervals 56 4525 912)
 (buffers 960 27)
 (heap 1024 62515 75886))

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Tue, 06 Dec 2016 06:42:01 GMT) Full text and rfc822 format available.

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Tue, 06 Dec 2016 07:41:13 +0100
Boruch Baum <boruch_baum <at> gmx.com> writes:

> Subject: 24.5; function describe-variable hangs on large variables
>
> 1) When evaluating function describe-variable for variable
> package-archive-conteqnts, emacs hangs for minutes before I gave up.

I have already reported this bug.
I use this to workaround it:

    (progn
        ;; Speedup `describe-variable' for variables with huge value description.
        (defun tv/describe-variable (old-fn &rest args)
          ;; `cl-flet' can't be used here because `pp' should
          ;; appear lexically in its body, which is not the case.
          ;; Using `flet' is an option, but even better is binding
          ;; (symbol-function 'pp) with `cl-letf'.
          (cl-letf (((symbol-function 'pp)
                     (lambda (object &optional stream)
                       (let ((fn (lambda (ob &optional stream)
                                   (princ (pp-to-string ob)
                                          (or stream standard-output))
                                   (terpri)))
                             (print-circle t))
                         (if (consp object)
                             (progn
                               (insert "\n(")
                               (mapc fn object)
                               (cl-letf (((point) (1- (point))))
                                 (insert ")")))
                             (funcall fn object stream))))))
            (apply old-fn args)))
        (advice-add 'describe-variable :around #'tv/describe-variable))





Forcibly Merged 21717 25122. Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Tue, 06 Dec 2016 16:47:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Wed, 07 Dec 2016 03:50:02 GMT) Full text and rfc822 format available.

Message #13 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Tue, 06 Dec 2016 22:50:46 -0500
merge 13439 25122
quit

Thierry Volpiatto <thierry.volpiatto <at> gmail.com> writes:

> Boruch Baum <boruch_baum <at> gmx.com> writes:
>
>> Subject: 24.5; function describe-variable hangs on large variables
>>
>> 1) When evaluating function describe-variable for variable
>> package-archive-conteqnts, emacs hangs for minutes before I gave up.
>
> I have already reported this bug.
> I use this to workaround it:

I wonder if the suggestion in #13439 might also help?




Merged 13439 21717 25122. Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Wed, 07 Dec 2016 03:50:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Wed, 07 Dec 2016 08:59:02 GMT) Full text and rfc822 format available.

Message #18 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: npostavs <at> users.sourceforge.net
Cc: 25122 <at> debbugs.gnu.org, Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Wed, 07 Dec 2016 09:58:25 +0100
npostavs <at> users.sourceforge.net writes:

> merge 13439 25122
> quit
>
> Thierry Volpiatto <thierry.volpiatto <at> gmail.com> writes:
>
>> Boruch Baum <boruch_baum <at> gmx.com> writes:
>>
>>> Subject: 24.5; function describe-variable hangs on large variables
>>>
>>> 1) When evaluating function describe-variable for variable
>>> package-archive-conteqnts, emacs hangs for minutes before I gave up.
>>
>> I have already reported this bug.
>> I use this to workaround it:
>
> I wonder if the suggestion in #13439 might also help?

I don't think it would be as fast as printing one by one each elements.
What is slow is printing the whole object.
See how bookmark file is saved, it was taking more than one minute for
my bookmarks before printing one by one, now it is instant or nearly.
  
-- 
Thierry




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 11 Mar 2017 05:39:02 GMT) Full text and rfc822 format available.

Message #21 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sat, 11 Mar 2017 00:40:03 -0500
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thierry.volpiatto <at> gmail.com> writes:
>
> I don't think it would be as fast as printing one by one each elements.
> What is slow is printing the whole object.
> See how bookmark file is saved, it was taking more than one minute for
> my bookmarks before printing one by one, now it is instant or nearly.

Actually, it's the indent-sexp on the whole object that takes time.
Possibly we could sacrifice some indentation correctness if the printed
representation is big.

I've been attempting an alternate approach which prettyprints the object
while scanning it instead of the current way of printing and then
reindenting.  Without optimizing, it's about 3 times as fast as the
current pp (it's the pp-prin1 in the benchmarks below), though more than
3 times slower than your mapc pp trick.  On the other hand, it also
doesn't yet handle function-specific indentation or any compound
structure apart from lists, so I'm not sure if it will end up being much
faster.

(benchmark 1 '(with-temp-buffer (pp-prin1 long-list (current-buffer)) nil)) "Elapsed time: 3.391232s (0.565806s in 11 GCs)"
(benchmark 1 '(progn (pp-to-string long-list) nil))                         "Elapsed time: 9.988515s (0.148034s in 3 GCs)"
(benchmark 1 '(progn (with-output-to-string (mapc 'pp long-list)) nil))     "Elapsed time: 0.983493s (0.144424s in 3 GCs)"
(benchmark 1 '(progn (cl-prin1-to-string long-list) nil))                   "Elapsed time: 0.511617s (0.152483s in 3 GCs)"
(benchmark 1 '(progn (prin1-to-string long-list) nil))                      "Elapsed time: 0.029320s"

[v1-0001-Initial-draft-of-new-pretty-printer-Bug-25122.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 11 Mar 2017 15:23:01 GMT) Full text and rfc822 format available.

Message #24 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sat, 11 Mar 2017 10:21:53 -0500
>           (cl-letf (((symbol-function 'pp)
>                      (lambda (object &optional stream)
>                        (let ((fn (lambda (ob &optional stream)
>                                    (princ (pp-to-string ob)
>                                           (or stream standard-output))
>                                    (terpri)))
>                              (print-circle t))
>                          (if (consp object)
>                              (progn
>                                (insert "\n(")
>                                (mapc fn object)
>                                (cl-letf (((point) (1- (point))))
>                                  (insert ")")))
>                              (funcall fn object stream))))))

Hmm... I wonder why this would be faster.  In the past, the
implementation of `print-circle` had a poor complexity, but we fixed
that around Emacs-24, IIRC so it now uses a hash-table and should have
O(n) complexity, which means that pp shouldn't be slower than (mapc
#'pp).


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 11 Mar 2017 15:35:01 GMT) Full text and rfc822 format available.

Message #27 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: npostavs <at> users.sourceforge.net
Cc: 25122 <at> debbugs.gnu.org, Boruch Baum <boruch_baum <at> gmx.com>,
 Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sat, 11 Mar 2017 10:33:44 -0500
> Actually, it's the indent-sexp on the whole object that takes time.
> Possibly we could sacrifice some indentation correctness if the printed
> representation is big.

Actually, I think that trying to optimize this is bound to fail: we want
*Help* to show up "instantly", so we're off by a factor of more
than 100, which seems way out of reach (unless there's really an
algorithmic problem in our code, admittedly).

I think the better way out is to do less work.  E.g. bound the total
amount printed (and replace the rest with "..." that can be expanded on
demand).


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 11 Mar 2017 15:35:03 GMT) Full text and rfc822 format available.

Message #30 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 25122 <at> debbugs.gnu.org, Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sat, 11 Mar 2017 10:35:24 -0500
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>           (cl-letf (((symbol-function 'pp)
>>                      (lambda (object &optional stream)
>>                        (let ((fn (lambda (ob &optional stream)
>>                                    (princ (pp-to-string ob)
>>                                           (or stream standard-output))
>>                                    (terpri)))
>>                              (print-circle t))
>>                          (if (consp object)
>>                              (progn
>>                                (insert "\n(")
>>                                (mapc fn object)
>>                                (cl-letf (((point) (1- (point))))
>>                                  (insert ")")))
>>                              (funcall fn object stream))))))
>
> Hmm... I wonder why this would be faster.  In the past, the
> implementation of `print-circle` had a poor complexity, but we fixed
> that around Emacs-24, IIRC so it now uses a hash-table and should have
> O(n) complexity, which means that pp shouldn't be slower than (mapc
> #'pp).

I think it's because when we indent-sexp only on individual entries, we
don't parse as far back.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 11 Mar 2017 19:28:01 GMT) Full text and rfc822 format available.

Message #33 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 25122 <at> debbugs.gnu.org
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sat, 11 Mar 2017 20:26:56 +0100
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>           (cl-letf (((symbol-function 'pp)
>>                      (lambda (object &optional stream)
>>                        (let ((fn (lambda (ob &optional stream)
>>                                    (princ (pp-to-string ob)
>>                                           (or stream standard-output))
>>                                    (terpri)))
>>                              (print-circle t))
>>                          (if (consp object)
>>                              (progn
>>                                (insert "\n(")
>>                                (mapc fn object)
>>                                (cl-letf (((point) (1- (point))))
>>                                  (insert ")")))
>>                              (funcall fn object stream))))))
>
> Hmm... I wonder why this would be faster.

However it is fast, it is anyway better than the actual situation were e.g C-h v
load-history takes minutes in the best case to load the 1.8M help buffer.
With this code it takes around 1s.

> In the past, the implementation of `print-circle` had a poor
> complexity, but we fixed that around Emacs-24, IIRC so it now uses a
> hash-table and should have O(n) complexity, which means that pp
> shouldn't be slower than (mapc #'pp).

The code above run at the same speed with and without print-circle...

-- 
Thierry




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 11 Mar 2017 19:30:02 GMT) Full text and rfc822 format available.

Message #36 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 25122 <at> debbugs.gnu.org, Boruch Baum <boruch_baum <at> gmx.com>,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sat, 11 Mar 2017 20:29:37 +0100
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> I think the better way out is to do less work.  E.g. bound the total
> amount printed (and replace the rest with "..." that can be expanded on
> demand).

The problem will be here again when you hit RET on "..." 

-- 
Thierry




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 11 Mar 2017 19:35:02 GMT) Full text and rfc822 format available.

Message #39 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: npostavs <at> users.sourceforge.net
Cc: 25122 <at> debbugs.gnu.org, Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sat, 11 Mar 2017 20:34:09 +0100
npostavs <at> users.sourceforge.net writes:

> I've been attempting an alternate approach which prettyprints the object
> while scanning it instead of the current way of printing and then
> reindenting.  Without optimizing, it's about 3 times as fast as the
> current pp (it's the pp-prin1 in the benchmarks below), though more than
> 3 times slower than your mapc pp trick.  On the other hand, it also
> doesn't yet handle function-specific indentation or any compound
> structure apart from lists, so I'm not sure if it will end up being much
> faster.
>
> (benchmark 1 '(with-temp-buffer (pp-prin1 long-list (current-buffer)) nil)) "Elapsed time: 3.391232s (0.565806s in 11 GCs)"
> (benchmark 1 '(progn (pp-to-string long-list) nil))                         "Elapsed time: 9.988515s (0.148034s in 3 GCs)"
> (benchmark 1 '(progn (with-output-to-string (mapc 'pp long-list)) nil))     "Elapsed time: 0.983493s (0.144424s in 3 GCs)"
> (benchmark 1 '(progn (cl-prin1-to-string long-list) nil))                   "Elapsed time: 0.511617s (0.152483s in 3 GCs)"
> (benchmark 1 '(progn (prin1-to-string long-list) nil))                      "Elapsed time: 0.029320s"

Interesting, thanks to work on this.

-- 
Thierry




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 11 Mar 2017 21:58:02 GMT) Full text and rfc822 format available.

Message #42 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sat, 11 Mar 2017 16:59:05 -0500
Thierry Volpiatto <thierry.volpiatto <at> gmail.com> writes:

> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>> I think the better way out is to do less work.  E.g. bound the total
>> amount printed (and replace the rest with "..." that can be expanded on
>> demand).

Yes, there's no method that will achieve "instant" speeds for
load-history sized lists (well except for plain prin1, but that output
is hardly readable).

> The problem will be here again when you hit RET on "..." 

Perhaps we could run the printing in a thread and suspend it after
printing X lines.  Then hitting RET on "..." would just print another X
lines.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 11 Mar 2017 23:57:01 GMT) Full text and rfc822 format available.

Message #45 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: npostavs <at> users.sourceforge.net, Thierry Volpiatto
 <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: RE: bug#25122: 24.5; function describe-variable hangs on large
 variables
Date: Sat, 11 Mar 2017 15:55:50 -0800 (PST)
> >> I think the better way out is to do less work.  E.g. bound the total
> >> amount printed (and replace the rest with "..." that can be expanded on
> >> demand).
> 
> Yes, there's no method that will achieve "instant" speeds for
> load-history sized lists (well except for plain prin1, but that output
> is hardly readable).
> 
> > The problem will be here again when you hit RET on "..."
> 
> Perhaps we could run the printing in a thread and suspend it after
> printing X lines.  Then hitting RET on "..." would just print another X
> lines.

(Caveat: I'm not really following this thread.)

If you plan to do things like what I think you're suggesting,
which will make a user hit several keys (e.g. RET on ...
repeatedly) then please make that behavior _optional_.

Give users the _possibility_ to just print the whole thing,
without any other action (no prefix arg, no clicking ... etc.).

I generally don't mind waiting, for example, and I don't
want to have to hit ... and wait for a bit more - I want
the whole thing the first time.

And if/when you do show the value only partially, please
make that _obvious_ to users.  They should _immediately_
know that they are not seeing the full value.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sun, 12 Mar 2017 05:58:01 GMT) Full text and rfc822 format available.

Message #48 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: npostavs <at> users.sourceforge.net
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sun, 12 Mar 2017 06:57:41 +0100
npostavs <at> users.sourceforge.net writes:

> Perhaps we could run the printing in a thread

This is a good idea.

> and suspend it after printing X lines.

Instead just print something like "Computing foo value ..." and let finish
the thread, letting user reading and moving cursor in docstring while it
finishes, once done save-excursion and send message "Computing foo value
done".

> Then hitting RET on "..."  would just print another X lines.

I think like Drew that this would be annoying.


That said, what's the reason of choosing the slower approach to compute
value (in a thread or not) instead of using the approach described in
the advice I sent here which takes 1s to compute load-history instead of
3mn ? (I use this advice since one year now without any problems).

-- 
Thierry




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sun, 12 Mar 2017 14:08:01 GMT) Full text and rfc822 format available.

Message #51 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Boruch Baum <boruch_baum <at> gmx.com>,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sun, 12 Mar 2017 10:07:39 -0400
> That said, what's the reason of choosing the slower approach to compute
> value (in a thread or not) instead of using the approach described in
> the advice I sent here which takes 1s to compute load-history instead of
> 3mn ? (I use this advice since one year now without any problems).

Yes, we should definitely do that.  I'm still not sure why it's so much
faster: it indicates we have a performance bug in pp.el and all it take
is to fix it.


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sun, 12 Mar 2017 14:15:02 GMT) Full text and rfc822 format available.

Message #54 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sun, 12 Mar 2017 10:15:21 -0400
Thierry Volpiatto <thierry.volpiatto <at> gmail.com> writes:

> npostavs <at> users.sourceforge.net writes:
>
>> and suspend it after printing X lines.
>
> Instead just print something like "Computing foo value ..." and let finish
> the thread, letting user reading and moving cursor in docstring while it
> finishes, once done save-excursion and send message "Computing foo value
> done".

Threads aren't truly parallel.  The user can't do anything while the
thread is running.

>
>> Then hitting RET on "..."  would just print another X lines.
>
> I think like Drew that this would be annoying.

I wonder if we could just hook this into scrolling somehow?  So the
lines would only be printed when you scroll to look at them.

>
> That said, what's the reason of choosing the slower approach to compute
> value (in a thread or not) instead of using the approach described in
> the advice I sent here which takes 1s to compute load-history instead of
> 3mn ? (I use this advice since one year now without any problems).

As mentioned in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21717#8,
it breaks circularity.  Try describing this variable:

    (defvar circular-list
      (let ((l (number-sequence 1 100)))
        (setcdr (last l) l)
        l)
      "A circular list that has problems with (mapc 'pp val).")

We could probably achieve something similar without breaking circular
printing by not calling indent-sexp on the full list, but 1s is longer
than "instant" anyway (which is about 50ms or less) which is why I'm
exploring other options.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sun, 12 Mar 2017 15:00:03 GMT) Full text and rfc822 format available.

Message #57 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: npostavs <at> users.sourceforge.net, Thierry Volpiatto
 <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: RE: bug#25122: 24.5; function describe-variable hangs on large
 variables
Date: Sun, 12 Mar 2017 07:59:03 -0700 (PDT)
> >> Then hitting RET on "..."  would just print another X lines.
> >
> > I think like Drew that this would be annoying.
> 
> I wonder if we could just hook this into scrolling somehow?  So the
> lines would only be printed when you scroll to look at them.

I still would not like that behavior, so would would want to
opt out, personally.  If it take a minute to display *Help*
I can at least do something different (outside Emacs) during
that time.  When I'm scrolling I'm likely examining the value
as it scrolls, and I don't want to wait (scrolling in chunks
separated by delays).

E.g., I do `C-h v bookmark-alist' or `load-history' with a
large list.  I know that it will not display immediately,
so I don't grumble about that fact.  I'm free not to sit and
stare at the screen waiting for it to return.  What you
describe just chops up the wait into scrolled chunks.

> > That said, what's the reason of choosing the slower approach to compute
> > value (in a thread or not) instead of using the approach described in
> > the advice I sent here which takes 1s to compute load-history instead of
> > 3mn ? (I use this advice since one year now without any problems).
> 
> As mentioned in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21717#8,
> it breaks circularity.  Try describing this variable:
> 
>   (defvar circular-list
>     (let ((l (number-sequence 1 100))) (setcdr (last l) l) l) "")
> 
> We could probably achieve something similar without breaking circular
> printing by not calling indent-sexp on the full list, but 1s is longer
> than "instant" anyway (which is about 50ms or less) which is why I'm
> exploring other options.

1 sec is not a problem, IMO.  0.7 sec is a typical Emacs
`sit-for' value, i.e., something that, yes, allows time to
notice the change/wait, but it is not so long that it
becomes annoying.

(It would be annoying if it happened for all or most *Help*
displays, but printing large values is the exception.)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sun, 12 Mar 2017 16:07:01 GMT) Full text and rfc822 format available.

Message #60 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sun, 12 Mar 2017 12:07:43 -0400
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thierry.volpiatto <at> gmail.com> writes:

> npostavs <at> users.sourceforge.net writes:
>
>> I've been attempting an alternate approach which prettyprints the object
>> while scanning it instead of the current way of printing and then
>> reindenting.  Without optimizing, it's about 3 times as fast as the
>> current pp (it's the pp-prin1 in the benchmarks below), though more than
>> 3 times slower than your mapc pp trick.  On the other hand, it also
>> doesn't yet handle function-specific indentation or any compound
>> structure apart from lists, so I'm not sure if it will end up being much
>> faster.
>>
>> (benchmark 1 '(with-temp-buffer (pp-prin1 long-list (current-buffer)) nil)) "Elapsed time: 3.391232s (0.565806s in 11 GCs)"
>> (benchmark 1 '(progn (pp-to-string long-list) nil))                         "Elapsed time: 9.988515s (0.148034s in 3 GCs)"
>> (benchmark 1 '(progn (with-output-to-string (mapc 'pp long-list)) nil))     "Elapsed time: 0.983493s (0.144424s in 3 GCs)"
>> (benchmark 1 '(progn (cl-prin1-to-string long-list) nil))                   "Elapsed time: 0.511617s (0.152483s in 3 GCs)"
>> (benchmark 1 '(progn (prin1-to-string long-list) nil))                      "Elapsed time: 0.029320s"
>
> Interesting, thanks to work on this.

With a couple of minor optimizations it's down to about 1.8s.  The first
is to reuse the tempbuffer instead of letting cl-prin1-to-string make a
new one each time.  Second is to add

    (eval-when-compile
      (cl-proclaim '(optimize (speed 3) (safety 0))))

This also stops these warnings (I guess they're caused by the "safety" code?):

../../emacs-master/lisp/emacs-lisp/pp.el:159:19:Warning: value returned from
    (aref state 6) is unused
../../emacs-master/lisp/emacs-lisp/pp.el:204:24:Warning: value returned from
    (aref state 10) is unused

New times:

(benchmark 1 '(with-temp-buffer (pp-prin1 long-list (current-buffer)) nil)) "Elapsed time: 1.800146s (0.231706s in 6 GCs)"
(benchmark 1 '(progn (pp-to-string long-list) nil))                         "Elapsed time: 9.950225s (0.154100s in 4 GCs)"
(benchmark 1 '(progn (with-output-to-string (mapc 'pp long-list)) nil))     "Elapsed time: 0.980923s (0.149787s in 4 GCs)"

I foolishly neglected to write down what exactly long-list was before,
starting from emacs -Q this seems to approximate it though:

(progn (require 'pp)
       (require 'dabbrev)
       (require 'edebug)
       (require 'cc-mode)
       (require 'vc)
       (setq long-list load-history)
       (length long-list)) ;=> 142

[v2-0001-New-pretty-printer-Bug-25122.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sun, 12 Mar 2017 16:30:02 GMT) Full text and rfc822 format available.

Message #63 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: npostavs <at> users.sourceforge.net
Cc: 25122 <at> debbugs.gnu.org, Boruch Baum <boruch_baum <at> gmx.com>,
 Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sun, 12 Mar 2017 12:29:16 -0400
> Threads aren't truly parallel.  The user can't do anything while the
> thread is running.

I think the idea is to build the printout while the user is reading the
docstring (for example).  The loop that prints would have regular
`yield` calls so that as soon as not to block other operations.

> I wonder if we could just hook this into scrolling somehow?

We definitely could (at least, there's no fundamental reason why we
can't insert the text from a jit-lock thingy, although modifying the
buffer text from jit-lock might trigger some latent bugs).

> As mentioned in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21717#8,
> it breaks circularity.  Try describing this variable:

>     (defvar circular-list
>       (let ((l (number-sequence 1 100)))
>         (setcdr (last l) l)
>         l)
>       "A circular list that has problems with (mapc 'pp val).")

That's why I think the (mapc #'pp) trick is an interesting observation
(it points to a performance bug that should be easy to fix) but is not
a solution in itself.


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sun, 12 Mar 2017 16:32:01 GMT) Full text and rfc822 format available.

Message #66 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sun, 12 Mar 2017 12:32:48 -0400
> We could probably achieve something similar without breaking circular
> printing by not calling indent-sexp on the full list

The following patch is almost as fast as the mapc 'pp trick, but doesn't
get stuck on circular lists.  The indentation is a bit off: sublists
after the first one incorrectly start in column 0.  Also, this doesn't
really solve the performance problem, it just makes it much less likely
to occur, e.g., (pp (list load-history)) is still slow.

---   i/lisp/emacs-lisp/pp.el
+++   w/lisp/emacs-lisp/pp.el
@@ -76,9 +76,15 @@ pp-buffer
        (progn (skip-chars-forward " \t\n") (point)))
       (insert ?\n))
      (t (goto-char (point-max)))))
   (goto-char (point-min))
-  (indent-sexp))
+  (condition-case () (down-list)
+    (scan-error nil))
+  (while (and (not (eobp))
+              (condition-case () (progn (indent-sexp)
+                                        (forward-sexp)
+                                        t)
+                (scan-error nil)))))
 
 ;;;###autoload
 (defun pp (object &optional stream)
   "Output the pretty-printed representation of OBJECT, any Lisp object.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Mon, 13 Mar 2017 04:47:02 GMT) Full text and rfc822 format available.

Message #69 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Mon, 13 Mar 2017 00:47:38 -0400
[Message part 1 (text/plain, inline)]
tags 25122 patch
quit

npostavs <at> users.sourceforge.net writes:

> Also, this doesn't really solve the performance problem, it just makes
> it much less likely to occur, e.g., (pp (list load-history)) is still
> slow.

Okay, I think I found the real fix now:

[0001-Don-t-reparse-the-sexp-in-indent-sexp-Bug-25122.patch (text/x-diff, inline)]
From 5188d6e366426d83934505296b585744f50e24a5 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 12 Mar 2017 23:59:19 -0400
Subject: [PATCH] Don't reparse the sexp in indent-sexp (Bug#25122)

* lisp/emacs-lisp/lisp-mode.el (calculate-lisp-indent): Let
PARSE-START be a parse state that can be reused.
(indent-sexp): Pass the running parse state to calculate-lisp-indent
instead of the sexp beginning position.
---
 lisp/emacs-lisp/lisp-mode.el | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index eb07c18b03..8d4abc24e8 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -781,6 +781,10 @@ calculate-lisp-indent
 If the value is nil, that means don't change the indentation
 because the line starts inside a string.
 
+PARSE-START may be a buffer position to start parsing from, or a
+parse state as returned by calling `parse-partial-sexp' up to the
+beginning of the current line.
+
 The value can also be a list of the form (COLUMN CONTAINING-SEXP-START).
 This means that following lines at the same level of indentation
 should not necessarily be indented the same as this line.
@@ -794,12 +798,14 @@ calculate-lisp-indent
           (desired-indent nil)
           (retry t)
           calculate-lisp-indent-last-sexp containing-sexp)
-      (if parse-start
-          (goto-char parse-start)
-          (beginning-of-defun))
-      ;; Find outermost containing sexp
-      (while (< (point) indent-point)
-        (setq state (parse-partial-sexp (point) indent-point 0)))
+      (cond ((or (markerp parse-start) (integerp parse-start))
+             (goto-char parse-start))
+            ((null parse-start) (beginning-of-defun))
+            (t (setq state parse-start)))
+      (unless state
+        ;; Find outermost containing sexp
+        (while (< (point) indent-point)
+          (setq state (parse-partial-sexp (point) indent-point 0))))
       ;; Find innermost containing sexp
       (while (and retry
 		  state
@@ -1070,11 +1076,6 @@ indent-sexp
 ENDPOS is encountered."
   (interactive)
   (let* ((indent-stack (list nil))
-         ;; If ENDPOS is non-nil, use beginning of defun as STARTING-POINT.
-         ;; If ENDPOS is nil, it is safe not to scan before point
-         ;; since every line we indent is more deeply nested than point is.
-         (starting-point (save-excursion (if endpos (beginning-of-defun))
-                                         (point)))
          ;; Use `syntax-ppss' to get initial state so we don't get
          ;; confused by starting inside a string.  We don't use
          ;; `syntax-ppss' in the loop, because this is measurably
@@ -1132,8 +1133,12 @@ indent-sexp
           (unless (or (eolp) (eq (char-syntax (char-after)) ?<))
             (let ((this-indent (car indent-stack)))
               (when (listp this-indent)
-                (let ((val (calculate-lisp-indent
-                            (or (car this-indent) starting-point))))
+                ;; The state here is actually to the end of the
+                ;; previous line, but that's fine for our purposes.
+                ;; And continuing the parse to the next line would
+                ;; destroy element 2 (last sexp position) which
+                ;; `calculate-lisp-indent' needs.
+                (let ((val (calculate-lisp-indent state)))
                   (setq
                    this-indent
                    (cond ((integerp val)
-- 
2.11.1


Added tag(s) patch. Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Mon, 13 Mar 2017 04:47:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Mon, 13 Mar 2017 14:01:01 GMT) Full text and rfc822 format available.

Message #74 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Mon, 13 Mar 2017 10:01:13 -0400
[Message part 1 (text/plain, inline)]
npostavs <at> users.sourceforge.net writes:

> npostavs <at> users.sourceforge.net writes:
>
>> Also, this doesn't really solve the performance problem, it just makes
>> it much less likely to occur, e.g., (pp (list load-history)) is still
>> slow.
>
> Okay, I think I found the real fix now:

Missed a corner case.

[v2-0001-Don-t-reparse-the-sexp-in-indent-sexp-Bug-25122.patch (text/x-diff, inline)]
From b8dd372f65ce8979324c00b12a9ae767c1ffabda Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 12 Mar 2017 23:59:19 -0400
Subject: [PATCH v2] Don't reparse the sexp in indent-sexp (Bug#25122)

* lisp/emacs-lisp/lisp-mode.el (calculate-lisp-indent): Let
PARSE-START be a parse state that can be reused.
(indent-sexp): Pass the running parse state to calculate-lisp-indent
instead of the sexp beginning position.  Saving the
CONTAINING-SEXP-START returned by `calculate-lisp-indent' is no longer
needed.
* test/lisp/emacs-lisp/lisp-mode-tests.el (indent-sexp): Add blank
line to test case.
---
 lisp/emacs-lisp/lisp-mode.el            | 71 ++++++++++++++++++---------------
 test/lisp/emacs-lisp/lisp-mode-tests.el |  5 ++-
 2 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index eb07c18b03..3fefb69066 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -781,6 +781,10 @@ calculate-lisp-indent
 If the value is nil, that means don't change the indentation
 because the line starts inside a string.
 
+PARSE-START may be a buffer position to start parsing from, or a
+parse state as returned by calling `parse-partial-sexp' up to the
+beginning of the current line.
+
 The value can also be a list of the form (COLUMN CONTAINING-SEXP-START).
 This means that following lines at the same level of indentation
 should not necessarily be indented the same as this line.
@@ -794,12 +798,14 @@ calculate-lisp-indent
           (desired-indent nil)
           (retry t)
           calculate-lisp-indent-last-sexp containing-sexp)
-      (if parse-start
-          (goto-char parse-start)
-          (beginning-of-defun))
-      ;; Find outermost containing sexp
-      (while (< (point) indent-point)
-        (setq state (parse-partial-sexp (point) indent-point 0)))
+      (cond ((or (markerp parse-start) (integerp parse-start))
+             (goto-char parse-start))
+            ((null parse-start) (beginning-of-defun))
+            (t (setq state parse-start)))
+      (unless state
+        ;; Find outermost containing sexp
+        (while (< (point) indent-point)
+          (setq state (parse-partial-sexp (point) indent-point 0))))
       ;; Find innermost containing sexp
       (while (and retry
 		  state
@@ -1070,11 +1076,6 @@ indent-sexp
 ENDPOS is encountered."
   (interactive)
   (let* ((indent-stack (list nil))
-         ;; If ENDPOS is non-nil, use beginning of defun as STARTING-POINT.
-         ;; If ENDPOS is nil, it is safe not to scan before point
-         ;; since every line we indent is more deeply nested than point is.
-         (starting-point (save-excursion (if endpos (beginning-of-defun))
-                                         (point)))
          ;; Use `syntax-ppss' to get initial state so we don't get
          ;; confused by starting inside a string.  We don't use
          ;; `syntax-ppss' in the loop, because this is measurably
@@ -1093,16 +1094,21 @@ indent-sexp
     (save-excursion
       (while (< (point) endpos)
         ;; Parse this line so we can learn the state to indent the
-        ;; next line.
-        (while (progn
-                 (setq state (parse-partial-sexp
-                              last-syntax-point (progn (end-of-line) (point))
-                              nil nil state))
-                 ;; Skip over newlines within strings.
-                 (nth 3 state))
-          (setq state (parse-partial-sexp (point) (point-max)
-                                          nil nil state 'syntax-table))
-          (setq last-syntax-point (point)))
+        ;; next line.  Preserve element 2 of the state (last sexp) for
+        ;; `calculate-lisp-indent'.
+        (let ((last-sexp (nth 2 state)))
+          (while (progn
+                   (setq state (parse-partial-sexp
+                                last-syntax-point (progn (end-of-line) (point))
+                                nil nil state))
+                   (setq last-sexp (or (nth 2 state) last-sexp))
+                   ;; Skip over newlines within strings.
+                   (nth 3 state))
+            (setq state (parse-partial-sexp (point) (point-max)
+                                            nil nil state 'syntax-table))
+            (setq last-sexp (or (nth 2 state) last-sexp))
+            (setq last-syntax-point (point)))
+          (setf (nth 2 state) last-sexp))
         (setq next-depth (car state))
         ;; If the line contains a comment indent it now with
         ;; `indent-for-comment'.
@@ -1115,6 +1121,8 @@ indent-sexp
                                     (make-list (- init-depth next-depth) nil))
                 last-depth (- last-depth next-depth)
                 next-depth init-depth))
+        ;; Now indent the next line according to what we learned from
+        ;; parsing the previous one.
         (forward-line 1)
         (when (< (point) endpos)
           (let ((depth-delta (- next-depth last-depth)))
@@ -1124,28 +1132,25 @@ indent-sexp
                    (setq indent-stack (nconc (make-list depth-delta nil)
                                              indent-stack))))
             (setq last-depth next-depth))
-          ;; Now indent the next line according
-          ;; to what we learned from parsing the previous one.
-          (skip-chars-forward " \t")
           ;; But not if the line is blank, or just a comment (we
           ;; already called `indent-for-comment' above).
+          (skip-chars-forward " \t")
           (unless (or (eolp) (eq (char-syntax (char-after)) ?<))
-            (let ((this-indent (car indent-stack)))
-              (when (listp this-indent)
-                (let ((val (calculate-lisp-indent
-                            (or (car this-indent) starting-point))))
-                  (setq
-                   this-indent
+            (indent-line-to
+             (or (car indent-stack)
+                 ;; The state here is actually to the end of the
+                 ;; previous line, but that's fine for our purposes.
+                 ;; And parsing over the newline would only destroy
+                 ;; element 2 (last sexp position).
+                 (let ((val (calculate-lisp-indent state)))
                    (cond ((integerp val)
                           (setf (car indent-stack) val))
                          ((consp val) ; (COLUMN CONTAINING-SEXP-START)
-                          (setf (car indent-stack) (cdr val))
                           (car val))
                          ;; `calculate-lisp-indent' only returns nil
                          ;; when we're in a string, but this won't
                          ;; happen because we skip strings above.
-                         (t (error "This shouldn't happen!"))))))
-              (indent-line-to this-indent))))))))
+                         (t (error "This shouldn't happen!"))))))))))))
 
 (defun indent-pp-sexp (&optional arg)
   "Indent each line of the list starting just after point, or prettyprint it.
diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el
index 2801f23df6..848dd7ca3e 100644
--- a/test/lisp/emacs-lisp/lisp-mode-tests.el
+++ b/test/lisp/emacs-lisp/lisp-mode-tests.el
@@ -31,6 +31,9 @@
          1
        2)
    2)
+ (fun arg1
+
+      arg2)
  (1
   \"string
 noindent\" (\"string2
@@ -58,7 +61,7 @@
         (save-excursion
           (let ((n 0))
             (while (not (eobp))
-              (unless (looking-at "noindent")
+              (unless (looking-at "noindent\\|^[[:blank:]]*$")
                 (insert (make-string n ?\s)))
               (cl-incf n)
               (forward-line))))
-- 
2.11.1


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Thu, 16 Mar 2017 02:54:01 GMT) Full text and rfc822 format available.

Message #77 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Wed, 15 Mar 2017 22:54:24 -0400
[Message part 1 (text/plain, inline)]
>> npostavs <at> users.sourceforge.net writes:
>>
>> Okay, I think I found the real fix now:

Same issue with indent-region.  The gains are not so dramatic for
typical lisp code that has normal size sexps, but C-x h C-M-\ on
subr.el's text runs twice as fast for me with the new lisp-indent-region
function.

[v2-0002-Remove-ignored-argument-from-lisp-indent-line.patch (text/plain, attachment)]
[v2-0003-Add-new-lisp-indent-region-that-doesn-t-reparse-t.patch (text/plain, attachment)]
[v2-0004-lisp-emacs-lisp-lisp-mode.el-indent-sexp-Clean-up.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Tue, 18 Apr 2017 03:53:02 GMT) Full text and rfc822 format available.

Message #80 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Mon, 17 Apr 2017 23:53:30 -0400
[Message part 1 (text/plain, inline)]
npostavs <at> users.sourceforge.net writes:

>>> npostavs <at> users.sourceforge.net writes:
>>>
>>> Okay, I think I found the real fix now:
>
> Same issue with indent-region.  The gains are not so dramatic for
> typical lisp code that has normal size sexps, but C-x h C-M-\ on
> subr.el's text runs twice as fast for me with the new lisp-indent-region
> function.

Had some test failures due to some corner cases, should all be fixed
now.  I think this is the final patch set.  This also fixes
lisp-indent-region and lisp-indent-line to not indent string literal
contents.

I intend to close this bug as fixed after merging these, as this does
fix the performance bug.  I will probably pursue the alternate pretty
printer separately; it doesn't help performance (after the indent-sexp
performance is fixed), but gives prettier results in some cases.  The
threading idea may also be worth looking at, but can also be considered
separately, as this bug report is already long enough.

[v3-0001-Don-t-reparse-the-sexp-in-indent-sexp-Bug-25122.patch (text/plain, attachment)]
[v3-0002-lisp-emacs-lisp-lisp-mode.el-indent-sexp-Clean-up.patch (text/plain, attachment)]
[v3-0003-Remove-ignored-argument-from-lisp-indent-line.patch (text/plain, attachment)]
[v3-0004-Add-new-lisp-indent-region-that-doesn-t-reparse-t.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Sat, 22 Apr 2017 18:25:01 GMT) Full text and rfc822 format available.

Message #83 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
Cc: 25122 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Boruch Baum <boruch_baum <at> gmx.com>
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Sat, 22 Apr 2017 14:25:27 -0400
tags 25122 fixed
close 25122 26.1
quit

npostavs <at> users.sourceforge.net writes:
>
> I intend to close this bug as fixed after merging these, as this does
> fix the performance bug.

Pushed to master.

[1: 6fa9cc0593]: 2017-04-22 14:18:46 -0400
  ; Merge: improve indent-sexp and lisp-indent-region performance
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=6fa9cc0593150a318f0e08e69ec10672d548a7c1

[2: 4713dd425b]: 2017-04-22 14:09:58 -0400
  Add new `lisp-indent-region' that doesn't reparse the code.
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4713dd425beac5cb459704e67dcb8f6faf714375

[3: 2f6769f9cd]: 2017-04-22 14:09:58 -0400
  Remove ignored argument from lisp-indent-line
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=2f6769f9cdb799e880fdcc09057353a0a2349bfc

[4: 8bb5d7adaf]: 2017-04-22 14:09:57 -0400
  * lisp/emacs-lisp/lisp-mode.el (indent-sexp): Clean up marker.
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=8bb5d7adaf45264900385530c7f76175ba490a77

[5: 43c84577a3]: 2017-04-22 14:09:57 -0400
  Don't reparse the sexp in indent-sexp (Bug#25122)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=43c84577a3055d5ddf1f5d1b999e6ecca6139f60




Added tag(s) fixed. Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Sat, 22 Apr 2017 18:25:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 26.1, send any further explanations to 25122 <at> debbugs.gnu.org and Boruch Baum <boruch_baum <at> gmx.com> Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Sat, 22 Apr 2017 18:25:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Wed, 26 Apr 2017 03:59:02 GMT) Full text and rfc822 format available.

Message #90 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: npostavs <at> users.sourceforge.net
Cc: 25122 <at> debbugs.gnu.org
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Wed, 26 Apr 2017 05:57:47 +0200
Hi Noam,

> > I intend to close this bug as fixed after merging these, as this does
> > fix the performance bug.

I have a question: how much performance gain can I expect from your
changes?

My use case is the following: "el-search" has now an occur-like
operation mode.  It copies all matches (i.e. s-expressions) into an *El
Occur* buffer.  Matches can be tiny (like `1') or arbitrarily large.

Since most expressions don't start at column 0 in their source buffer
(unless they are top-level), I have to remove the amount of additional
indentation from all but the first line of a match.  Well - not from
all, that's where it gets complicated - there are cases where this is
wrong, like in strings and some comments.

To have a sane solution, I just used `indent-region' to reindent the
expression in the occur buffer.  This works great, but `indent-region'
was so slow that it took up to 50 percent of the whole running time of
the occur search.

With an naive alternative approach (just remove additional indentation
from lines where it makes sense (whereby finding out whether it "makes
sense" involves running syntax scanning)), I get an improvement of a
factor around five or ten relative to `indent-region' for reindenting.

Would you expect your improved `indent-region' is fast enough now so
that it makes sense that I switch back to it in my code?  Is the running
time now something close to O(number of lines to indent)?


Thanks,

Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#25122; Package emacs. (Wed, 26 Apr 2017 10:36:01 GMT) Full text and rfc822 format available.

Message #93 received at 25122 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: npostavs <at> users.sourceforge.net
Cc: 25122 <at> debbugs.gnu.org
Subject: Re: bug#25122: 24.5;
 function describe-variable hangs on large variables
Date: Wed, 26 Apr 2017 12:35:06 +0200
Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Would you expect your improved `indent-region' is fast enough now so
> that it makes sense that I switch back to it in my code?  Is the
> running time now something close to O(number of lines to indent)?

After some tests, I think it is fast enough for my scenario.  And I see
from the comments in the code that you addressed the complexity problem.

Michael.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 24 May 2017 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 310 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.