Be more efficient when buffering output in Eshell

This makes the built-in 'eshell/cat' 5-10x faster on large files in my
(somewhat limited) tests.  In addition, this change periodically
redisplays when using the Eshell buffered output so that users can see
some progress.

* lisp/eshell/esh-io.el (eshell-print-queue-size, eshell-print-queue,
eshell-print-queue-count): Make obsolete in favor of...
(eshell-buffered-print-size, eshell--buffered-print-queue)
(eshell--buffered-print-current-size): ... these.
(eshell-buffered-print-redisplay-throttle): New user option.
(eshell--buffered-print-next-redisplay): New variable.
(eshell-init-print-buffer): Make obsolete.
(eshell-flush): Add new REDISPLAY-NOW argument in favor of CLEAR (which
only 'eshell-init-print-buffer' should have used).
(eshell-buffered-print): Compare queued output length to
'eshell--buffered-print-current-size'.
(eshell-with-buffered-print): New macro.

* lisp/eshell/esh-var.el (eshell/env):
* lisp/eshell/em-dirs.el (eshell/cd):
* lisp/eshell/em-hist.el (eshell/history):
* lisp/eshell/em-unix.el (eshell/cat):
* lisp/eshell/em-ls.el (eshell/ls): Use 'eshell-with-buffered-print'.
(flush-func): Remove.
(eshell-ls--insert-directory, eshell-do-ls): Remove 'flush-func'.

* test/lisp/eshell/em-unix-tests.el (em-unix-test/compile/interactive)
(em-unix-test/compile/pipeline, em-unix-test/compile/subcommand): Fix
indentation.
(em-unix-test/cat/file-output): New test.

* etc/NEWS: Announce these improvements.

etc/NEWS

@@ -963,6 +963,13 @@ files and deny read permission for users who are not members of the
 file's group.  See the Info node "(coreutils) File permissions" for
 more information on this notation.
 
+---
+*** Performance improvements for interactive output in Eshell.
+Interactive output in Eshell should now be significantly faster,
+especially for built-in commands that can print large amounts of output
+(e.g. "cat").  In addition, these commands can now update the display
+periodically to show their progress.
+
 +++
 *** New special reference type '#<marker POSITION BUFFER>'.
 This special reference type returns a marker at 'POSITION' in

lisp/eshell/em-dirs.el

@@ -400,13 +400,12 @@ in the minibuffer:
 		(index 0))
 	    (if (= len 0)
 		(error "Directory ring empty"))
-	    (eshell-init-print-buffer)
-	    (while (< index len)
-	      (eshell-buffered-print
-	       (concat (number-to-string index) ": "
-		       (ring-ref eshell-last-dir-ring index) "\n"))
-	      (setq index (1+ index)))
-	    (eshell-flush)
+            (eshell-with-buffered-print
+              (while (< index len)
+                (eshell-buffered-print
+                 (concat (number-to-string index) ": "
+                         (ring-ref eshell-last-dir-ring index) "\n"))
+                (setq index (1+ index))))
 	    (setq handled t)))))
      (path
       (setq path (eshell-expand-multiple-dots path))))

lisp/eshell/em-hist.el

@@ -333,7 +333,6 @@ Returns nil if INPUT is prepended by blank space, otherwise non-nil."
 
 (defun eshell/history (&rest args)
   "List in help buffer the buffer's input history."
-  (eshell-init-print-buffer)
   (eshell-eval-using-options
    "history" args
    '((?r "read" nil read-history
@@ -370,12 +369,12 @@ unless a different file is specified on the command line.")
        (let* ((index (1- (or length (ring-length eshell-history-ring))))
 	      (ref (- (ring-length eshell-history-ring) index)))
 	 ;; We have to build up a list ourselves from the ring vector.
-	 (while (>= index 0)
-	   (eshell-buffered-print
-	    (format "%5d  %s\n" ref (eshell-get-history index)))
-	   (setq index (1- index)
-		 ref (1+ ref)))))))
-   (eshell-flush)
+         (eshell-with-buffered-print
+           (while (>= index 0)
+             (eshell-buffered-print
+              (format "%5d  %s\n" ref (eshell-get-history index)))
+             (setq index (1- index)
+                   ref (1+ ref))))))))
    nil))
 
 (defun eshell-put-history (input &optional ring at-beginning)

lisp/eshell/em-ls.el

@@ -229,7 +229,6 @@ scope during the evaluation of TEST-SEXP."
 (defvar dereference-links)
 (defvar dir-literal)
 (defvar error-func)
-(defvar flush-func)
 (defvar human-readable)
 (defvar ignore-pattern)
 (defvar insert-func)
@@ -278,7 +277,6 @@ instead."
           (require 'em-glob)
           (let* ((insert-func 'insert)
                  (error-func 'insert)
-                 (flush-func 'ignore)
                  (eshell-error-if-no-glob t)
                  (target ; Expand the shell wildcards if any.
                   (if (and (atom file)
@@ -324,10 +322,10 @@ instead."
 
 (defsubst eshell/ls (&rest args)
   "An alias version of `eshell-do-ls'."
-  (let ((insert-func 'eshell-buffered-print)
-	(error-func 'eshell-error)
-	(flush-func 'eshell-flush))
-    (apply 'eshell-do-ls args)))
+  (eshell-with-buffered-print
+    (let ((insert-func #'eshell-buffered-print)
+          (error-func #'eshell-error))
+      (apply 'eshell-do-ls args))))
 
 (put 'eshell/ls 'eshell-no-numeric-conversions t)
 (put 'eshell/ls 'eshell-filename-arguments t)
@@ -336,7 +334,6 @@ instead."
 
 (defun eshell-do-ls (&rest args)
   "Implementation of \"ls\" in Lisp, passing ARGS."
-  (funcall flush-func -1)
   ;; Process the command arguments, and begin listing files.
   (eshell-eval-using-options
    "ls" (if eshell-ls-initial-args
@@ -422,8 +419,7 @@ Sort entries alphabetically across.")
 		      (eshell-file-attributes
 		       arg (if numeric-uid-gid 'integer 'string))))
 	      args)
-      t (expand-file-name default-directory)))
-   (funcall flush-func)))
+      t (expand-file-name default-directory)))))
 
 (defsubst eshell-ls-printable-size (filesize &optional by-blocksize)
   "Return a printable FILESIZE."

lisp/eshell/em-unix.el

@@ -659,7 +659,6 @@ symlink, then revert to the system's definition of cat."
 	  (if eshell-in-pipeline-p
 	      (error "Eshell's `cat' does not work in pipelines")
 	    (error "Eshell's `cat' cannot display one of the files given"))))
-    (eshell-init-print-buffer)
     (eshell-eval-using-options
      "cat" args
      '((?h "help" nil nil "show this usage screen")
@@ -672,18 +671,18 @@ Concatenate FILE(s), or standard input, to standard output.")
 	   (throw 'eshell-external
 		  (eshell-external-command "cat" args))))
      (let ((curbuf (current-buffer)))
-       (dolist (file args)
-	 (with-temp-buffer
-	   (insert-file-contents file)
-	   (goto-char (point-min))
-	   (while (not (eobp))
-	     (let ((str (buffer-substring
-			 (point) (min (1+ (line-end-position))
-				      (point-max)))))
-	       (with-current-buffer curbuf
-		 (eshell-buffered-print str)))
-	     (forward-line)))))
-     (eshell-flush))))
+       (eshell-with-buffered-print
+         (dolist (file args)
+	   (with-temp-buffer
+	     (insert-file-contents file)
+	     (goto-char (point-min))
+             (while (not (eobp))
+               (let* ((pos (min (+ (point) eshell-buffered-print-size)
+                                (point-max)))
+                      (str (buffer-substring (point) pos)))
+                 (with-current-buffer curbuf
+                   (eshell-buffered-print str))
+                 (goto-char pos))))))))))
 
 (put 'eshell/cat 'eshell-no-numeric-conversions t)
 (put 'eshell/cat 'eshell-filename-arguments t)

lisp/eshell/esh-io.el

@@ -112,10 +112,30 @@ other buffers)."
 
 (defcustom eshell-print-queue-size 5
   "The size of the print queue, for doing buffered printing.
-This is basically a speed enhancement, to avoid blocking the Lisp code
-from executing while Emacs is redisplaying."
+This variable is obsolete.  You should use `eshell-buffered-print-size'
+instead."
   :type 'integer
   :group 'eshell-io)
+(make-obsolete-variable 'eshell-print-queue-size
+                        'eshell-buffered-print-size "30.1")
+
+(defcustom eshell-buffered-print-size 2048
+  "The size of the print queue in characters, for doing buffered printing.
+Larger values for this option will generally result in faster execution
+by reducing the overhead associated with each print operation, but will
+increase the time it takes to see any progress in the output; smaller
+values will do the reverse."
+  :type 'integer
+  :group 'eshell-io
+  :version "30.1")
+
+(defcustom eshell-buffered-print-redisplay-throttle 0.025
+  "The minimum time in seconds between redisplays when using buffered printing.
+If nil, don't redisplay while printing."
+  :type '(choice number
+                 (const :tag "Don't redisplay" nil))
+  :group 'eshell-io
+  :version "30.1")
 
 (defcustom eshell-virtual-targets
   '(;; The literal string "/dev/null" is intentional here.  It just
@@ -460,40 +480,74 @@ INDEX is the handle index to check.  If nil, check
              (equal (caar (aref handles eshell-error-handle)) '(t)))
       (equal (caar (aref handles index)) '(t)))))
 
+(defvar eshell--buffered-print-queue nil)
+(defvar eshell--buffered-print-current-size nil)
+(defvar eshell--buffered-print-next-redisplay nil)
+
 (defvar eshell-print-queue nil)
+(make-obsolete-variable 'eshell-print-queue
+                        'eshell--buffered-print-queue "30.1")
 (defvar eshell-print-queue-count -1)
+(make-obsolete-variable 'eshell-print-queue-count
+                        'eshell--buffered-print-current-size "30.1")
 
 (defsubst eshell-print (object)
   "Output OBJECT to the standard output handle."
   (eshell-output-object object eshell-output-handle))
 
-(defun eshell-flush (&optional reset-p)
-  "Flush out any lines that have been queued for printing.
-Must be called before printing begins with -1 as its argument, and
-after all printing is over with no argument."
-  (ignore
-   (if reset-p
-       (setq eshell-print-queue nil
-	     eshell-print-queue-count reset-p)
-     (if eshell-print-queue
-	 (eshell-print eshell-print-queue))
-     (eshell-flush 0))))
-
 (defun eshell-init-print-buffer ()
   "Initialize the buffered printing queue."
-  (eshell-flush -1))
+  (declare (obsolete #'eshell-with-buffered-print "30.1"))
+  (setq eshell--buffered-print-queue nil
+        eshell--buffered-print-current-size 0))
+
+(defun eshell-flush (&optional redisplay-now)
+  "Flush out any text that has been queued for printing.
+When printing interactively, this will call `redisplay' every
+`eshell-buffered-print-redisplay-throttle' seconds so that the user can
+see the progress.  If REDISPLAY-NOW is non-nil, call `redisplay' for
+interactive output even if the throttle would otherwise prevent it."
+  (ignore
+   (when eshell--buffered-print-queue
+     (eshell-print (apply #'concat eshell--buffered-print-queue))
+     ;; When printing interactively (see `eshell-with-buffered-print'),
+     ;; periodically redisplay so the user can see some progress.
+     (when (and eshell--buffered-print-next-redisplay
+                (or redisplay-now
+                    (time-less-p eshell--buffered-print-next-redisplay
+                                 (current-time))))
+       (redisplay)
+       (setq eshell--buffered-print-next-redisplay
+             (time-add eshell--buffered-print-next-redisplay
+                       eshell-buffered-print-redisplay-throttle)))
+     (setq eshell--buffered-print-queue nil
+           eshell--buffered-print-current-size 0))))
 
 (defun eshell-buffered-print (&rest strings)
-  "A buffered print -- *for strings only*."
-  (if (< eshell-print-queue-count 0)
-      (progn
-	(eshell-print (apply 'concat strings))
-	(setq eshell-print-queue-count 0))
-    (if (= eshell-print-queue-count eshell-print-queue-size)
-	(eshell-flush))
-    (setq eshell-print-queue
-	  (concat eshell-print-queue (apply 'concat strings))
-	  eshell-print-queue-count (1+ eshell-print-queue-count))))
+  "A buffered print -- *for strings only*.
+When the buffer exceeds `eshell-buffered-print-size' in characters, this
+will flush it using `eshell-flush' (which see)."
+  (setq eshell--buffered-print-queue
+        (nconc eshell--buffered-print-queue strings))
+  (cl-incf eshell--buffered-print-current-size
+           (apply #'+ (mapcar #'length strings)))
+  (when (> eshell--buffered-print-current-size eshell-buffered-print-size)
+    (eshell-flush)))
+
+(defmacro eshell-with-buffered-print (&rest body)
+  "Initialize buffered printing for Eshell, and then evaluate BODY.
+Within BODY, call `eshell-buffered-print' to perform output."
+  (declare (indent 0))
+  `(let ((eshell--buffered-print-queue nil)
+         (eshell--buffered-print-current-size 0)
+         (eshell--buffered-print-next-redisplay
+          (when (and eshell-buffered-print-redisplay-throttle
+                     (eshell-interactive-output-p))
+            (time-add (current-time)
+                      eshell-buffered-print-redisplay-throttle))))
+     (unwind-protect
+         ,@body
+       (eshell-flush))))
 
 (defsubst eshell-error (object)
   "Output OBJECT to the standard error handle."

lisp/eshell/esh-var.el

@@ -437,10 +437,9 @@ the values of nil for each."
    (if args
        (or (eshell-parse-local-variables args)
            (eshell-named-command (car args) (cdr args)))
-     (eshell-init-print-buffer)
-     (dolist (setting (sort (eshell-environment-variables) 'string-lessp))
-       (eshell-buffered-print setting "\n"))
-     (eshell-flush))))
+     (eshell-with-buffered-print
+       (dolist (setting (sort (eshell-environment-variables) 'string-lessp))
+         (eshell-buffered-print setting "\n"))))))
 
 (defun eshell-insert-envvar (envvar-name)
   "Insert ENVVAR-NAME into the current buffer at point."

test/lisp/eshell/em-unix-tests.el

@@ -26,10 +26,12 @@
 (require 'ert)
 (require 'em-unix)
 
+(eval-and-compile
+  (defvar this-directory (file-name-directory
+                          (or load-file-name default-directory))))
+
 (require 'eshell-tests-helpers
-         (expand-file-name "eshell-tests-helpers"
-                           (file-name-directory (or load-file-name
-                                                    default-directory))))
+         (expand-file-name "eshell-tests-helpers" this-directory))
 
 ;;; Tests:
 
@@ -37,11 +39,11 @@
   "Check that `eshell/compile' opens a compilation buffer interactively."
   (skip-unless (executable-find "echo"))
   (with-temp-eshell
-   (eshell-match-command-output "compile echo hello"
-                                "#<buffer \\*compilation\\*>")
-   (with-current-buffer "*compilation*"
-     (forward-line 3)
-     (should (looking-at "echo hello")))))
+    (eshell-match-command-output "compile echo hello"
+                                 "#<buffer \\*compilation\\*>")
+    (with-current-buffer "*compilation*"
+      (forward-line 3)
+      (should (looking-at "echo hello")))))
 
 (ert-deftest em-unix-test/compile/noninteractive ()
   "Check that `eshell/compile' writes to stdout noninteractively."
@@ -54,15 +56,26 @@
   (skip-unless (and (executable-find "echo")
                     (executable-find "cat")))
   (with-temp-eshell
-   (eshell-match-command-output "compile echo hello | *cat"
-                                "\\`hello\n")))
+    (eshell-match-command-output "compile echo hello | *cat"
+                                 "\\`hello\n")))
 
 (ert-deftest em-unix-test/compile/subcommand ()
   "Check that `eshell/compile' writes to stdout from a subcommand."
   (skip-unless (and (executable-find "echo")
                     (executable-find "cat")))
   (with-temp-eshell
-   (eshell-match-command-output "echo ${compile echo hello}"
-                                "\\`hello\n")))
+    (eshell-match-command-output "echo ${compile echo hello}"
+                                 "\\`hello\n")))
+
+(ert-deftest em-unix-test/cat/file-output ()
+  "Check that `eshell/cat' can print a file's contents."
+  (with-temp-eshell
+    (let* ((this-file (expand-file-name "em-unix-tests.el" this-directory))
+           (contents (save-current-buffer
+                       (find-file this-file)
+                       (buffer-string))))
+      (eshell-match-command-output
+       (format "cat '%s'" (string-replace "'" "''" this-file))
+       (concat (regexp-quote contents))))))
 
 ;; em-unix-tests.el ends here