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

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

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

fre 2019-05-10 klockan 14:43 +0200 skrev Michael Albinus:
> Mattias Engdegård <mattiase@acm.org> writes:
> 
> So we must document in the Elisp manual, that buffers, which want to
> participate in global-auto-revert-mode after a renaming, shall change
> the name via set-visited-file-name.

Are you sure about that? It sounds quite technical. But if you think it
is necessary, I'll add it.

> 
> > Perhaps we should exclude all buffers whose name start with a space
> > from any kind of auto-revert, just in case.
> 
> Agreed. Those buffers are special (internal) anyway, it's already
> tricky
> to show them. Nobody needs auto-revert for invisible buffers :-)

The revised patch now excludes such non-file buffers. I wonder if
buffers with file names should be excluded as well. They trivially
occur when visiting a file whose name starts with a space.

The new patch also has the tracking variable renamed to
`auto-revert--global-mode' and added a new hook,
`after-set-visited-file-name'; the advice calls are gone.


[-- Attachment #2: 0001-Avoid-polling-in-global-auto-revert-mode.patch --]
[-- Type: text/x-patch, Size: 11730 bytes --]

From 7dabf61a47a20f45ecb7e24e98fcf735f6ab1bd2 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 `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.

(Bug#35418).

* 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.
* 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     | 122 ++++++++++++++++++++++++++++++++---------
 lisp/files.el          |   6 +-
 4 files changed, 103 insertions(+), 29 deletions(-)

diff --git a/doc/lispref/hooks.texi b/doc/lispref/hooks.texi
index 71992464e0..f775aa4d4b 100644
--- a/doc/lispref/hooks.texi
+++ b/doc/lispref/hooks.texi
@@ -251,6 +251,7 @@ Standard Hooks
 
 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 43ad8be1cc..641fc8e116 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1425,8 +1425,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 7cd5e7ee8b..1bf29f04f4 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -312,10 +312,7 @@ auto-revert-avoid-polling
 
 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 @@ auto-revert-buffer-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 @@ global-auto-revert-mode
   :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 @@ auto-revert-notify-handler
                      (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 +772,7 @@ auto-revert-active-p
   "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)))))
+      auto-revert--global-mode))
 
 (defun auto-revert-handler ()
   "Revert current buffer, if appropriate.
diff --git a/lisp/files.el b/lisp/files.el
index 8477c227bc..453a7f4584 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4222,6 +4222,9 @@ change-major-mode-with-file-name
   :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.
@@ -4342,7 +4345,8 @@ set-visited-file-name
 	      (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.
-- 
2.20.1