[Emacs-diffs] master e9f9827: Avoid polling in global-auto-revert-mode (bug#35418)

[Mattias Engdegård]

branch: master
commit e9f9827eb01a382bead6c180f6703322167e6f89
Author: Mattias Engdegård <address@hidden>
Commit: Mattias Engdegård <address@hidden>

    Avoid polling in global-auto-revert-mode (bug#35418)
    
    Make `auto-revert-avoid-polling' have effect in global-auto-revert-mode.
    Buffers actually handled by that mode are marked with a non-nil value
    of `auto-revert--global-mode'.  When global-auto-revert-mode is
    entered, eligible buffers are marked in that way, and hooks are set up
    to mark new buffers and take care of buffers whose file names change.
    This way the existing poll-avoidance logic can be used, since the
    entire set of buffers in auto-revert is known.
    
    A new hook, `after-set-visited-file-name-hook', was added to handle
    the case when the file name of a tracked buffer changes.
    
    * lisp/autorevert.el (auto-revert-avoid-polling): Amend doc string.
    (auto-revert--global-mode): New buffer-local variable.
    (global-auto-revert-mode): Mark existing buffers and set up hooks when
    mode is entered; do the opposite when exited.
    (auto-revert--global-add-current-buffer)
    (auto-revert--global-adopt-current-buffer)
    (auto-revert--set-visited-file-name-advice): New functions.
    (auto-revert--polled-buffers, auto-revert--need-polling-p)
    (auto-revert-notify-handler)
    (auto-revert-active-p): Modify logic to cover global-auto-revert-mode.
    * lisp/files.el (after-set-visited-file-name-hook): New hook.
    (set-visited-file-name-hook): Call new hook.
    * test/lisp/autorevert-tests.el (top):  Use lexical-binding.
    (auto-revert-test--write-file, auto-revert-test--buffer-string)
    (auto-revert-test--wait-for, auto-revert-test--wait-for-buffer-text)
    (auto-revert-test05-global-notify): New test.
    * doc/lispref/hooks.texi (Standard Hooks):
    Mention new hook (in a comment, since it's unclear whether it should
    actually be documented here)
    * etc/NEWS (Changes in Specialized Modes and Packages): Update entry.
---
 doc/lispref/hooks.texi        |   1 +
 etc/NEWS                      |   3 +-
 lisp/autorevert.el            | 124 +++++++++++++++++++++++++++++++++---------
 lisp/files.el                 |   6 +-
 test/lisp/autorevert-tests.el | 107 +++++++++++++++++++++++++++++++++++-
 5 files changed, 210 insertions(+), 31 deletions(-)

diff --git a/doc/lispref/hooks.texi b/doc/lispref/hooks.texi
index 7199246..f775aa4 100644
--- a/doc/lispref/hooks.texi
+++ b/doc/lispref/hooks.texi
@@ -251,6 +251,7 @@ I thought did not need to be mentioned here:
 
 Lisp:
 after-load-functions
+after-set-visited-file-name-hook
 auto-coding-functions
 choose-completion-string-functions
 completing-read-function
diff --git a/etc/NEWS b/etc/NEWS
index d70cda1..9ca98c3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1443,8 +1443,7 @@ When set to a non-nil value, buffers in Auto Revert mode 
are no longer
 polled for changes periodically.  This reduces the power consumption
 of an idle Emacs, but may fail on some network file systems; set
 'auto-revert-notify-exclude-dir-regexp' to match files where
-notification is not supported.  The new variable currently has no
-effect in 'global-auto-revert-mode'.  The default value is nil.
+notification is not supported.  The default value is nil.
 
 *** New variable 'buffer-auto-revert-by-notification'
 A major mode can declare that notification on the buffer's default
diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index 197a2bf..2de855b 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -312,10 +312,7 @@ when those files are modified from another computer.
 
 When nil, buffers in Auto-Revert Mode will always be polled for
 changes to their files on disk every `auto-revert-interval'
-seconds, in addition to using notification for those files.
-
-In Global Auto-Revert Mode, polling is always done regardless of
-the value of this variable."
+seconds, in addition to using notification for those files."
   :group 'auto-revert
   :type 'boolean
   :set (lambda (variable value)
@@ -335,6 +332,9 @@ buffers to this list.
 The timer function `auto-revert-buffers' is responsible for purging
 the list of old buffers.")
 
+(defvar-local auto-revert--global-mode nil
+  "Non-nil if buffer is handled by Global Auto-Revert mode.")
+
 (defvar auto-revert-remaining-buffers ()
   "Buffers not checked when user input stopped execution.")
 
@@ -501,34 +501,107 @@ specifies in the mode line."
   :global t :group 'auto-revert :lighter global-auto-revert-mode-text
   (auto-revert-set-timer)
   (if global-auto-revert-mode
-      (auto-revert-buffers)
+      ;; Turn global-auto-revert-mode ON.
+      (progn
+        (dolist (buf (buffer-list))
+          (with-current-buffer buf
+            (auto-revert--global-add-current-buffer)))
+        ;; Make sure future buffers are added as well.
+        (add-hook 'find-file-hook #'auto-revert--global-adopt-current-buffer)
+        (add-hook 'after-set-visited-file-name-hook
+                  #'auto-revert--global-set-visited-file-name)
+        ;; To track non-file buffers, we need to listen in to buffer
+        ;; creation in general.  Listening to major-mode changes is
+        ;; suitable, since we then know whether it's a mode that is tracked.
+        (when global-auto-revert-non-file-buffers
+          (add-hook 'after-change-major-mode-hook
+                    #'auto-revert--global-adopt-current-buffer))
+        (auto-revert-buffers))
+    ;; Turn global-auto-revert-mode OFF.
+    (remove-hook 'after-change-major-mode-hook
+                 #'auto-revert--global-adopt-current-buffer)
+    (remove-hook 'after-set-visited-file-name-hook
+                 #'auto-revert--global-set-visited-file-name)
+    (remove-hook 'find-file-hook #'auto-revert--global-adopt-current-buffer)
     (dolist (buf (buffer-list))
       (with-current-buffer buf
-        (when (and auto-revert-notify-watch-descriptor
-                   (not (memq buf auto-revert-buffer-list)))
-         (auto-revert-notify-rm-watch))))))
+        (when auto-revert--global-mode
+          (setq auto-revert--global-mode nil)
+          (when (and auto-revert-notify-watch-descriptor
+                     (not (or auto-revert-mode auto-revert-tail-mode)))
+           (auto-revert-notify-rm-watch)))))))
+
+(defun auto-revert--global-add-current-buffer ()
+  "Set current buffer to be tracked by Global Auto-Revert if appropriate."
+  (when (and (not auto-revert--global-mode)
+             (or buffer-file-name
+                 (and global-auto-revert-non-file-buffers
+                      (not (string-prefix-p " " (buffer-name)))
+                      ;; Any non-file buffer must have a custom
+                      ;; `buffer-stale-function' to be tracked, since
+                      ;; we wouldn't know when to revert it otherwise.
+                      (not (eq buffer-stale-function
+                               #'buffer-stale--default-function))))
+             (not (memq 'major-mode global-auto-revert-ignore-modes))
+             (not global-auto-revert-ignore-buffer))
+    (setq auto-revert--global-mode t)))
+
+(defun auto-revert--global-adopt-current-buffer ()
+  "Consider tracking current buffer in a running Global Auto-Revert mode."
+  (auto-revert--global-add-current-buffer)
+  (auto-revert-set-timer))
+
+(defun auto-revert--global-set-visited-file-name ()
+  "Update Global Auto-Revert management of the current buffer.
+Called after `set-visited-file-name'."
+  ;; Remove any existing notifier first so that we don't track the
+  ;; wrong file in case the file name was changed.
+  (when auto-revert-notify-watch-descriptor
+    (auto-revert-notify-rm-watch))
+  (auto-revert--global-adopt-current-buffer))
 
 (defun auto-revert--polled-buffers ()
   "List of buffers that need to be polled."
-  (cond (global-auto-revert-mode (buffer-list))
+  (cond (global-auto-revert-mode
+         (mapcan (lambda (buffer)
+                   (and (not (and auto-revert-avoid-polling
+                                  (buffer-local-value
+                                   'auto-revert-notify-watch-descriptor
+                                   buffer)))
+                        (or (buffer-local-value
+                             'auto-revert--global-mode buffer)
+                            (buffer-local-value 'auto-revert-mode buffer)
+                            (buffer-local-value 'auto-revert-tail-mode buffer))
+                        (list buffer)))
+                 (buffer-list)))
         (auto-revert-avoid-polling
          (mapcan (lambda (buffer)
-                     (and (not (buffer-local-value
-                                'auto-revert-notify-watch-descriptor buffer))
-                          (list buffer)))
-                   auto-revert-buffer-list))
+                   (and (not (buffer-local-value
+                              'auto-revert-notify-watch-descriptor buffer))
+                        (list buffer)))
+                 auto-revert-buffer-list))
         (t auto-revert-buffer-list)))
 
 ;; Same as above in a boolean context, but cheaper.
 (defun auto-revert--need-polling-p ()
   "Whether periodic polling is required."
-  (or global-auto-revert-mode
-      (if auto-revert-avoid-polling
-          (not (cl-every (lambda (buffer)
-                           (buffer-local-value
-                            'auto-revert-notify-watch-descriptor buffer))
-                         auto-revert-buffer-list))
-        auto-revert-buffer-list)))
+  (cond (global-auto-revert-mode
+         (or (not auto-revert-avoid-polling)
+             (cl-some
+              (lambda (buffer)
+                (and (not (buffer-local-value
+                           'auto-revert-notify-watch-descriptor buffer))
+                     (or (buffer-local-value 'auto-revert--global-mode buffer)
+                         (buffer-local-value 'auto-revert-mode buffer)
+                         (buffer-local-value 'auto-revert-tail-mode buffer))))
+              (buffer-list))))
+        (auto-revert-avoid-polling
+         (not (cl-every
+               (lambda (buffer)
+                 (buffer-local-value
+                  'auto-revert-notify-watch-descriptor buffer))
+               auto-revert-buffer-list)))
+        (t auto-revert-buffer-list)))
 
 (defun auto-revert-set-timer ()
   "Restart or cancel the timer used by Auto-Revert Mode.
@@ -652,9 +725,8 @@ system.")
                      (null buffer-file-name))
                 (auto-revert-notify-rm-watch)
                 ;; Restart the timer if it wasn't running.
-                (when (and (memq buffer auto-revert-buffer-list)
-                           (not auto-revert-timer))
-                  (auto-revert-set-timer)))))
+                (unless auto-revert-timer)
+                  (auto-revert-set-timer))))
 
         ;; Loop over all buffers, in order to find the intended one.
         (cl-dolist (buffer buffers)
@@ -697,12 +769,10 @@ If the buffer needs to be reverted, do it now."
         (auto-revert-handler)))))
 
 (defun auto-revert-active-p ()
-  "Check if auto-revert is active (in current buffer or globally)."
+  "Check if auto-revert is active in current buffer."
   (or auto-revert-mode
       auto-revert-tail-mode
-      (and global-auto-revert-mode
-           (not global-auto-revert-ignore-buffer)
-           (not (memq major-mode global-auto-revert-ignore-modes)))))
+      auto-revert--global-mode))
 
 (defun auto-revert-handler ()
   "Revert current buffer, if appropriate.
diff --git a/lisp/files.el b/lisp/files.el
index 21fa614..989d1cb 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4268,6 +4268,9 @@ However, the mode will not be changed if
   :type 'boolean
   :group 'editing-basics)
 
+(defvar after-set-visited-file-name-hook nil
+  "Normal hook run just after setting visited file name of current buffer.")
+
 (defun set-visited-file-name (filename &optional no-query along-with-file)
   "Change name of file visited in current buffer to FILENAME.
 This also renames the buffer to correspond to the new file.
@@ -4388,7 +4391,8 @@ the old visited file has been renamed to the new name 
FILENAME."
              (set-auto-mode t)
              (or (eq old major-mode)
                  (hack-local-variables))))
-    (error nil))))
+      (error nil))
+    (run-hooks 'after-set-visited-file-name-hook)))
 
 (defun write-file (filename &optional confirm)
   "Write current buffer into file FILENAME.
diff --git a/test/lisp/autorevert-tests.el b/test/lisp/autorevert-tests.el
index 040dbb4..5024a2d 100644
--- a/test/lisp/autorevert-tests.el
+++ b/test/lisp/autorevert-tests.el
@@ -1,4 +1,4 @@
-;;; auto-revert-tests.el --- Tests of auto-revert
+;;; auto-revert-tests.el --- Tests of auto-revert   -*- lexical-binding: t -*-
 
 ;; Copyright (C) 2015-2019 Free Software Foundation, Inc.
 
@@ -436,6 +436,111 @@ This expects `auto-revert--messages' to be bound by
 (auto-revert--deftest-remote auto-revert-test04-auto-revert-mode-dired
   "Check remote autorevert for dired.")
 
+(defun auto-revert-test--write-file (string file)
+  "Write STRING to FILE."
+  (write-region string nil file nil 'no-message))
+
+(defun auto-revert-test--buffer-string (buffer)
+  "Contents of BUFFER as a string."
+  (with-current-buffer buffer
+    (buffer-string)))
+
+(defun auto-revert-test--wait-for (pred max-wait)
+  "Wait until PRED is true, or MAX-WAIT seconds elapsed."
+  (let ((ct (current-time)))
+    (while (and (< (float-time (time-subtract (current-time) ct)) max-wait)
+                (not (funcall pred)))
+      (read-event nil nil 0.1))))
+
+(defun auto-revert-test--wait-for-buffer-text (buffer string max-wait)
+  "Wait until BUFFER has the contents STRING, or MAX-WAIT seconds elapsed."
+  (auto-revert-test--wait-for
+   (lambda () (string-equal (auto-revert-test--buffer-string buffer) string))
+   max-wait))
+
+(ert-deftest auto-revert-test05-global-notify ()
+  "Test `global-auto-revert-mode' without polling."
+  :tags '(:expensive-test)
+  (skip-unless (or file-notify--library
+                   (file-remote-p temporary-file-directory)))
+  (let* ((auto-revert-use-notify t)
+         (auto-revert-avoid-polling t)
+         (was-in-global-auto-revert-mode global-auto-revert-mode)
+         (file-1 (make-temp-file "global-auto-revert-test-1"))
+         (file-2 (make-temp-file "global-auto-revert-test-2"))
+         (file-3 (make-temp-file "global-auto-revert-test-3"))
+         (file-2b (concat file-2 "-b"))
+         buf-1 buf-2 buf-3)
+    (unwind-protect
+        (progn
+          (setq buf-1 (find-file-noselect file-1))
+          (setq buf-2 (find-file-noselect file-2))
+          (auto-revert-test--write-file "1-a" file-1)
+          (should (equal (auto-revert-test--buffer-string buf-1) ""))
+
+          (global-auto-revert-mode 1)   ; Turn it on.
+
+          (should (buffer-local-value
+                   'auto-revert-notify-watch-descriptor buf-1))
+          (should (buffer-local-value
+                   'auto-revert-notify-watch-descriptor buf-2))
+
+          ;; buf-1 should have been reverted immediately when the mode
+          ;; was enabled.
+          (should (equal (auto-revert-test--buffer-string buf-1) "1-a"))
+
+          ;; Alter a file.
+          (auto-revert-test--write-file "2-a" file-2)
+          ;; Allow for some time to handle notification events.
+          (auto-revert-test--wait-for-buffer-text buf-2 "2-a" 1)
+          (should (equal (auto-revert-test--buffer-string buf-2) "2-a"))
+
+          ;; Visit a file, and modify it on disk.
+          (setq buf-3 (find-file-noselect file-3))
+          ;; Newly opened buffers won't be use notification until the
+          ;; first poll cycle; wait for it.
+          (auto-revert-test--wait-for
+           (lambda () (buffer-local-value
+                       'auto-revert-notify-watch-descriptor buf-3))
+           (+ auto-revert-interval 1))
+          (should (buffer-local-value
+                   'auto-revert-notify-watch-descriptor buf-3))
+          (auto-revert-test--write-file "3-a" file-3)
+          (auto-revert-test--wait-for-buffer-text buf-3 "3-a" 1)
+          (should (equal (auto-revert-test--buffer-string buf-3) "3-a"))
+
+          ;; Delete a visited file, and re-create it with new contents.
+          (delete-file file-1)
+          (sleep-for 0.5)
+          (should (equal (auto-revert-test--buffer-string buf-1) "1-a"))
+          (auto-revert-test--write-file "1-b" file-1)
+          (auto-revert-test--wait-for-buffer-text buf-1 "1-b"
+           (+ auto-revert-interval 1))
+          (should (buffer-local-value
+                   'auto-revert-notify-watch-descriptor buf-1))
+
+          ;; Write a buffer to a new file, then modify the new file on disk.
+          (with-current-buffer buf-2
+            (write-file file-2b))
+          (should (equal (auto-revert-test--buffer-string buf-2) "2-a"))
+          (auto-revert-test--write-file "2-b" file-2b)
+          (auto-revert-test--wait-for-buffer-text buf-2 "2-b"
+           (+ auto-revert-interval 1))
+          (should (buffer-local-value
+                   'auto-revert-notify-watch-descriptor buf-2)))
+
+      ;; Clean up.
+      (unless was-in-global-auto-revert-mode
+        (global-auto-revert-mode 0))    ; Turn it off.
+      (dolist (buf (list buf-1 buf-2 buf-3))
+        (ignore-errors (kill-buffer buf)))
+      (dolist (file (list file-1 file-2 file-2b file-3))
+        (ignore-errors (delete-file file)))
+      )))
+
+(auto-revert--deftest-remote auto-revert-test04-auto-revert-mode-dired
+  "Test `global-auto-revert-mode' without polling for remote buffers.")
+
 (defun auto-revert-test-all (&optional interactive)
   "Run all tests for \\[auto-revert]."
   (interactive "p")