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

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

[Felicián Németh]

[-- Attachment #1.1: Type: text/plain, Size: 306 bytes --]

Hi,

The attached patch enhances the remote file support of project.el. It also
has a new "require" statement, which might be problematic.  Additionally,
project--collect-matches will look very similar to xref-collect-matches
creating a chance to unify the two functions later.

Thanks,
Felicián

[-- Attachment #1.2: Type: text/html, Size: 389 bytes --]

[-- Attachment #2: 0001-Make-project-find-regexp-in-files-work-with-remote-f.patch --]
[-- Type: text/x-patch, Size: 3382 bytes --]

From 6f65ec9462eebfd405768e1b2c730fc3f4e65b4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Felici=C3=A1n=20N=C3=A9meth?= <felician.nemeth@gmail.com>
Date: Wed, 6 Feb 2019 08:59:34 +0100
Subject: [PATCH] Make project--find-regexp-in-files work with remote files

* project.el: Require seq.
(project--collect-matches): New function extracted
from project--find-regexp-in-files; handle remote files.
(project--find-regexp-in-files): Group files based on their
remote-id, call the new function on the groups.
---
 lisp/progmodes/project.el | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 815cc7cd3d..5833e6401b 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -88,6 +88,7 @@
 ;;; Code:
 
 (require 'cl-generic)
+(require 'seq)
 
 (defvar project-find-functions (list #'project-try-vc)
   "Special hook to find the project containing a given directory.
@@ -374,12 +375,29 @@ project-or-external-find-regexp
     (project--find-regexp-in-files regexp files)))
 
 (defun project--find-regexp-in-files (regexp files)
+  (let* ((xref-groups
+          (mapcar
+           (lambda (group)
+             (let* ((files (cdr group))
+                    (local-files (mapcar #'file-local-name files))
+                    (dir (file-name-directory (car files))))
+               (project--collect-matches regexp local-files dir)))
+           (seq-group-by #'file-remote-p files)))
+         (xrefs (apply #'append xref-groups)))
+    (unless xrefs
+      (user-error "No matches for: %s" regexp))
+    (xref--show-xrefs xrefs nil)))
+
+(defun project--collect-matches (regexp files dir)
+  "Collect matches for REGEXP inside FILES in DIR.
+FILES is a list of file names."
+  ;; Cf. `xref-collect-matches'.
   (pcase-let*
       ((output (get-buffer-create " *project grep output*"))
        (`(,grep-re ,file-group ,line-group . ,_) (car grep-regexp-alist))
        (status nil)
        (hits nil)
-       (xrefs nil)
+       (remote-id (file-remote-p dir))
        (command (format "xargs -0 grep %s -nHe %s"
                         (if (and case-fold-search
                                  (isearch-no-upper-case-p regexp t))
@@ -390,6 +408,7 @@ project--find-regexp-in-files
       (erase-buffer)
       (with-temp-buffer
         (insert (mapconcat #'identity files "\0"))
+        (setq default-directory dir)
         (setq status
               (project--process-file-region (point-min)
                                             (point-max)
@@ -407,13 +426,10 @@ project--find-regexp-in-files
                     (buffer-substring (point-min) (line-end-position))))
       (while (re-search-forward grep-re nil t)
         (push (list (string-to-number (match-string line-group))
-                    (match-string file-group)
+                    (concat remote-id (match-string file-group))
                     (buffer-substring-no-properties (point) (line-end-position)))
               hits)))
-    (setq xrefs (xref--convert-hits (nreverse hits) regexp))
-    (unless xrefs
-      (user-error "No matches for: %s" regexp))
-    (xref--show-xrefs xrefs nil)))
+    (xref--convert-hits (nreverse hits) regexp)))
 
 (defun project--process-file-region (start end program
                                      &optional buffer display
-- 
2.11.0

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

[Dmitry Gutov]

On 06.02.2019 11:18, Felicián Németh wrote:
> The attached patch enhances the remote file support of project.el. It 
> also has a new "require" statement, which might be problematic.

Thank you for the patch, but I'm not a big fan of first creating a list 
of all files, and then grouping them again by directories (or a property 
of directories, but still).

First of all, I think we might reasonably expect that either all files 
are remote (and on the same host), or none of them are.

If that assumption doesn't ring true to you, I think we should instead 
run one xargs process per directory, not the whole file list.

Then, project--find-regexp-in-files would return a list of xrefs (for 
files belonging to one directory), project-find-regexp can nconc it for 
all its directories and call xref--show-xrefs on the result.

Since project-find-regexp and project-or-external-find-regexp will both 
have to do that, a helper function will also be needed.

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

[Felicián Németh]

[-- Attachment #1: Type: text/plain, Size: 277 bytes --]

On Thu, Feb 14, 2019 at 2:17 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> First of all, I think we might reasonably expect that either all files
> are remote (and on the same host), or none of them are.

OK.  I've attached a new patch following that assumption.
Thanks again.

[-- Attachment #2: 0001-Make-project-find-regexp-in-files-work-with-remote-f.patch --]
[-- Type: text/x-patch, Size: 2084 bytes --]

From 9deb42823fd043b80c8a5e8f8e28d94dd41d7b00 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Felici=C3=A1n=20N=C3=A9meth?= <felician.nemeth@gmail.com>
Date: Fri, 15 Feb 2019 19:45:54 +0100
Subject: [PATCH] Make project--find-regexp-in-files work with remote files

* project.el (project--find-regexp-in-files): Handle remote files.
---
 lisp/progmodes/project.el | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 533e27be7e..3647029f56 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -390,6 +390,12 @@ project--find-regexp-in-files
        (status nil)
        (hits nil)
        (xrefs nil)
+       ;; Assuming all files are on the same host.
+       (dir (file-name-directory (car files)))
+       (remote-id (file-remote-p dir))
+       (local-files (if remote-id
+                        (mapcar #'file-local-name files)
+                      files))
        (command (format "xargs -0 grep %s -nHe %s"
                         (if (and case-fold-search
                                  (isearch-no-upper-case-p regexp t))
@@ -399,7 +405,8 @@ project--find-regexp-in-files
     (with-current-buffer output
       (erase-buffer)
       (with-temp-buffer
-        (insert (mapconcat #'identity files "\0"))
+        (insert (mapconcat #'identity local-files "\0"))
+        (setq default-directory dir)
         (setq status
               (project--process-file-region (point-min)
                                             (point-max)
@@ -417,7 +424,7 @@ project--find-regexp-in-files
                     (buffer-substring (point-min) (line-end-position))))
       (while (re-search-forward grep-re nil t)
         (push (list (string-to-number (match-string line-group))
-                    (match-string file-group)
+                    (concat remote-id (match-string file-group))
                     (buffer-substring-no-properties (point) (line-end-position)))
               hits)))
     (setq xrefs (xref--convert-hits (nreverse hits) regexp))
-- 
2.11.0

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

Hi Dmitry,

> Thank you. Before I apply this, I'd like to clarify one question with
> Tramp's maintainer.
>
> Michael, do you think it's doing the right thing, conceptually?
>
> We first call the process in project-files, then process the returned
> files names to make them remote (if necessary).
>
> Then, in project--find-regexp-in-files parses the files names again to
> extract the remote id and the local names, does the process calls and
> prepends the remote id again.
>
> Some kind of alternative would be to only use the local names until
> the end, and keep the remote id in one place only. E.g. in
> default-directory.

Well, I don't know the internal logic in project.el. But indeed, it is
an option to work only with the local file names, and to prepend the
remote part of default-directory at the very end to the results. This
must be done consequently, everywhere.

Best regards, Michael.

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

[Felicián Németh]

Hi Dmitry,

> > Some kind of alternative would be to only use the local names until
> > the end, and keep the remote id in one place only. E.g. in
> > default-directory.
>
> Well, I don't know the internal logic in project.el. But indeed, it is
> an option to work only with the local file names, and to prepend the
> remote part of default-directory at the very end to the results. This
> must be done consequently, everywhere.

So how do you envision remote system support in project.el?

One possibility is to have two new defgenerics,
project-local-files and project-remote-id, and to use those
internally.  But I don't see how project-files could be changed in
a backward compatible manner.  New/updated backends don't have to
implement p-files, because the result can be constructed using
p-local-files and p-remote-id.  But for old backends that only
implement project-files it's the other way around: p-local-files
and p-remote-id should rely on p-files to calculate their return
values.

What do you think?

Thanks,
Felicián

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

[Dmitry Gutov]

Hi Felician,

On 06.03.2019 10:47, Felicián Németh wrote:

> So how do you envision remote system support in project.el?
> 
> One possibility is to have two new defgenerics,
> project-local-files and project-remote-id, and to use those
> internally.

There's no point: generics dispatch to different backends, and I don't 
think we want to have a separate project backend for "remote" files. 
It's not a real type of project.

> But I don't see how project-files could be changed in
> a backward compatible manner.  New/updated backends don't have to
> implement p-files, because the result can be constructed using
> p-local-files and p-remote-id.  But for old backends that only
> implement project-files it's the other way around: p-local-files
> and p-remote-id should rely on p-files to calculate their return
> values.

The way to do that is to either pass the remote-ness information via 
composite return values or a global variable.

I don't think using default-directory is viable for that purpose, 
though: the "current project" is allowed to be in a totally different 
directory. So I'll mull it over a little bit more (if you're not in a 
hurry, of course), and then probably commit one of your patches. Thank you.

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

[Dmitry Gutov]

On 06.03.2019 17:33, Dmitry Gutov wrote:
>>
>> One possibility is to have two new defgenerics,
>> project-local-files and project-remote-id, and to use those
>> internally.
> 
> There's no point: generics dispatch to different backends, and I don't 
> think we want to have a separate project backend for "remote" files. 
> It's not a real type of project.

Although... I see your point now.

So we can mandate that project-files returns local names. And add a new 
method that returns remote-id corresponding to the project.

This will codify that the whole project must have one remote-id. Which 
is the assumption I asked you to make in the implementation, but it's a 
different thing to have in an API.

So I wonder if somebody has an opinion on that. Maybe we'll want to 
include remote files as "external roots" in some projects? Or files 
inside archives?

Opinions welcome.

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

[Felicián Németh]

Hi Dmitry,

I forgot that project-files are new in emacs-27, so there's no need to
worry about backward compatibility.  Also, there is no need to hurry
either, because emacs-27 won't be released soon.  Nevertheless, I would
love to have a convenience function returning "complete" file names,
even if the function's name is not project-files.

> So we can mandate that project-files returns local names. And add a
> new method that returns remote-id corresponding to the project.
>
> This will codify that the whole project must have one remote-id.

> So I wonder if somebody has an opinion on that. Maybe we'll want to
> include remote files as "external roots" in some projects? Or files
> inside archives?

I never worked on a project with multiple remote-ids.  However, if we
can come up with something simple that does not have significant
performance impact on the local case, we should solve the general case,
I think.  Remote operations tend to be slow, so I think project.el can
run more complex algorithms in that case.

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

[Dmitry Gutov]

On 08.03.2019 10:28, Felicián Németh wrote:
> Hi Dmitry,
> 
> I forgot that project-files are new in emacs-27, so there's no need to
> worry about backward compatibility.  Also, there is no need to hurry
> either, because emacs-27 won't be released soon.

Emacs 27 is approaching now. :-)

>> So I wonder if somebody has an opinion on that. Maybe we'll want to
>> include remote files as "external roots" in some projects? Or files
>> inside archives?
> 
> I never worked on a project with multiple remote-ids.  However, if we
> can come up with something simple that does not have significant
> performance impact on the local case, we should solve the general case,
> I think.  Remote operations tend to be slow, so I think project.el can
> run more complex algorithms in that case.

I've done some benchmarking. If the project is big (e.g. gecko-dev with 
200000+ files), and it resides locally, and the hdd is fast, and we can 
fetch the list of files quickly (i.e. it uses Git),

   (seq-group-by #'file-remote-p all-files)

is pretty slow.

For instance, in that example on my machine,

   (project-files (project-current))

takes 1.6s.

And

   (seq-group-by #'file-remote-p all-files)

itself takes 1s, and

   (mapcar #'file-local-name files))

takes another 0.9s. While Xargs+Grep only take another 3-4 seconds. So 
overall the remoteness processing takes a significant portion of the 
time in the local case even if we do it the smart way (e.g. avoid 
mapping through #'file-local-name if remote-id is nil).

So I've pushed a more simplistic patch to emacs-27 (commit be38e39fcc).

Felicián and Michael, please take a look.

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

Hi Dmitry,

> So I've pushed a more simplistic patch to emacs-27 (commit be38e39fcc).
>
> Felicin and Michael, please take a look.

The patch looks reasonable to me. Since I don't use project-files, I
can't say whether this works correctly.

Your use of remote-id reminds me of comint-file-name-prefix, which
serves a similar purpose. Maybe you can check it in comint.el and the
other packages where it is used, and steal some ideas from there :-)

Best regards, Michael.

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

[Dmitry Gutov]

On 27.12.2019 10:24, Michael Albinus wrote:

> The patch looks reasonable to me. Since I don't use project-files, I
> can't say whether this works correctly.

Unless it's a religious position, you can test it with:

   M-x project-find-regexp

or

   M-x dired-do-find-regexp (also bound to 'A' in Dired)

(the latter since one of my yesterday's commits).

For now, I've done some testing myself. It prepended 'remote-id' twice, 
now fixed.

And some benchmarking.

While project-find-file (the command actually using project-files) 
completed fairly quickly with my remote connection (1 to 3 seconds), the 
regexp search took a while (like 50 seconds) when there are a lot of 
matches (~500).

Things I've noticed:

- (mapcar #'file-local-name files) takes like ~2 seconds when FILES only 
has ~3000 elements. The same code completes instantly on a local 
machine. Shouldn't it be the same?

- Most of the time is spent in 'find-buffer-visiting'. I wonder if that 
function's performance can be improved. For now I've decided to avoid 
calling it unless really necessary (so, no free syntax highlighting for 
Tramp users, even when the file is already visited).

Anyway, I also wanted to compare this to what we had before 
(dired-do-search), and that one doesn't work at all on remote 
directories. So it must be an improvement either way.

> Your use of remote-id reminds me of comint-file-name-prefix, which
> serves a similar purpose. Maybe you can check it in comint.el and the
> other packages where it is used, and steal some ideas from there :-)

Good suggestion, it could be a buffer-local var, or a field in 
xref-file-location. Or we could just avoid printing it in the group names.

The simplest fix is below, but it kind of requires file-local-name to be 
fast:

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index bbd3940be4..5c38ae910c 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -142,7 +142,7 @@ xref-location-marker

  (cl-defmethod xref-location-group ((l xref-file-location))
    (cl-ecase xref-file-name-display
-    (abs (oref l file))
+    (abs (file-local-name (oref l file)))
      (nondirectory (file-name-nondirectory (oref l file)))))

  (defclass xref-buffer-location (xref-location)

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

[Michael Albinus]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

Dmitry Gutov <dgutov@yandex.ru> writes:

Hi Dmitry,

>> The patch looks reasonable to me. Since I don't use project-files, I
>> can't say whether this works correctly.
>
> Unless it's a religious position, you can test it with:
>
>   M-x project-find-regexp
>
> or
>
>   M-x dired-do-find-regexp (also bound to 'A' in Dired)
>
> (the latter since one of my yesterday's commits).

No religious barrier here :-)

> For now, I've done some testing myself. It prepended 'remote-id'
> twice, now fixed.

I've tested with the emacs-27 branch, and it seems to work.

> Things I've noticed:
>
> - (mapcar #'file-local-name files) takes like ~2 seconds when FILES
>   only has ~3000 elements. The same code completes instantly on a
>  local machine. Shouldn't it be the same?

file-local-name uses internally file-remote-p, which calls
expand-file-name if the connection is already established. Not needed in
case of local names.

Could you pls test the appended patch, whether it improves performance?

Best regards, Michael.


[-- Attachment #2: Type: text/plain, Size: 913 bytes --]

diff --git a/lisp/tramp.el b/lisp/tramp.el
index 0dbbfa59..a25b2715 100644
--- a/lisp/tramp.el
+++ b/lisp/tramp.el
 ;; This file is part of GNU Emacs.
@@ -3361,10 +3361,12 @@ User is always nil."
       (let* ((v (tramp-dissect-file-name filename))
 	     (p (tramp-get-connection-process v))
 	     (c (and (process-live-p p)
-		     (tramp-get-connection-property p "connected" nil))))
-	;; We expand the file name only, if there is already a connection.
+		     (tramp-get-connection-property p "connected" nil)))
+	     (e (and c (not (eq identification 'localname)))))
+	;; We expand the file name only, if there is already a
+	;; connection, and IDENTIFICATION is not `localname'.
 	(with-parsed-tramp-file-name
-	    (if c (expand-file-name filename) filename) nil
+	    (if e (expand-file-name filename) filename) nil
 	  (and (or (not connected) c)
 	       (cond
 		((eq identification 'method) method)

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

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Dmitry,

> file-local-name uses internally file-remote-p, which calls
> expand-file-name if the connection is already established. Not needed in
> case of local names.
>
> Could you pls test the appended patch, whether it improves performance?

Please forget this. I ran Tramp regression tests over night, and some of
them failed due to this change.

So we need a cheaper file-local-name. Just checking for
tramp-file-name-regexp isn't sufficient, because the file could be
"remote" independent of Tramp. Let me contemplate about.

Best regards, Michael.

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

[Dmitry Gutov]

On 28.12.2019 12:21, Michael Albinus wrote:
> So we need a cheaper file-local-name.

Or a change in semantics for the existing functions. Which is riskier, 
but... *shrug*

> Just checking for
> tramp-file-name-regexp isn't sufficient, because the file could be
> "remote" independent of Tramp.

Why does that matter? Why should the local part of the file name be 
different depending on the transport?

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> Just checking for tramp-file-name-regexp isn't sufficient, because
>> the file could be "remote" independent of Tramp.
>
> Why does that matter? Why should the local part of the file name be
> different depending on the transport?

file-remote-p doesn't exist for Tramp only. For example, there is also
url-handlers.el.

Best regards, Michael.

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

[Dmitry Gutov]

On 27.12.2019 19:57, Michael Albinus wrote:
> file-local-name uses internally file-remote-p, which calls
> expand-file-name if the connection is already established

Why? If the caller needs an expanded name, they can call 
expand-file-name themselves. Or if they need an expanded local name:

     (file-local-name (expand-file-name filename))

Aside from performance problems, this feels like an inconsistent behavior.

> Could you pls test the appended patch, whether it improves performance?

In any case, this didn't help because in my main scenario 
identification=localname. Whatever that means.

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 27.12.2019 19:57, Michael Albinus wrote:
>> file-local-name uses internally file-remote-p, which calls
>> expand-file-name if the connection is already established
>
> Why? If the caller needs an expanded name, they can call
> expand-file-name themselves. Or if they need an expanded local name:
>
>     (file-local-name (expand-file-name filename))
>
> Aside from performance problems, this feels like an inconsistent behavior.

I said already, that expand-file-name isn't needed for
file-local-name. I will try to find a better implementation.

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

[Dmitry Gutov]

On 28.12.2019 20:46, Michael Albinus wrote:

> I said already, that expand-file-name isn't needed for
> file-local-name. I will try to find a better implementation.

Sorry, I guess I misunderstood you, then. I thought you meant to write a 
new function, "a cheaper file-local-name".

BTW, does file-remote-p always call expand-file-name if the connection 
is already established?

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

Hi Dmitry,

>> I said already, that expand-file-name isn't needed for
>> file-local-name. I will try to find a better implementation.
>
> Sorry, I guess I misunderstood you, then. I thought you meant to write
> a new function, "a cheaper file-local-name".

There could be both solutions, improve file-local-name, or create a new
function. My strong preference is the former, of course.

> BTW, does file-remote-p always call expand-file-name if the connection
> is already established?

Not always. But even if it calls expand-file-name, this doesn't mean
always that a remote command is fired. This is what is expensive.

Best regards, Michael.

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

[Dmitry Gutov]

On 29.12.2019 14:34, Michael Albinus wrote:
> Not always. But even if it calls expand-file-name, this doesn't mean
> always that a remote command is fired. This is what is expensive.

That depends on our performance goals. As described in 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=34343#29, when called on a 
lot of files, it can be fairly expensive even when offline.

But eliminating remote calls will be a win, for sure.

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> Not always. But even if it calls expand-file-name, this doesn't mean
>> always that a remote command is fired. This is what is expensive.
>
> That depends on our performance goals. As described in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=34343#29, when called on
> a lot of files, it can be fairly expensive even when offline.
>
> But eliminating remote calls will be a win, for sure.

I've played a little bit with this. Even if there is no remote file name
involved, file-local-name is slow due to the file-remote-p call and the
file name handler mechanery. See:

--8<---------------cut here---------------start------------->8---
(benchmark-run-compiled
    nil
  (dotimes (i 1000000) (format "/tmp/%d" i)))

=> (1.720033035 16 1.2495203040000007)
--8<---------------cut here---------------end--------------->8---

This is my initial example. Evall'ing `format' 1.000.000 times.

--8<---------------cut here---------------start------------->8---
(benchmark-run-compiled
    nil
  (dotimes (i 1000000) (file-local-name (format "/tmp/%d" i))))

=> (5.072258441 16 1.2559830709999957)
--8<---------------cut here---------------end--------------->8---

Although still a local file name, the elapsed time is about three times
as large in the initial example.

--8<---------------cut here---------------start------------->8---

(benchmark-run-compiled
    nil
  (let ((remote (file-remote-p  "/tmp")))
    (dotimes (i 1000000)
      (if remote
	  (file-local-name (format "%s/%d" remote i))
	(format "%s/%d" remote i)))))

=> (1.831333636 16 1.2604051830000103)
--8<---------------cut here---------------end--------------->8---

Refactoring the code, and applying `file-local-name' just to the cases
where it is needed, shows an elapsed time similar to the initial one.

Best regards, Michael.

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

[Dmitry Gutov]

Hi Michael,

On 01.01.2020 14:29, Michael Albinus wrote:
> I've played a little bit with this. Even if there is no remote file name
> involved, file-local-name is slow due to the file-remote-p call and the
> file name handler mechanery. See:

This is unfortunate. But there's a bigger problem: file-local-name is 
even slower when the file name actually *is* remote.

> Refactoring the code, and applying `file-local-name' just to the cases
> where it is needed, shows an elapsed time similar to the initial one.

If you take a look at xref-matches-in-files, you will find that is 
exactly what I did.

But check this out:

ELISP> (benchmark-run-compiled
     nil
   (dotimes (i 10000) (file-local-name (format "/tmp/%d" i))))

(0.01541569 0 0.0)

ELISP> (benchmark-run-compiled
     nil
   (dotimes (_i 10000) (file-local-name (format "/ssh:abc@def.com:/"))))
(3.229403787 28 1.9053286330000034)

And that is for just 10'000 files, not for 1'000'000.

And with longer file names, the process takes even longer (twice as long 
in my real-world example).

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hi Michael,

Hi Dmitry,

>
> On 01.01.2020 14:29, Michael Albinus wrote:
>> I've played a little bit with this. Even if there is no remote file name
>> involved, file-local-name is slow due to the file-remote-p call and the
>> file name handler mechanery. See:
>
> This is unfortunate. But there's a bigger problem: file-local-name is
> even slower when the file name actually *is* remote.
>
>> Refactoring the code, and applying `file-local-name' just to the cases
>> where it is needed, shows an elapsed time similar to the initial one.
>
> If you take a look at xref-matches-in-files, you will find that is
> exactly what I did.
>
> But check this out:
>
> ELISP> (benchmark-run-compiled
>     nil
>   (dotimes (i 10000) (file-local-name (format "/tmp/%d" i))))
>
> (0.01541569 0 0.0)
>
> ELISP> (benchmark-run-compiled
>     nil
>   (dotimes (_i 10000) (file-local-name (format "/ssh:abc@def.com:/"))))
> (3.229403787 28 1.9053286330000034)
>
> And that is for just 10'000 files, not for 1'000'000.

I've started with

--8<---------------cut here---------------start------------->8---
(benchmark-run-compiled
    nil
  (dotimes (i 10000) (file-local-name (format "/ssh:abc@def.com:/%d" i))))

(4.222947337 31 1.384718521)
--8<---------------cut here---------------end--------------->8---

Then I have removed everything from tramp-handle-file-remote-p which
isn't necessary for the 'localname case, and I came to

--8<---------------cut here---------------start------------->8---
(benchmark-run-compiled
    nil
  (dotimes (i 10000) (file-local-name (format "/ssh:abc@def.com:/%d" i))))

(3.876818843 28 1.3257780540000006)
--8<---------------cut here---------------end--------------->8---

Better, but not significantly. And tramp-handle-file-remote-p wasn't
usable for other cases anymore. So I fear we cannot do much better as it
is.

> And with longer file names, the process takes even longer (twice as
> long in my real-world example).

I cannot confirm "twice as long". I've tried

--8<---------------cut here---------------start------------->8---

(benchmark-run-compiled
    nil
  (dotimes (i 10000) (file-local-name (format "/ssh:abc@def.com:/a/b/c/d/e/f/g/h/i/j/k/l/m%d" i))))

(4.185161814 30 1.425226717000001)
--8<---------------cut here---------------end--------------->8---

The difference is likely because tramp-file-name-regexp has been
applied to a longer string.

If I simplify tramp-file-name-regexp, we get

--8<---------------cut here---------------start------------->8---
(setq tramp-file-name-regexp "\\`/[^/:]+:[^/:]*:")
"\\`/[^/:]+:[^/:]*:"

(benchmark-run-compiled
    nil
  (dotimes (i 10000) (file-local-name (format "/ssh:abc@def.com:/a/b/c/d/e/f/g/h/i/j/k/l/m%d" i))))
(3.510464452 30 1.4363330669999996)
--8<---------------cut here---------------end--------------->8---

Better, but with the penalty that the other parts of a remote file name
aren't detected any longer, and tramp-handle-file-remote-p still works
only for the 'localname part. Both not acceptable.

So I fear we must live with the status. Yes, file-local-name is slow for
remote files, and yes, it doesn't matter for single invocations. Bulk
invocation, as you have it, will show an additional performance penalty,
even if there is no remote execution of any command.

Best regards, Michael.

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

[Dmitry Gutov]

On 02.01.2020 12:48, Michael Albinus wrote:
> Then I have removed everything from tramp-handle-file-remote-p which
> isn't necessary for the 'localname case, and I came to
>  ...
> I cannot confirm "twice as long". I've tried ...

My file names are longer still. I can give a specific example later, if 
you like.

> The difference is likely because tramp-file-name-regexp has been
> applied to a longer string.
> 
> If I simplify tramp-file-name-regexp, we get ...

Both are fine avenues for exploration, I think.

But most importantly for now: tramp-handle-file-remote-p does not take 
up the majority of time spent in file-remote-p. tramp-file-name-handler 
does:

ELISP> (benchmark 10000 '(file-remote-p "/ssh:abc@def.com:/%d"
                                  nil t))
"Elapsed time: 3.535115s (2.112970s in 27 GCs)"

ELISP> (benchmark 10000 '(tramp-handle-file-remote-p 
"/ssh:abc@def.com:/%d" nil t))
"Elapsed time: 0.780867s (0.645621s in 8 GCs)"

ELISP> (benchmark 10000 '(find-file-name-handler "/ssh:abc@def.com:/" 
'file-remote-p))
"Elapsed time: 0.070135s"

ELISP> (benchmark 10000 '(tramp-file-name-handler 'file-remote-p 
"/ssh:abc@def.com:/%d"  nil t))
"Elapsed time: 3.409674s (2.070461s in 27 GCs)"

And that function does a lot of things that I'm not sure we need to just 
get the remote id.

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

> But most importantly for now: tramp-handle-file-remote-p does not take
> up the majority of time spent in
> file-remote-p. tramp-file-name-handler does:
>
> ELISP> (benchmark 10000 '(file-remote-p "/ssh:abc@def.com:/%d"
>                                  nil t))
> "Elapsed time: 3.535115s (2.112970s in 27 GCs)"
>
> ELISP> (benchmark 10000 '(tramp-handle-file-remote-p
> "/ssh:abc@def.com:/%d" nil t))
> "Elapsed time: 0.780867s (0.645621s in 8 GCs)"
>
> ELISP> (benchmark 10000 '(find-file-name-handler "/ssh:abc@def.com:/"
> 'file-remote-p))
> "Elapsed time: 0.070135s"
>
> ELISP> (benchmark 10000 '(tramp-file-name-handler 'file-remote-p
> "/ssh:abc@def.com:/%d"  nil t))
> "Elapsed time: 3.409674s (2.070461s in 27 GCs)"
>
> And that function does a lot of things that I'm not sure we need to
> just get the remote id.

Yes. But in case you want to be generic (handling all possible remote
file names, not only the Tramp implementation), you must use the file
name handler implementation, as it is described in
(info "(elisp) Magic File Names")

In case you want to go your own path, I could provide you an own
tramp-file-local-name function, which bypasses this mechanism:

--8<---------------cut here---------------start------------->8---
;; This function bypasses the file name handler approach.  It is NOT
;; recommended to use it in any package if not absolutely necessary,
;; because it won't work for remote file names not supported by Tramp.
;; However, it is more performant than `file-local-name', and might be
;; useful where performance matters, like in operations over a bulk
;; list of file names.
(defun tramp-file-local-name (file)
  "Return the local name component of FILE.
This function removes from FILE the specification of the remote
host and the method of accessing the host, leaving only the part
that identifies FILE locally on the remote system.  FILE must be
a string that matches `tramp-file-name-regexp'.
The returned file name can be used directly as argument of
‘process-file’, ‘start-file-process’, or ‘shell-command’."
  (and (tramp-tramp-file-p file)
       (string-match (nth 0 tramp-file-name-structure) file)
       (match-string (nth 4 tramp-file-name-structure) file)))
--8<---------------cut here---------------end--------------->8---

Here are the performance figures for local and remote files. I still use
the dolist construct in order to have different file names for every
file-local-name call; this avoids possible caching effects.

--8<---------------cut here---------------start------------->8---
(benchmark-run-compiled
    nil
  (let* ((dir "/path")
	 (remote (file-remote-p  dir)))
    (dotimes (i 10000)
      (if remote
	  (tramp-file-local-name (format "%s/%d" dir i))
	(format "%s/%d" dir i)))))

(0.09890154 1 0.07902910999999996)
--8<---------------cut here---------------end--------------->8---

--8<---------------cut here---------------start------------->8---
(benchmark-run-compiled
    nil
  (let* ((dir "/ssh:abc@def.com:/path")
	 (remote (file-remote-p  dir)))
    (dotimes (i 10000)
      (if remote
	  (tramp-file-local-name (format "%s/%d" dir i))
	(format "%s/%d" dir i)))))

(0.370237273 2 0.05851823899999997)
--8<---------------cut here---------------end--------------->8---

So it is four times slower for remote file names, but I believe the
numbers are acceptable. At least compared with file-local-name.

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

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 1460 bytes --]

Hi Michael,

On 03.01.2020 11:28, Michael Albinus wrote:

> Yes. But in case you want to be generic (handling all possible remote
> file names, not only the Tramp implementation), you must use the file
> name handler implementation, as it is described in
> (info "(elisp) Magic File Names")

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.

> In case you want to go your own path, I could provide you an own
> tramp-file-local-name function, which bypasses this mechanism:

Thank you. I see you've already pushed this function to the release branch.

I could use it like this, sure, but we can have a significant 
performance improvement and keep the generic-ness at the same time.

The simplest option is below. It works fast enough in my testing (*); a 
bit slower than calling tramp-file-local-name directly, but not by much. 
Certainly much faster than the current state of affairs.

It's a bit messy, but I'm not sure how to structure the resulting 
function best. There are several ways, though. What do you think?

(*) The 1'000'000 files example was an extreme one, to optimize for when 
files are local. The 10'000 file project is a real one, on the other hand.

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.

[-- Attachment #2: tramp-local-name.diff --]
[-- Type: text/x-patch, Size: 1080 bytes --]

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 85330e98aa..34438bd276 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -2322,6 +2322,9 @@ tramp-locker
 (defun tramp-file-name-handler (operation &rest args)
   "Invoke Tramp file name handler for OPERATION and ARGS.
 Fall back to normal file name handler if no Tramp file name handler exists."
+  (if (and (eq operation 'file-remote-p)
+           (eq (cadr args) 'localname))
+      (tramp-file-local-name (car args))
   (let ((filename (apply #'tramp-file-name-for-operation operation args))
 	;; `file-remote-p' is called for everything, even for symbolic
 	;; links which look remote.  We don't want to get an error.
@@ -2415,7 +2418,7 @@ tramp-file-name-handler
 
       ;; When `tramp-mode' is not enabled, or the file name is quoted,
       ;; we don't do anything.
-      (tramp-run-real-handler operation args))))
+      (tramp-run-real-handler operation args)))))
 
 (defun tramp-completion-file-name-handler (operation &rest args)
   "Invoke Tramp file name completion handler for OPERATION and ARGS.

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> 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. 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.

>> 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 could use it like this, sure, but we can have a significant
> performance improvement and keep the generic-ness at the same time.
>
> The simplest option is below. It works fast enough in my testing (*);
> a bit slower than calling tramp-file-local-name directly, but not by
> much. Certainly much faster than the current state of affairs.
>
> It's a bit messy, but I'm not sure how to structure the resulting
> function best. There are several ways, though. What do you think?

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

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.

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.

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

Well, I will analyze all these calls, and maybe there are only one or
two places. In this case I can remove save-match-data from the function,
and apply it directly where needed. Let's see.

Best regards, Michael.

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

[Dmitry Gutov]

On 06.01.2020 20:48, Michael Albinus wrote:
> Dmitry Gutov <dgutov@yandex.ru> 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.

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

Hi Dmitry,

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

I've done this, tramp-file-local-name :-)

But I wonder how you want to extract a file name's local part w/o
knowing how the remote part is identified. Again, we're not speaking
only about Tramp. If you have a better proposal for file-local-name
implementation I'm all ears.

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

At least for Emacs 27.1 there won't be another solution. Eli is right,
when he rejects any substantial change to the emacs-27 branch before
Emacs 27.1 is released.

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

I told you already that I have bad experience with such exceptions. All
of them I had to revert later, and I don't want to reopen this can of
worms.

epa-file-handler is different. In epa, there exist exactly one
implementation for a given file name operation; in Tramp you have
different implementations due to the subpackages. epa doesn't need to
care about timers, filters, sentinels, threads AFAICT. epa cares about
exactly two file name operations (insert-file-contents and
write-region), Tramp is responsible for 71 file name operations.

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

No, it shouldn't.

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

I'll check today how I could remove save-match-data.

OTOH, the version of tramp-file-local-name in master handles also the
case that the argument is NOT a remote file name, but a local
one. Needed in my Tramp internal use cases. I will backport this to the
emacs-27 branch, that you could also profit from if you want. It does
not cause a performance penalty for remote file names; an additional
`or' shouldn't hurt.

Best regards, Michael.

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

[Dmitry Gutov]

On 07.01.2020 11:19, Michael Albinus wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
> Hi Dmitry,
> 
>> 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.
> 
> I've done this, tramp-file-local-name :-)

I mean a new operation in terms of file handlers. As in the OPERATION 
argument to tramp-file-name-handler.

> But I wonder how you want to extract a file name's local part w/o
> knowing how the remote part is identified. Again, we're not speaking
> only about Tramp. If you have a better proposal for file-local-name
> implementation I'm all ears.

Just this:

(defun file-local-name (file)
   "Return the local name component of FILE.
This function removes from FILE the specification of the remote host
and the method of accessing the host, leaving only the part that
identifies FILE locally on the remote system.
The returned file name can be used directly as argument of
`process-file', `start-file-process', or `shell-command'."
   (let ((handler (find-file-name-handler file 'file-local-name)))
     (if handler
         (funcall handler 'file-local-name file)
       ;; Until all the implementations switch over,
       ;; not sure how long to keep the compatibility here.
       (or (file-remote-p file 'localname) file))))

And yes, if we were speaking only about Tramp, I wouldn't worry about 
keeping the operation generic.

> At least for Emacs 27.1 there won't be another solution. Eli is right,
> when he rejects any substantial change to the emacs-27 branch before
> Emacs 27.1 is released.

Fair enough.

>> 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.
> 
> I told you already that I have bad experience with such exceptions. All
> of them I had to revert later, and I don't want to reopen this can of
> worms.

Well, if you like the following piece of code, I guess we could live 
with that.

       (setq files (mapcar
                    (if (tramp-tramp-file-p dir)
                        #'tramp-file-local-name
                        #'file-local-name)
                    files)))

By the way, I have no idea what to do about having tramp-tramp-file-p 
called twice.

> epa-file-handler is different. In epa, there exist exactly one
> implementation for a given file name operation; in Tramp you have
> different implementations due to the subpackages. epa doesn't need to
> care about timers, filters, sentinels, threads AFAICT. epa cares about
> exactly two file name operations (insert-file-contents and
> write-region), Tramp is responsible for 71 file name operations.

OK.

>>> 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?
> 
> No, it shouldn't.

That seems to hint that it doesn't need to be implemented in terms of 
file-remote-p.

> OTOH, the version of tramp-file-local-name in master handles also the
> case that the argument is NOT a remote file name, but a local
> one. Needed in my Tramp internal use cases. I will backport this to the
> emacs-27 branch, that you could also profit from if you want. It does
> not cause a performance penalty for remote file names; an additional
> `or' shouldn't hurt.

It won't affect my use, but it sounds like a good idea either way.

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

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

> (defun file-local-name (file)
>   "Return the local name component of FILE.
> This function removes from FILE the specification of the remote host
> and the method of accessing the host, leaving only the part that
> identifies FILE locally on the remote system.
> The returned file name can be used directly as argument of
> `process-file', `start-file-process', or `shell-command'."
>   (let ((handler (find-file-name-handler file 'file-local-name)))
>     (if handler
>         (funcall handler 'file-local-name file)
>       ;; Until all the implementations switch over,
>       ;; not sure how long to keep the compatibility here.
>       (or (file-remote-p file 'localname) file))))

handler would be tramp-file-name-handler, so I don't see how it helps.

> Well, if you like the following piece of code, I guess we could live
> with that.
>
>       (setq files (mapcar
>                    (if (tramp-tramp-file-p dir)
>                        #'tramp-file-local-name
>                        #'file-local-name)
>                    files)))

That would work. If you are sure that you will handle only Tramp based
remote files, you could simplify this to

(setq files (mapcar #'tramp-file-local-name files))

> By the way, I have no idea what to do about having tramp-tramp-file-p
> called twice.

Take the tramp-tramp-file-p call out of mapcar, like

(let ((fun (if (tramp-tramp-file-p dir)
               #'tramp-file-local-name #'file-local-name)))
  (setq files (mapcar fun files)))

Best regards, Michael.

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

[Dmitry Gutov]

On 07.01.2020 16:29, Michael Albinus wrote:

> handler would be tramp-file-name-handler, so I don't see how it helps.

See my previous patch. It would only need one comparison, and 
file-remote-p wouldn't need to be mentioned.

>> Well, if you like the following piece of code, I guess we could live
>> with that.
>>
>>        (setq files (mapcar
>>                     (if (tramp-tramp-file-p dir)
>>                         #'tramp-file-local-name
>>                         #'file-local-name)
>>                     files)))
> 
> That would work. If you are sure that you will handle only Tramp based
> remote files, you could simplify this to
> 
> (setq files (mapcar #'tramp-file-local-name files))

Of course I'm not sure. Hence the conditional.

>> By the way, I have no idea what to do about having tramp-tramp-file-p
>> called twice.
> 
> Take the tramp-tramp-file-p call out of mapcar, like
> 
> (let ((fun (if (tramp-tramp-file-p dir)
>                 #'tramp-file-local-name #'file-local-name)))
>    (setq files (mapcar fun files)))

The above is pretty much equivalent.

Anyway, please never mind. It's just one extra call to 
tramp-tramp-file-p. Not twice per each file.

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

[Lars Ingebrigtsen]

Skimming this thread, I'm not quite sure what the status here is.   But
there's this:

commit be38e39fccab7c2f8e86c59ffb9c022d3d5b9382
Author:     Dmitry Gutov <dgutov@yandex.ru>
AuthorDate: Thu Dec 26 15:38:53 2019 +0200

    project--find-regexp-in-files: Support remote files
    
    * lisp/progmodes/project.el (project--find-regexp-in-files):
    Support remote files (bug#34343).

Does this mean that this bug report should be closed?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Dmitry Gutov]

On 22.07.2021 16:00, Lars Ingebrigtsen wrote:
> commit be38e39fccab7c2f8e86c59ffb9c022d3d5b9382
> Author:     Dmitry Gutov<dgutov@yandex.ru>
> AuthorDate: Thu Dec 26 15:38:53 2019 +0200
> 
>      project--find-regexp-in-files: Support remote files
>      
>      * lisp/progmodes/project.el (project--find-regexp-in-files):
>      Support remote files (bug#34343).
> 
> Does this mean that this bug report should be closed?

I suppose the original complaint is resolved, even if file-local-name's 
performance necessitated the workaround.

The seems to be no progress on improving the latter, but it's out of my 
hands anyway, so I guess we can close this report. Thanks to all involved.

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

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> The seems to be no progress on improving the latter, but it's out of
> my hands anyway, so I guess we can close this report. Thanks to all
> involved.

OK; closing.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Dmitry Gutov]

On 02.01.2020 12:48, Michael Albinus wrote:
> Yes, file-local-name is slow for
> remote files, and yes, it doesn't matter for single invocations. Bulk
> invocation, as you have it, will show an additional performance penalty,
> even if there is no remote execution of any command.

The unfortunate imbalance here at the moment is that piping the list of 
file names through file-local-name takes longer than the time to fetch 
all the project files, and the time to search them, combined with the 
all the necessary round-trips to the servers I have been testing at 
(where the RTT is > 200 ms). As soon as the project is at least of 
moderate size.

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

[Felician Nemeth]

Hi Dmitry,

>> So I've pushed a more simplistic patch to emacs-27 (commit be38e39fcc).
>>
>> Felicián and Michael, please take a look.

I ran some simple tests and project-file-regexp on a remote project also
works for me.  I cannot comment on the performance issues but I'm
grateful for your and Michael's work on that. 

(I guess it's an independent issue, because it seems project-search is
implemented differently, but if I project-search for something and the
point is below a match in a non-visible buffer, then project-search pops
the buffer, but doesn't move the point to the matching string.)

Thanks,
Felicián

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

[Dmitry Gutov]

On 06.01.2020 19:29, Felician Nemeth wrote:

> I ran some simple tests and project-file-regexp on a remote project also
> works for me.  I cannot comment on the performance issues but I'm
> grateful for your and Michael's work on that.

Very good, thanks for checking.

> (I guess it's an independent issue, because it seems project-search is
> implemented differently, but if I project-search for something and the
> point is below a match in a non-visible buffer, then project-search pops
> the buffer, but doesn't move the point to the matching string.)

It is. Please file a new bug report with a step-by-step repro, and we'll 
forward it to someone better qualified to respond.