Subject: Re: bug#34343: [PATCH] Make project--find-regexp-in-files work with remote files

From: Dmitry Gutov <[email-masked]>
Date: Tue, 7 Jan 2020 06:23:45 +0300

On 06.01.2020 20:48, Michael Albinus wrote:
> Dmitry Gutov <[email-masked]> writes:
> 
>> Hi Michael,
> 
> Hi Dmitry,
> 
>> As I've shown before, the generic dispatch is far from being the
>> bottleneck at the moment. tramp-file-name-handler doing a lot of
>> unrelated work is what takes the most time.
> 
> It might be unrelated to yozur use case.

As far as I can tell, it's unrelated to 'file-local-name' altogether, 
not just my use case.

> But tramp-file-name-handler is
> the central heart of Tramp's file name handling. It ensures that the
> proper function is called; that a Tramp subpackage is autoloaded if
> needed; that there are no blocking situations due to simultaneous
> invocations from timers, filters, sentinels; that there are no
> unintended infloops; that Tramp events and errors are caught etc pp. In
> the Tramp repository, it is also responsible for thread handling. All of
> this must be applied for every file name operation, also for
> file-remote-p, even if you don't need some of this in your use case.

I believe it's an incidental detail that file-local-name is implemented 
through file-remote-p. Maybe we could also introduce a new operation as 
well.

>>> In case you want to go your own path, I could provide you an own
>>> tramp-file-local-name function, which bypasses this mechanism:
> 
> That's why I have provided this function. In order to avoid a call of
> file-local-name (which implies the call of file-remote-p).

I can use it, and I will if I can't convince you otherwise, but we *can* 
keep operation both fast and generic at the same time, so... why not, 
really?

> No. file-remote-p must pass the whole machinery. It isn't called only
> for your use case.

Hence the double check which verifies that the second argument is 
'localname'.

> And if we start such bypassing for a file name operation, we will end in
> unmaintainability soon. Over the years, there has been often the
> temptation to bypass this or that file name operation, similar to your
> approach. Sometimes I even did, but soon I have reverted such
> exceptions. Tramp isn't maintainable any longer, if we introduce such
> coding style. Subtle errors you have no idea where they come from.

It's up to you, but having an alist of exceptions should be both 
maintainable and readable. Like most other file handlers do (e.g. 
epa-file-handler) which maintain mappings of operations to functions.

> In general, there's no problem calling tramp-file-name-handler, because
> the remote operations take much more time. This machinery is neglectable
> then. Your use case is different, because you do not trigger any remote
> operation.

Would any use of file-local-name trigger a remote operation?

>> P.S. BTW, I think our convention is not to call save-match-data unless
>> obviously required. If the caller needs to save the match data, it
>> will use it itself.
> 
> Well, as you might have seen with my recent commit in master, I intend
> to use tramp-file-local-name internally. At some places it seems to be
> necessary to protect match data.

Not in the case of the new proposed use in project.el either.

Anyway, the idea is only to do that where necessary to squeeze just a 
little bit of more performance in places that don't need to protect 
match data.