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

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> Most of the time was taken by `find-buffer-visiting'. Replacing
> `find-buffer-visiting' with `get-file-buffer' in certain (not all)
> places reduced the total runtime by 30%. I do not have more granular data
> because the profiler did not give very granular data for the internals
> of `find-buffer-visiting'.
>
> I will try to setup a test on my machine for more detailed data.

Here is a reproducer anyone can try locally:

1. Create a dummy set of 1000 files in /tmp/test/:
   (dotimes (i 1000) (with-temp-file (format "/tmp/test/%d.org" i) (insert "* 
This is test")))

2. emacs -Q
3. Open all the 1000 files one by one:
   (dolist (file (directory-files "/tmp/test/" t "org"))
     (unless (find-buffer-visiting file) (find-file-noselect file)))

Step (3) takes 18.8 seconds on my machine. The CPU profile attached as
cpu-profile.

If one uses `get-file-buffer' instead of `find-buffer-visiting', the
total runtime becomes 5.1 sec - almost 4x faster.

To test:

(dolist (file (directory-files "/tmp/test/" t "org"))
  (unless (get-file-buffer file)
    (cl-letf (((symbol-function 'find-buffer-visiting)
               (lambda (file &optional predicate)
                 (when-let ((buf (get-file-buffer file)))
                   (and (funcall predicate buf) buf)))))
      (find-file-noselect file))))

With `get-file-buffer' instead of `find-buffer-visiting', matching
against the opened buffers no longer dominates the profiler. See the
attached cpu-profile-get-file-buffer.

So, it looks like caching `get-file-buffer' is not really necessary.
>From the profile, the slowest parts of `find-buffer-visiting' are the
two loops checking `buffer-file-truename' and `buffer-file-number' with
most of the time apparently spent executing `with-current-buffer'. I
tested whether `with-current-buffer' is the culprit by replacing it with
`buffer-local-value' calls:

(defun find-buffer-visiting (filename &optional predicate)
  "Return the buffer visiting file FILENAME (a string).
This is like `get-file-buffer', except that it checks for any buffer
visiting the same file, possibly under a different name.

If PREDICATE is non-nil, only buffers satisfying it are eligible,
and others are ignored.  PREDICATE is called with the buffer as
the only argument, but not with the buffer as the current buffer.

If there is no such live buffer, return nil."
  (let ((predicate (or predicate #'identity))
        (truename (abbreviate-file-name (file-truename filename))))
    (or (let ((buf (get-file-buffer filename)))
          (when (and buf (funcall predicate buf)) buf))
        (let ((list (buffer-list)) found)
          (while (and (not found) list)
            (if (and (buffer-local-value 'buffer-file-name (car list))
                     (string= (buffer-local-value 'buffer-file-truename (car 
list)) truename)
                     (funcall predicate (car list)))
                (setq found (car list)))
            (setq list (cdr list)))
          found)
        (let* ((attributes (file-attributes truename))
               (number (file-attribute-file-identifier attributes))
               (list (buffer-list)) found)
          (and buffer-file-numbers-unique
               (car-safe number)       ;Make sure the inode is not just nil.
               (while (and (not found) list)
                 (if (and (buffer-local-value 'buffer-file-name (car list))
                          (equal (buffer-local-value 'buffer-file-number (car 
list)) number)
                          ;; Verify this buffer's file number
                          ;; still belongs to its file.
                          (file-exists-p (buffer-local-value 'buffer-file-name 
(car list)))
                          (equal (file-attributes (buffer-local-value 
'buffer-file-truename (car list)))
                                 attributes)
                          (funcall predicate (car list)))
                     (setq found (car list)))
                 (setq list (cdr list))))
          found))))

The result is 7.8 sec execution time - much better compared to 18.8
seconds in `with-current-buffer' version, but still worse compared to
5.1 sec in `get-file-buffer' version. See the attached
cpu-profile-buffer-local-value.

So, using `with-current-buffer' when looping over all the buffers is
certainly not optimal (maybe in other places as well).

However, even `buffer-local-value' is still costly - it adds up over 50%
run time.

Also, looking at the 5.1 sec profile, there are other things that may
slow down opening a large number of files:

0. GC (as usual)
1. hack-local-variables
2. vc-refresh-state
3. uniquify--create-file-buffer-advice -> uniquify-rationalize-file-buffer-names
4. Org mode loading (nothing new here for me)

cpu-profile
Description: Binary data

cpu-profile-get-file-buffer
Description: Binary data

cpu-profile-buffer-local-value
Description: Binary data

-- 
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>