bug#35418: [PATCH] Don't poll auto-revert files that use notification

["Mattias Engdegård" <mattiase@acm.org>]

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

5 maj 2019 kl. 11.58 skrev Mattias Engdegård <mattiase@acm.org>:
> 
> What remains is avoiding polling in global-auto-revert-mode. I'll send a patch soon.

Here is that patch.

I understand that some people are queasy about using advice in code like this, and am open to suggestions about alternatives.
What the code needs is a reasonable (not necessarily bullet-proof) way to detect new file buffers and changes to buffer-file-name of those buffers. Monitoring `find-file-noselect' and `set-visited-file-name' turned out to be good enough.
For the former, it might be possible to get away with `after-change-major-mode-hook' instead (already used for non-file buffers).


[-- Attachment #2: 0001-Avoid-polling-in-global-auto-revert-mode.patch --]
[-- Type: application/octet-stream, Size: 10847 bytes --]

From 34302c20db88917a9a5bea90a70d315b44f1647d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Thu, 9 May 2019 09:40:46 +0200
Subject: [PATCH] Avoid polling in global-auto-revert-mode

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 `global-auto-revert--tracked-buffer'.  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.
(Bug#35418).

* lisp/autorevert.el (auto-revert-avoid-polling): Amend doc string.
(global-auto-revert--tracked-buffer): 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-buffer)
(auto-revert--find-file-noselect-advice)
(auto-revert--set-visited-file-name-advice)
(auto-revert--after-change-major-mode): 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.
* etc/NEWS (Changes in Specialized Modes and Packages): Update entry.
---
 etc/NEWS           |   3 +-
 lisp/autorevert.el | 133 ++++++++++++++++++++++++++++++++++++---------
 2 files changed, 108 insertions(+), 28 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 9e3559d27e..56c7163f7f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1396,8 +1396,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.
 
 \f
 * New Modes and Packages in Emacs 27.1
diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index fbaffbf0d6..402301c448 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 global-auto-revert--tracked-buffer 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,118 @@ 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
+        (mapc #'auto-revert--global-add-buffer (buffer-list))
+        ;; Make sure future buffers are added as well.
+        (advice-add 'find-file-noselect :filter-return
+                    #'auto-revert--find-file-noselect-advice)
+        (advice-add 'set-visited-file-name :after
+                    #'auto-revert--set-visited-file-name-advice)
+        ;; 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--after-change-major-mode))
+        (auto-revert-buffers))
+    ;; Turn global-auto-revert-mode OFF.
+    (remove-hook 'after-change-major-mode-hook
+                 #'auto-revert--after-change-major-mode)
+    (advice-remove 'set-visited-file-name
+                   #'auto-revert--set-visited-file-name-advice)
+    (advice-remove 'find-file-noselect
+                   #'auto-revert--find-file-noselect-advice)
     (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 global-auto-revert--tracked-buffer
+          (setq global-auto-revert--tracked-buffer 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-buffer (buffer)
+  "Set BUFFER to be tracked by Global Auto-Revert if appropriate."
+  (with-current-buffer buffer
+    (when (and (not global-auto-revert--tracked-buffer)
+               (or buffer-file-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.
+                   (and global-auto-revert-non-file-buffers
+                        (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 global-auto-revert--tracked-buffer t))))
+
+(defun auto-revert--find-file-noselect-advice (buffer)
+  "Adopt BUFFER for Global Auto-Revert if appropriate.
+Called with the return value of `find-file-noselect'."
+  (auto-revert--global-add-buffer buffer)
+  (auto-revert-set-timer)
+  buffer)
+
+(defun auto-revert--set-visited-file-name-advice (&rest _)
+  "Adopt the current buffer for Global Auto-Revert if appropriate.
+Called after `set-visited-file-name'."
+  ;; In case the file name was changed, remove any existing notifier
+  ;; first so that we don't track the wrong file.
+  (when auto-revert-notify-watch-descriptor
+    (auto-revert-notify-rm-watch))
+  (auto-revert--global-add-buffer (current-buffer))
+  (auto-revert-set-timer))
+
+(defun auto-revert--after-change-major-mode ()
+  "Adopt the current buffer for Global Auto-Revert if appropriate.
+Called after the current buffer got a new major mode."
+  (auto-revert--global-add-buffer (current-buffer))
+  (auto-revert-set-timer))
 
 (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
+                             'global-auto-revert--tracked-buffer 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
+                          'global-auto-revert--tracked-buffer 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 +736,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)
@@ -700,9 +783,7 @@ If the buffer needs to be reverted, do it now."
   "Check if auto-revert is active (in current buffer or globally)."
   (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)))))
+      global-auto-revert--tracked-buffer))
 
 (defun auto-revert-handler ()
   "Revert current buffer, if appropriate.
-- 
2.20.1 (Apple Git-117)