From c732c4857dd04d64c7874d7fab6faa0bc5387d1a Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Mon, 3 Jun 2024 22:01:48 -0700 Subject: [PATCH] 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. (cherry picked from commit 2fac71255f2e216481f956ad318378cdfddb9402) --- etc/NEWS | 7 ++ lisp/eshell/em-dirs.el | 13 ++-- lisp/eshell/em-hist.el | 13 ++-- lisp/eshell/em-ls.el | 14 ++-- lisp/eshell/em-unix.el | 25 ++++--- lisp/eshell/esh-io.el | 104 +++++++++++++++++++++++------- lisp/eshell/esh-var.el | 7 +- test/lisp/eshell/em-unix-tests.el | 37 +++++++---- 8 files changed, 143 insertions(+), 77 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index e464168908f..d68fa49397c 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -940,6 +940,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 '#'. This special reference type returns a marker at 'POSITION' in diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el index a3d1a349540..e70f2cfe196 100644 --- a/lisp/eshell/em-dirs.el +++ b/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)))) diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el index 8865cc745a3..9ffddfb611f 100644 --- a/lisp/eshell/em-hist.el +++ b/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) diff --git a/lisp/eshell/em-ls.el b/lisp/eshell/em-ls.el index 82d4b01393f..8bf2e20d320 100644 --- a/lisp/eshell/em-ls.el +++ b/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." diff --git a/lisp/eshell/em-unix.el b/lisp/eshell/em-unix.el index 4137c05fa41..e6bd0381a14 100644 --- a/lisp/eshell/em-unix.el +++ b/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) diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el index 0fe177d4c60..9de9cc4509a 100644 --- a/lisp/eshell/esh-io.el +++ b/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." diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el index 02b5c785625..f0270aca92c 100644 --- a/lisp/eshell/esh-var.el +++ b/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." diff --git a/test/lisp/eshell/em-unix-tests.el b/test/lisp/eshell/em-unix-tests.el index a92c7d3f80a..2ee42c81333 100644 --- a/test/lisp/eshell/em-unix-tests.el +++ b/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" - "#") - (with-current-buffer "*compilation*" - (forward-line 3) - (should (looking-at "echo hello"))))) + (eshell-match-command-output "compile echo hello" + "#") + (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 -- 2.39.2