bug#66117: 30.0.50; `find-buffer-visiting' is slow when opening large number of buffers

[Ihor Radchenko <yantar92@posteo.net>]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Ehmm...  I see that test/src/buffer-tests now have 5 failures that
>> were not there before:
>> 
>>   5 unexpected results:
>>      FAILED  test-buffer-modifications
>>      FAILED  test-kill-buffer-auto-save-default
>>      FAILED  test-kill-buffer-auto-save-delete-no
>>      FAILED  test-kill-buffer-auto-save-delete-yes
>>      FAILED  test-restore-buffer-modified-p
>> 
>> Also, 7 of the lisp/files-test fail:
>> 
>>   7 unexpected results:
>>      FAILED  files-tests-bug-18141
>>      FAILED  files-tests-file-name-non-special-make-auto-save-file-name
>>      FAILED  files-tests-file-name-non-special-set-visited-file-modtime
>>      FAILED  files-tests-no-file-write-contents
>>      FAILED  files-tests-revert-buffer
>>      FAILED  files-tests-revert-buffer-with-fine-grain
>>      FAILED  files-tests-zzdont-rewrite-precious-files
>> 
>> Could you please look into these?  (Interestingly, those 5+7 don't
>> fail for me on MS-Windows, only on GNU/Linux...)

FYI, all the tests are passing on my side (GNU/Linux):

cd test
make src/buffer-tests

Ran 406 tests, 406 results as expected, 0 unexpected (2023-12-30 11:24:04+0100, 0.464788 sec)

> I think at least part of the problem is in this fragment from
> find-file-noselect:
>
> 	     ;; Find any buffer for a file that has same truename.
> 	     (other (and (not buf)
>                          (find-buffer-visiting
>                           filename
>                           ;; We want to filter out buffers that we've
>                           ;; visited via symlinks and the like, where
>                           ;; the symlink no longer exists.
>                           (lambda (buffer)
>                             (let ((file (buffer-local-value
>                                          'buffer-file-name buffer)))
>                               (and file (file-exists-p file))))))))

Not this one - (and file ...) will never trigger when file is nil.

>   Test test-buffer-modifications backtrace:
>     file-exists-p(nil)
>     find-buffer-visiting("/tmp/emacs-test-MzitxT-buffer" #f(compiled-fun
>     find-file-noselect("/tmp/emacs-test-MzitxT-buffer" nil nil nil)
>     find-file("/tmp/emacs-test-MzitxT-buffer")
>
> Please attend to this as soon as you can, because I think this same
> problem will break many other use cases.

I think I found the problem - it is rather silly.
See the attached patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-find-buffer-visiting-Fix-test-breakage-introduced-in.patch --]
[-- Type: text/x-patch, Size: 2157 bytes --]

From 0edc966cd7e2b9c2ce3af44a4a72e6f467eaf713 Mon Sep 17 00:00:00 2001
Message-ID: <0edc966cd7e2b9c2ce3af44a4a72e6f467eaf713.1703932575.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Sat, 30 Dec 2023 11:31:51 +0100
Subject: [PATCH] find-buffer-visiting: Fix test breakage introduced in
 b7a737ef49

* lisp/files.el (find-buffer-visiting): Fix code branch checking for
buffers referring to the same file number.  We should check the found
buffer with the file number, not current.

Link: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66117#412
---
 lisp/files.el | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 315ba831e8..78e2bca3cf 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2220,15 +2220,17 @@ find-buffer-visiting
                 (number (file-attribute-file-identifier attributes)))
            (and buffer-file-numbers-unique
                 (car-safe number)       ;Make sure the inode is not just nil.
-                (let ((buf (find-buffer 'buffer-file-number number)))
-                  (when (and buf (buffer-local-value 'buffer-file-name buf)
+                (let* ((buf (find-buffer 'buffer-file-number number))
+                       (buf-file-name (and buf (buffer-local-value 'buffer-file-name buf))))
+                  (when (and buf-file-name
                              ;; Verify this buffer's file number
                              ;; still belongs to its file.
-                             (file-exists-p buffer-file-name)
-                             (equal (file-attributes buffer-file-truename)
-                                    attributes)
+                             (file-exists-p buf-file-name)
+                             (equal
+                              (file-attributes (buffer-local-value 'buffer-file-truename buf))
+                              attributes)
                              (or (not predicate)
-                                 (funcall predicate (current-buffer))))
+                                 (funcall predicate buf)))
                     buf))))))))
 
 \f
-- 
2.42.0


[-- Attachment #3: Type: text/plain, Size: 224 bytes --]


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>