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

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: dmitry@gutov.dev, 66117@debbugs.gnu.org
> Date: Tue, 26 Sep 2023 13:06:04 +0000
> 
> 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.

Since find-file-noselect calls find-buffer-visiting internally, I'm
not sure the above test case makes sense.  A Lisp program should feel
free to call find-file-noselect directly, and Emacs will find the
visiting buffer, if it already exists, as part of the job of
find-file-noselect.

Let's please focus on test cases where the Lisp code being benchmarked
doesn't do any unnecessary stuff, since what's at stake is a
significant change in our internals.

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

This is also not very interesting, since find-file-noselect calls
get-file-buffer as well.

> So, it looks like caching `get-file-buffer' is not really necessary.

I don't think we are ready for conclusions yet, see above.

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

If we come to the conclusion that those loops in find-buffer-visiting
are the hot spot, the right thing is to implement them in C, where we
don't need to use the equivalent of with-current-buffer to examine the
truename and file-number of every buffer, we can just access them
directly.

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

As explained above, both the 18.8 and the 5.1 figures are not good
base lines upon which to make decisions.

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

with-current-buffer is normally very expensive.  Which is why any
performance-critical loop should try to avoid it as much as possible.