bug#51699: 29.0.50; [PATCH] Improve performance of 'file-name-case-insensitive-p' for Tramp files

[Michael Albinus <michael.albinus@gmx.de>]

Jim Porter <jporterbugs@gmail.com> writes:

Hi Jim,

>>> Most Tramp methods have a 'tramp-FOO-file-name-p', and most of *those*
>>> take a file name string and dissect it. This is a lot of duplicated
>>> effort, so I modified 'tramp-find-foreign-file-name-handler' to pass
>>> the dissected file name to any of the functions that support it (this
>>> is indicated by an 'accepts-vec' property on the function). This
>>> probably warrants some documentation (at least a NEWS entry), but I
>>> wanted to be sure the strategy made sense before I wrote any docs.
>> Yes, this makes sense, and it works in my environment (more
>> regression
>> tests running). I don't understand why you need the 'accepts-vec'
>> property -- is there any operation left, which is passed to
>> `tramp-find-foreign-file-name-handler' and which doesn't accept a VEC,
>> after applying your patch? And if yes, couldn't we apply usual error
>> handling?
>
> There's `tramp-archive-file-name-p' and `tramp-crypt-file-name-p',
> which both want a string filename. I looked over the implementation
> for those and couldn't figure out an easy way to convert them to take
> a VEC. Maybe it's possible though. I also looked into passing both the
> string form and the vec form as separate arguments, but that turned
> out to be even more complex than this implementation.

`tramp-find-foreign-file-name-handler' loops through
`tramp-register-foreign-file-name-handler'. Only operations registered
there need to support a VEC-OR-FILENAME argument.
`tramp-archive-file-name-p' and `tramp-crypt-file-name-p' are not
registered, so no change is needed.

> In addition, I did it this way to prevent any breakage for third-party
> code that calls `tramp-register-foreign-file-name-handler'. If we
> change `tramp-find-foreign-file-name-handler' to pass a VEC all the
> time, then any code out there that expects a string will break. This
> is probably rare, but I've seen a few examples of people doing stuff
> like this over the years,
> e.g. <https://github.com/thinkiny/emacs.d/blob/master/lisp/tramp-jumper.el>.

Yes, and there's also <https://github.com/emacsattic/magit-tramp/blob/master/magit-tramp.el>.
These packages use a lot of internal Tramp functions which are not
documented publicly. So they have always the risk that something fails.

We could (and should) inform the authors of both packages, that the
signature for the `tramp-FOO-file-name-p' functions will change with
Tramp 2.6. As I can see, there's no problem to adapt
`magit-tramp-file-name-p' and `tramp-jumper-file-name-p'. And yes, an
entry in etc/NEWS should be added as well.

> I'm not quite sure what you mean by the "usual error handling"
> though. If there's a simpler way to do this, I'm happy to change the
> implementation. So long as we can minimize the number of times
> `tramp-dissect-file-name' is called, it should be possible to get
> similar performance improvements to the current version of my patch.

The most simple approach in `tramp-find-foreign-file-name-handler' is

--8<---------------cut here---------------start------------->8---
        (when (ignore-errors (funcall (car elt) vec))
--8<---------------cut here---------------end--------------->8---

A  little bit more friendly for debugging:

--8<---------------cut here---------------start------------->8---
	;; The signature of `tramp-FOO-file-name-p' has changed, it
	;; expects a VEC here.
        (when (with-demoted-errors "Error: %S" (funcall (car elt) vec))
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.