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: Sat, 23 Sep 2023 08:22:17 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Because `buffer-file-name' can be modified from Lisp (via
> >> `set-visited-file-name' or directly). Same for `buffer-file-truename'
> >> and `buffer-file-number'.
> >
> > You could update the cache in set-visited-file-name, and ignore
> > direct changes.
> 
> I have eyeballed Emacs sources, and it looks like a huge number of
> libraries sets `buffer-file-name' directly.

I think this is an exaggeration, see below.

> Also, even `find-file-noselect' does not use
> `set-visited-file-name'.

Why does it matter?  We need to catch this in find-file-noselect and
in set-visited-file-name anyway.

> - Functions setting ~buffer-file-name~ manually (except those setting it to 
> nil):
>   - tests
>   - ~vc-find-revision-no-save~
>   - ~url-insert-buffer-contents~
>   - ~plstore-open~
>   - ~protect-innocence-hook~ (really?)
>   - ~tramp-handle-insert-file-contents~
>   - ~tramp-archive-handle-insert-file-contents~
>   - ~mailcap-view-file~
>   - ~ange-ftp-parse-netrc~
>   - ~ange-ftp-write-region~
>   - ~ange-ftp-insert-file-contents~
>   - ~mh-display-msg~
>   - ~mh-make-folder~
>   - ~mh-read-draft~
>   - ~feedmail-vm-mail-mode~
>   - ~feedmail-send-it~
>   - ~jka-compr-write-region~
>   - ~jka-compr-insert-file-contents~
>   - ~image-dired-write-tags~
>   - ~image-dired-remove-tag~
>   - ~image-dired-write-comments~
>   - ~hfy-buffer~
>   - ~nndraft-request-associate-buffer~
>   - ~nndraft-auto-save-file-name~
>   - ~nnbabyl-create-mbox~
>   - ~mm-display-inline-fontify~
>   - ~mm-url-insert-file-contents~
>   - ~mm-extern-url~
>   - ~message-send-mail-with-mh~
>   - ~message-set-auto-save-file-name~
>   - ~gnus-dribble-read-file~
>   - ~gnus-save-newsrc-file~
>   - ~gnus-gnus-to-newsrc-format~
>   - ~gnus-mime-copy-part~

IMO, these are not important for your purpose.  The few buffers whose
buffer-file-name is set as in those functions are not going to get in
the way of your looking for a buffer that visits a specific name.  And
if a few of them do, we can always add the cache-updating code to them.

>   - ~find-alternate-file~
>   - ~find-file-noselect-1~ (but not by default?)
>   - ~set-visited-file-name~
>   - ~file-name-non-special~

These _must_ update the cache.

>   - ~erc-dcc-find-file~
>   - ~epa-file-insert-file-contents~
>   - ~epa-file-write-region~
>   - ~save-completions-to-file~
>   - ~load-completions-from-file~
>   - ~archive-extract~

These belong to the first group, I think.

> - Functions setting ~buffer-file-truename~ manually (except those setting it 
> to nil):

Are the cases where we find the buffer via file's truename significant
in the profiles you've seen? if not, these functions are not relevant
to the issue at hand.  If the search by truename _is_ significant, we
could cache that as well.

>   - ~tar-extract~
>   - ~find-alternate-file~
>   - ~find-file-noselect-1~
>   - ~set-visited-file-name~
>   - ~revert-buffer--default~
>   - ~archive-extract~
> 
> 
> - Functions setting ~buffer-file-number~ manually (except those setting it to 
> nil):

Same question as for truename above.

>   - ~find-alternate-file~
>   - ~find-file-noselect-1~
>   - ~set-visited-file-name~
>   - ~basic-save-buffer~
> 
> 
> >> Or should we just assume that these variables remain unchanged other
> >> than by primitives?
> >
> > Programs that make these changes are asking for trouble, IMO.  AFAICT,
> > find-buffer-visiting will never find such buffers anyway.
> 
> It would, in its current form. Because it calls `get-file-buffer' that
> loops over all the buffers and checks their buffer-local
> `buffer-file-name' value, including values set via `setq' in Elisp.

Again, which of the loops took the significant time in the profiles
you have? the one in get-file-buffer or the ones in
find-buffer-visiting?