Re: Remote asynchronous processes

Re: Remote asynchronous processes

[Felipe Lema]

Hello, there (I hope I'm replying correctly)

Answering as I'm interested in this matter, although I'm not a formal
Emacs maintainer myself.

I've tried this patch for async processes and, given that I deal with
far away servers that have a significant delay in answering queries, I
find this most useful. It cuts starting processes from about 5s to less
than 1s.

While this looks promising, I was also wondering about an alternative
solution.

I understand that users have requested dealing with very specific
setups in remote servers and that currently tramp-sh deals with them:
It sends some commands to the server and adjusts the current "command
to be sent".

Instead of sending-and-waiting-for commands, can't these results be
stored in memory? That way tramp-sh can construct the "command to be
sent" /without/ doing queries. If necessary, the results stored in
memory can be updated frequently in the background. Maybe they can even
be persisted for future use (using the `persist` package).

If this is not feasible, then I can manage with this "non-checks-just-
run-this-command-as-is" approach.

Gards

Michael Albinus <address@hidden> wrote:

>Hi,
>
>> On the Tramp ML, there is a discussion about performance of remote
>> asynchronous processes. start-file-process / make-process take too
much
>> time to finish.
>>
>> One of the reasons is, that Tramp opens first a shell on the remote
>> host, performs sanity checks, and runs the command after that. Well,
I
>> cannot change this in general; the sanity checks have been added due
to
>> feedback from users.
>>
>> One idea to change the situation is, to remove all sanity checks
from
>> make-process. That is, if a user has a default directory
>> "/ssh:user@host:/path/to/dir", and if he calls
>>
>> (make-process
>>  :name "test"
>>  :buffer (current-buffer)
>>  :command '("cmd")
>>  :file-handler t))
>>
>> this is translated directly into
>>
>> ssh -l user -o ControlMaster=auto -o ControlPath='tramp.%C' \
>>   -o ControlPersist=no host "cd /path/to/dir; cmd"
>>
>> This would improve performance significantly. The drawback is, that
>> Tramp does not perform convenience checks, like password handling.
>
>I have played with this idea, and the output is the appended file
>tramp-make-process.el. It changes the make-process implementation of
>Tramp for all methods defined in tramp-sh.el (like "ssh") and
>tramp-adb.el.
>
>This is a proof-of-concept, and shouldn't be used for production. Read
>the commentary in the file for limitations.
>
>However, the speed optimization is remarkable. I've tested it with the
>following code snippet:
>
>--8<---------------cut here---------------start------------->8---
>(let ((tramp-verbose 0)
>      (default-directory "/ssh::/"))
>  ;; Fill the caches.
>  (start-file-process "" nil "true")
>  ;; Run benchmark.
>  (benchmark-run 10
>    (start-file-process "" nil "true")))
>--8<---------------cut here---------------end--------------->8---
>
>In the default case, the result is
>
>--8<---------------cut here---------------start------------->8---
>(3.623666842 80 1.183512312)
>--8<---------------cut here---------------end--------------->8---
>
>If tramp-make-process.el is loaded, the result is
>
>--8<---------------cut here---------------start------------->8---
>(0.022762177 0 0.0)
>--8<---------------cut here---------------end--------------->8---
>
>Similar results, if I use "/adb::/" as default directory:
>
>--8<---------------cut here---------------start------------->8---
>(4.599374061 2 0.03429497299999973)
>--8<---------------cut here---------------end--------------->8---
>
>vs
>
>--8<---------------cut here---------------start------------->8---
>(0.013003183 0 0.0)
>--8<---------------cut here---------------end--------------->8---
>
>Comments?
>
>Best regards, Michael.
>

Re: Remote asynchronous processes

[Michael Albinus]

Felipe Lema <felipelema@mortemale.org> writes:

> Hello, there (I hope I'm replying correctly)

Hi Felipe,

Yes, that's the correct reply. And I'm happy that you did. Since there
hasn't been any response for months, I am closed to throw the code away.

> I've tried this patch for async processes and, given that I deal with
> far away servers that have a significant delay in answering queries, I
> find this most useful. It cuts starting processes from about 5s to less
> than 1s.

I'm still undecided how to integrate it into Tramp. Since there are some
restrictions (for example, it must be password-less), I fear a rise of
fault reports ...

> While this looks promising, I was also wondering about an alternative
> solution.
>
> Instead of sending-and-waiting-for commands, can't these results be
> stored in memory? That way tramp-sh can construct the "command to be
> sent" /without/ doing queries. If necessary, the results stored in
> memory can be updated frequently in the background. Maybe they can even
> be persisted for future use (using the `persist` package).

Tramp uses a cache. For every needed information it checks, whether it
is already in the cache. If not, a command is sent remote in order to
gather that information, and the result is cached again. You can see the
cache in `tramp-cache-data'.

This cache has a timeout, defined in `remote-file-name-inhibit-cache'.
The default value is 10 seconds for evry singla data. You might increase
the value (or set it to nil). This shall enable Tramp to use more cache
values.

> If this is not feasible, then I can manage with this "non-checks-just-
> run-this-command-as-is" approach.

If integrated, it must be optional per connection. There might be remote
hosts which could be treated this way; and other hosts which might not.

> Gards

Best regards, Michael.

Re: Remote asynchronous processes

[Michael Albinus]

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

Hi Felipe,

>> If this is not feasible, then I can manage with this "non-checks-just-
>> run-this-command-as-is" approach.
>
> If integrated, it must be optional per connection. There might be remote
> hosts which could be treated this way; and other hosts which might not.

Finally, I've committed a respective patch to Emacs master. Read the
Tramp manual (info "(tramp) Improving performance of asynchronous remote processes")
about.

>> Gards

Best regards, Michael.

Re: Remote asynchronous processes

[Felipe Lema]

whoah, cool... will definitely check it out

Thanks

Felipe

On Tue, 2020-08-04 at 14:27 +0200, Michael Albinus wrote:
> Michael Albinus <michael.albinus@gmx.de> writes:
> 
> Hi Felipe,
> 
> > > If this is not feasible, then I can manage with this "non-checks-
> > > just-
> > > run-this-command-as-is" approach.
> > 
> > If integrated, it must be optional per connection. There might be
> > remote
> > hosts which could be treated this way; and other hosts which might
> > not.
> 
> Finally, I've committed a respective patch to Emacs master. Read the
> Tramp manual (info "(tramp) Improving performance of asynchronous
> remote processes")
> about.
> 
> > > Gards
> 
> Best regards, Michael.

Re: Remote asynchronous processes

[Sean Whitton]

Hello Michael,

On Tue 04 Aug 2020 at 02:27PM +02, Michael Albinus wrote:

> Finally, I've committed a respective patch to Emacs master. Read the
> Tramp manual (info "(tramp) Improving performance of asynchronous
> remote processes") about.

I hope you don't mind me replying here to ask a question about this.

Is there a way to turn this on for all SSH connections which don't use
multi-hops?  I.e. all SSH connections originating from localhost with no
further hops?  I imagine this is something a lot of users would want to
turn on.

I assume it would be unwise to use

(add-to-list 'tramp-connection-properties
                  '("\\`/ssh:.+:/"
                  "direct-async-process" t))

or similar, because there can be implicit multi-hops thanks to tramp
tramp-default-proxies-alist and tramp remembering used proxies.

-- 
Sean Whitton

Re: Remote asynchronous processes

[Michael Albinus]

> Am 06.08.2020 um 21:08 schrieb Sean Whitton <spwhitton@spwhitton.name>:
> 
> Hello Michael,

Hi Sean,

> 
>> On Tue 04 Aug 2020 at 02:27PM +02, Michael Albinus wrote:
>> 
>> Finally, I've committed a respective patch to Emacs master. Read the
>> Tramp manual (info "(tramp) Improving performance of asynchronous
>> remote processes") about.
> 
> I hope you don't mind me replying here to ask a question about this.
> 
> Is there a way to turn this on for all SSH connections which don't use
> multi-hops?  I.e. all SSH connections originating from localhost with no
> further hops?  I imagine this is something a lot of users would want to
> turn on.
> 
> I assume it would be unwise to use
> 
> (add-to-list 'tramp-connection-properties
>                  '("\\`/ssh:.+:/"
>                  "direct-async-process" t))
> 
> or similar, because there can be implicit multi-hops thanks to tramp
> tramp-default-proxies-alist and tramp remembering used proxies.

That's a good idea. Will implement a fallback to Tramp's "classic" implementation of make-network, when there are multi-hops, or stderr is given.

This will take some days before it happens, due to vacation time.

> Sean Whitton

Best regards, Michael.

Re: Remote asynchronous processes

[Philipp Stephani]

Am So., 9. Aug. 2020 um 09:22 Uhr schrieb Michael Albinus
<michael.albinus@gmx.de>:
>
>
> > Am 06.08.2020 um 21:08 schrieb Sean Whitton <spwhitton@spwhitton.name>:
> >
> > Hello Michael,
>
> Hi Sean,
>
> >
> >> On Tue 04 Aug 2020 at 02:27PM +02, Michael Albinus wrote:
> >>
> >> Finally, I've committed a respective patch to Emacs master. Read the
> >> Tramp manual (info "(tramp) Improving performance of asynchronous
> >> remote processes") about.
> >
> > I hope you don't mind me replying here to ask a question about this.
> >
> > Is there a way to turn this on for all SSH connections which don't use
> > multi-hops?  I.e. all SSH connections originating from localhost with no
> > further hops?  I imagine this is something a lot of users would want to
> > turn on.
> >
> > I assume it would be unwise to use
> >
> > (add-to-list 'tramp-connection-properties
> >                  '("\\`/ssh:.+:/"
> >                  "direct-async-process" t))
> >
> > or similar, because there can be implicit multi-hops thanks to tramp
> > tramp-default-proxies-alist and tramp remembering used proxies.
>
> That's a good idea. Will implement a fallback to Tramp's "classic" implementation of make-network, when there are multi-hops, or stderr is given.
>

Why stderr? Does the SSH binary not correctly connect the remote
process's stderr to its own?

Re: Remote asynchronous processes

[Michael Albinus]

> Am 09.08.2020 um 16:47 schrieb Philipp Stephani <p.stephani2@gmail.com>:

Hi Philipp,

>> That's a good idea. Will implement a fallback to Tramp's "classic" implementation of make-network, when there are multi-hops, or stderr is given.
>> 
> 
> Why stderr? Does the SSH binary not correctly connect the remote
> process's stderr to its own?

Because it's not implemented (yet) for direct async processes, see the Tramp manual. When possible, Tramp will also support stderr in this case, but I must check first. If anybody beats me: patches welcome. I'm kind of offline due to vacations.

Best regards, Michael.

Re: Remote asynchronous processes

[Michael Albinus]

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

Hi Sean,

>> Is there a way to turn this on for all SSH connections which don't use
>> multi-hops?  I.e. all SSH connections originating from localhost with no
>> further hops?  I imagine this is something a lot of users would want to
>> turn on.
>>
>> I assume it would be unwise to use
>>
>> (add-to-list 'tramp-connection-properties
>>                  '("\\`/ssh:.+:/"
>>                  "direct-async-process" t))
>>
>> or similar, because there can be implicit multi-hops thanks to tramp
>> tramp-default-proxies-alist and tramp remembering used proxies.
>
> That's a good idea. Will implement a fallback to Tramp's "classic"
> implementation of make-network, when there are multi-hops, or stderr
> is given.

I've committed a patch to master.

>> Sean Whitton

Best regards, Michael.

Re: Remote asynchronous processes

[Sean Whitton]

Hello Michael,

On Wed 12 Aug 2020 at 12:46PM +02, Michael Albinus wrote:

> Michael Albinus <michael.albinus@gmx.de> writes:
>
> Hi Sean,
>
>>> Is there a way to turn this on for all SSH connections which don't use
>>> multi-hops?  I.e. all SSH connections originating from localhost with no
>>> further hops?  I imagine this is something a lot of users would want to
>>> turn on.
>>>
>>> I assume it would be unwise to use
>>>
>>> (add-to-list 'tramp-connection-properties
>>>                  '("\\`/ssh:.+:/"
>>>                  "direct-async-process" t))
>>>
>>> or similar, because there can be implicit multi-hops thanks to tramp
>>> tramp-default-proxies-alist and tramp remembering used proxies.
>>
>> That's a good idea. Will implement a fallback to Tramp's "classic"
>> implementation of make-network, when there are multi-hops, or stderr
>> is given.
>
> I've committed a patch to master.

Current master seems to be working well.

-- 
Sean Whitton

Re: Remote asynchronous processes

[Philipp Stephani]

Am Fr., 31. Juli 2020 um 12:24 Uhr schrieb Michael Albinus
<michael.albinus@gmx.de>:
>
> Felipe Lema <felipelema@mortemale.org> writes:
>
> > Hello, there (I hope I'm replying correctly)
>
> Hi Felipe,
>
> Yes, that's the correct reply. And I'm happy that you did. Since there
> hasn't been any response for months, I am closed to throw the code away.

Please don't! Sorry for not responding earlier, I'm definitely still
interested in this.

>
> > I've tried this patch for async processes and, given that I deal with
> > far away servers that have a significant delay in answering queries, I
> > find this most useful. It cuts starting processes from about 5s to less
> > than 1s.
>
> I'm still undecided how to integrate it into Tramp. Since there are some
> restrictions (for example, it must be password-less), I fear a rise of
> fault reports ...

I guess it should be opt-in since whether or not such an approach can
be taken depends on how the process is to be used (background vs.
foreground, interactive vs. batch, advisory vs. mandatory, ...).
Ideally this would be an optional parameter to `make-process', but I
guess that's infeasible, so I guess either a special filename syntax
("fast-ssh:..."?) or a dynamic variable (not a customization option)
would be the next best approach.


> > If this is not feasible, then I can manage with this "non-checks-just-
> > run-this-command-as-is" approach.
>
> If integrated, it must be optional per connection. There might be remote
> hosts which could be treated this way; and other hosts which might not.

It would definitely be helpful to select the optimized behavior on a
per-host basis, and/or allow users to enable it by default if
feasible. I'd assume that nowadays many of the backwards-compatibility
workarounds are no longer required, and TRAMP could assume (at least
on an opt-in basis) that the optimal way "just works."

Remote asynchronous processes

[Michael Albinus]

Hi,

On the Tramp ML, there is a discussion about performance of remote
asynchronous processes. start-file-process / make-process take too much
time to finish.

One of the reasons is, that Tramp opens first a shell on the remote
host, performs sanity checks, and runs the command after that. Well, I
cannot change this in general; the sanity checks have been added due to
feedback from users.

One idea to change the situation is, to remove all sanity checks from
make-process. That is, if a user has a default directory
"/ssh:user@host:/path/to/dir", and if he calls

--8<---------------cut here---------------start------------->8---
(make-process
 :name "test"
 :buffer (current-buffer)
 :command '("cmd")
 :file-handler t))
--8<---------------cut here---------------end--------------->8---

this is translated directly into

--8<---------------cut here---------------start------------->8---
ssh -l user -o ControlMaster=auto -o ControlPath='tramp.%C' \
  -o ControlPersist=no host "cd /path/to/dir; cmd"
--8<---------------cut here---------------end--------------->8---

This would improve performance significantly. The drawback is, that
Tramp does not perform convenience checks, like password handling.

start-file-process would not be changed, and it behaves like before.

Comments?

Best regards, Michael.

Re: Remote asynchronous processes

[Philippe Vaucher]

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

> Hi,
>

Hello,



> One of the reasons is, that Tramp opens first a shell on the remote
> host, performs sanity checks, and runs the command after that. Well, I
> cannot change this in general; the sanity checks have been added due to
> feedback from users.
>

It'd be great to have the list of checks that are done and the reasons to
see if they still make sense nowadays. Maybe these checks were added to
safe-guard against silly terminals/implementations but can be ignored in
recent times.



> One idea to change the situation is, to remove all sanity checks from
> make-process.
>
> This would improve performance significantly. The drawback is, that
> Tramp does not perform convenience checks, like password handling.
>
> start-file-process would not be changed, and it behaves like before.


I'm not clear on make-process vs start-file-process usage, but I assume the
former is used by API developers and the later is called by TRAMP, so we
still get the sanity checks for normal TRAMP usage?

If you ask _my_ preferences I'd remove the sanity checks and require sane
terminals, maybe offer some tramp-sane-terminal-p that could run the sanity
checks for debugging. That or cache the sanity checks so you only run them
once on the initial connection but never later on. I understand my position
is probably not everyone's position tho :-)

Kind regards,
Philippe

[-- Attachment #2: Type: text/html, Size: 2057 bytes --]

Re: Remote asynchronous processes

[Michael Albinus]

Philippe Vaucher <philippe.vaucher@gmail.com> writes:

> Hello,

Hi Philippe,

>     One of the reasons is, that Tramp opens first a shell on the
>     remote
>     host, performs sanity checks, and runs the command after that.
>     Well, I
>     cannot change this in general; the sanity checks have been added
>     due to
>     feedback from users.
>
> It'd be great to have the list of checks that are done and the reasons
> to see if they still make sense nowadays. Maybe these checks were
> added to safe-guard against silly terminals/implementations but can be
> ignored in recent times.

I've started with such an analysis on the Tramp mailing list, see thread
starting at
<https://lists.gnu.org/archive/html/tramp-devel/2020-03/msg00000.html>. This
work is continued with more fine-tuning, but this isn't the topic of
*this* discussion.

>     One idea to change the situation is, to remove all sanity checks
>     from
>     make-process.
>
>     This would improve performance significantly. The drawback is,
>     that
>     Tramp does not perform convenience checks, like password handling.
>
>     start-file-process would not be changed, and it behaves like
>     before.
>
> I'm not clear on make-process vs start-file-process usage, but I
> assume the former is used by API developers and the later is called by
> TRAMP, so we still get the sanity checks for normal TRAMP usage?

Currently, they behave similar (start-file-process is just a wrapper
around make-process in Tramp). I propose to change this.

> If you ask _my_ preferences I'd remove the sanity checks and require
> sane terminals, maybe offer some tramp-sane-terminal-p that could run
> the sanity checks for debugging. That or cache the sanity checks so
> you only run them once on the initial connection but never later on. I
> understand my position is probably not everyone's position tho :-)

Tramp is intended to work on most remote systems which run a bournish
shell. It does not know in advance the properties of that shell, so it
runs the sanity checks. Tramp caches the properties as much as possible
already.

But that's not the point of my proposal. If we change make-process such a
way that it calls the command directly, we will gain a performace
boost. Philipp Stephani has shown some figures in the discussion
mentioned above. Maybe he can explain more detailed which kind of
performance requirements he has in mind, and the use case(s).

However, we will loose features of remote asynchronous processes. At
least (and not comprehensive), processes started via make-process

- are not checked for passwords or other interactive dialogues
- do not not support multi-hops anymore
- cannot be killed via interrupt-process (??? I'm not sure)
- do not tell the remote tty
- ...

Processes started via start-file-process won't change their behavior.

> Kind regards,
> Philippe

Best regards, Michael.

Re: Remote asynchronous processes

[Philippe Vaucher]

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

>
> >     One idea to change the situation is, to remove all sanity checks
> >     from
> >     make-process.
> >
> >     This would improve performance significantly. The drawback is,
> >     that
> >     Tramp does not perform convenience checks, like password handling.
> >
> >     start-file-process would not be changed, and it behaves like
> >     before.
> >
> > I'm not clear on make-process vs start-file-process usage, but I
> > assume the former is used by API developers and the later is called by
> > TRAMP, so we still get the sanity checks for normal TRAMP usage?
>
> Currently, they behave similar (start-file-process is just a wrapper
> around make-process in Tramp). I propose to change this.
>

I see.



> But that's not the point of my proposal. If we change make-process such a
> way that it calls the command directly, we will gain a performace
> boost. Philipp Stephani has shown some figures in the discussion
> mentioned above. Maybe he can explain more detailed which kind of
> performance requirements he has in mind, and the use case(s).
>
> However, we will loose features of remote asynchronous processes. At
> least (and not comprehensive), processes started via make-process
>
> - are not checked for passwords or other interactive dialogues
> - do not not support multi-hops anymore
> - cannot be killed via interrupt-process (??? I'm not sure)
> - do not tell the remote tty
> - ...
>
> Processes started via start-file-process won't change their behavior.
>

So basically what you need to know is how `make-process` is used in the
wild. If that helps, magit only uses `start-file-process` and never
`make-process`.

You could try to grep other common libraries to see, on the ones I follow
the only reference to `make-process` I found was in counsel's
`counsel--call` doc string, to mention it uses the same arguments list (but
never actually calls it).

A simple way would be to grep all MELPA libraries, try to ask @purcell
or @tarsius on github (I cc'ed tarsius here).

Kind regards,
Philippe

[-- Attachment #2: Type: text/html, Size: 2694 bytes --]

Re: Remote asynchronous processes

[Michael Albinus]

Philippe Vaucher <philippe.vaucher@gmail.com> writes:

Hi Philippe,

> So basically what you need to know is how `make-process` is used in
> the wild. If that helps, magit only uses `start-file-process` and
> never `make-process`.
>
> You could try to grep other common libraries to see, on the ones I
> follow the only reference to `make-process` I found was in counsel's
> `counsel--call` doc string, to mention it uses the same arguments list
> (but never actually calls it).

I don't believe that make-process is used for remote processes
already. This feature has been added in Emacs 27, see etc/NEWS:

--8<---------------cut here---------------start------------->8---
** 'make-process' now takes a keyword argument ':file-handler'; if
that is non-nil, it will look for a file name handler for the current
buffer's 'default-directory' and invoke that file name handler to make
the process.  That way 'make-process' can start remote processes.
--8<---------------cut here---------------end--------------->8---

If we change the behavior as proposed, we should add a warning about the
prospective change in Emacs 28 here.

> Kind regards,
> Philippe

Best regards, Michael.

Re: Remote asynchronous processes

[Philipp Stephani]

Am Di., 14. Apr. 2020 um 14:35 Uhr schrieb Philippe Vaucher
<philippe.vaucher@gmail.com>:
>>
>> But that's not the point of my proposal. If we change make-process such a
>> way that it calls the command directly, we will gain a performace
>> boost. Philipp Stephani has shown some figures in the discussion
>> mentioned above. Maybe he can explain more detailed which kind of
>> performance requirements he has in mind, and the use case(s).
>>
>> However, we will loose features of remote asynchronous processes. At
>> least (and not comprehensive), processes started via make-process
>>
>> - are not checked for passwords or other interactive dialogues
>> - do not not support multi-hops anymore
>> - cannot be killed via interrupt-process (??? I'm not sure)
>> - do not tell the remote tty
>> - ...
>>
>> Processes started via start-file-process won't change their behavior.
>
>
> So basically what you need to know is how `make-process` is used in the wild. If that helps, magit only uses `start-file-process` and never `make-process`.
>

There are lots of different use cases, and no single correct answer.
For example, when running M-x shell, I probably want SSH prompts and
am willing to accept a brief blocking wait, but not when starting a
Flymake backend process.
I don't think there's a real difference between start-file-process and
make-process, though; make-process is newer and more powerful, but
it's not a conceptually different thing.

Re: Remote asynchronous processes

[Stefan Monnier]

> However, we will loose features of remote asynchronous processes. At
> least (and not comprehensive), processes started via make-process
>
> - are not checked for passwords or other interactive dialogues
> - do not not support multi-hops anymore
> - cannot be killed via interrupt-process (??? I'm not sure)
> - do not tell the remote tty
> - ...

How 'bout a user-config setting?
At least the first two's importance depend on the user's situation or
usage pattern.

I can't judge on the rest: the 3rd is unsure and I don't understand the 4th.


        Stefan

Re: Remote asynchronous processes

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

>> However, we will loose features of remote asynchronous processes. At
>> least (and not comprehensive), processes started via make-process
>>
>> - are not checked for passwords or other interactive dialogues
>> - do not not support multi-hops anymore
>> - cannot be killed via interrupt-process (??? I'm not sure)
>> - do not tell the remote tty
>> - ...
>
> How 'bout a user-config setting?
> At least the first two's importance depend on the user's situation or
> usage pattern.

It's not so simple to decide for a user. I fear myriads of bug reports
towards Tramp, if make-process behaves differently depending on user config.

Experience shows, that there aren't so many people who respond to Tramp
bug reports.

> I can't judge on the rest: the 3rd is unsure and I don't understand the 4th.

The 4th problem means, that process-tty-name cannot return proper
information anymore for that kind of remote processes.

>         Stefan

Best regards, Michael.

Re: Remote asynchronous processes

[Philipp Stephani]

Am Di., 14. Apr. 2020 um 16:48 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:
>
> > However, we will loose features of remote asynchronous processes. At
> > least (and not comprehensive), processes started via make-process
> >
> > - are not checked for passwords or other interactive dialogues
> > - do not not support multi-hops anymore
> > - cannot be killed via interrupt-process (??? I'm not sure)
> > - do not tell the remote tty
> > - ...
>
> How 'bout a user-config setting?


It's not really something that depends on user preferences, but how
processes are used: a noninteractive background process has very
different requirements from an interactive foreground one.

Re: Remote asynchronous processes

[Philipp Stephani]

Am Di., 14. Apr. 2020 um 11:03 Uhr schrieb Michael Albinus
<michael.albinus@gmx.de>:

> Philipp Stephani has shown some figures in the discussion
> mentioned above. Maybe he can explain more detailed which kind of
> performance requirements he has in mind, and the use case(s).

There are lots of different use cases. The broadest category that I
can think of is "processes started in the background to improve the
editing experience." This includes things like M-x compile, the VC
operations, etc. The immediate trigger for my request, however, was
Flymake. Flymake wants to start backends frequently and at arbitrary
times during editing. So starting (not completing!) a backend can't
block for more than a few milliseconds, as that would lead to
unacceptable editing disruptions. This immediately rules out anything
calling call-process, accept-process-output, sleep-for, etc., or
anything else that calls pselect. Remote Flymake backends are really
only possible using the suggested approach. Lots of other use cases
can benefit from it, too.
In summary, the basic requirement is "make-process must reliably
return within a few milliseconds."

>
> However, we will loose features of remote asynchronous processes. At
> least (and not comprehensive), processes started via make-process
>
> - are not checked for passwords or other interactive dialogues
> - do not not support multi-hops anymore
> - cannot be killed via interrupt-process (??? I'm not sure)
> - do not tell the remote tty
> - ...

These are typically irrelevant for the editing use cases I have in
mind. In particular, starting a Flymake backend couldn't ever prompt
the user; any prompt should be converted into a silent failure.

Re: Remote asynchronous processes

[Michael Albinus]

Philipp Stephani <p.stephani2@gmail.com> writes:

Hi Philipp,

>> However, we will loose features of remote asynchronous processes. At
>> least (and not comprehensive), processes started via make-process
>>
>> - are not checked for passwords or other interactive dialogues
>> - do not not support multi-hops anymore
>> - cannot be killed via interrupt-process (??? I'm not sure)
>> - do not tell the remote tty
>> - ...
>
> These are typically irrelevant for the editing use cases I have in
> mind. In particular, starting a Flymake backend couldn't ever prompt
> the user; any prompt should be converted into a silent failure.

There are prompts you aren't aware of. Password requests are handled by
Tramp silently, using auth-sources caches. Don't think only about ssh
where you can use public keys, there's also sudo or whatever.

Best regards, Michael.

Re: Remote asynchronous processes

[Philipp Stephani]

Am Di., 4. Aug. 2020 um 19:35 Uhr schrieb Michael Albinus
<michael.albinus@gmx.de>:
>
> Philipp Stephani <p.stephani2@gmail.com> writes:
>
> Hi Philipp,
>
> >> However, we will loose features of remote asynchronous processes. At
> >> least (and not comprehensive), processes started via make-process
> >>
> >> - are not checked for passwords or other interactive dialogues
> >> - do not not support multi-hops anymore
> >> - cannot be killed via interrupt-process (??? I'm not sure)
> >> - do not tell the remote tty
> >> - ...
> >
> > These are typically irrelevant for the editing use cases I have in
> > mind. In particular, starting a Flymake backend couldn't ever prompt
> > the user; any prompt should be converted into a silent failure.
>
> There are prompts you aren't aware of. Password requests are handled by
> Tramp silently, using auth-sources caches. Don't think only about ssh
> where you can use public keys, there's also sudo or whatever.

Fair enough, though I think these prompts are also not really feasible
for the purpose of fast-starting asynchronous processes, as TRAMP
needs to wait at least a tiny bit for the prompts, and every bit of
synchronous waiting is already too much for this use case.

Re: Remote asynchronous processes

[Michael Albinus]

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

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

Hi,

> On the Tramp ML, there is a discussion about performance of remote
> asynchronous processes. start-file-process / make-process take too much
> time to finish.
>
> One of the reasons is, that Tramp opens first a shell on the remote
> host, performs sanity checks, and runs the command after that. Well, I
> cannot change this in general; the sanity checks have been added due to
> feedback from users.
>
> One idea to change the situation is, to remove all sanity checks from
> make-process. That is, if a user has a default directory
> "/ssh:user@host:/path/to/dir", and if he calls
>
> (make-process
>  :name "test"
>  :buffer (current-buffer)
>  :command '("cmd")
>  :file-handler t))
>
> this is translated directly into
>
> ssh -l user -o ControlMaster=auto -o ControlPath='tramp.%C' \
>   -o ControlPersist=no host "cd /path/to/dir; cmd"
>
> This would improve performance significantly. The drawback is, that
> Tramp does not perform convenience checks, like password handling.

I have played with this idea, and the output is the appended file
tramp-make-process.el. It changes the make-process implementation of
Tramp for all methods defined in tramp-sh.el (like "ssh") and
tramp-adb.el.

This is a proof-of-concept, and shouldn't be used for production. Read
the commentary in the file for limitations.

However, the speed optimization is remarkable. I've tested it with the
following code snippet:

--8<---------------cut here---------------start------------->8---
(let ((tramp-verbose 0)
      (default-directory "/ssh::/"))
  ;; Fill the caches.
  (start-file-process "" nil "true")
  ;; Run benchmark.
  (benchmark-run 10
    (start-file-process "" nil "true")))
--8<---------------cut here---------------end--------------->8---

In the default case, the result is

--8<---------------cut here---------------start------------->8---
(3.623666842 80 1.183512312)
--8<---------------cut here---------------end--------------->8---

If tramp-make-process.el is loaded, the result is

--8<---------------cut here---------------start------------->8---
(0.022762177 0 0.0)
--8<---------------cut here---------------end--------------->8---

Similar results, if I use "/adb::/" as default directory:

--8<---------------cut here---------------start------------->8---
(4.599374061 2 0.03429497299999973)
--8<---------------cut here---------------end--------------->8---

vs

--8<---------------cut here---------------start------------->8---
(0.013003183 0 0.0)
--8<---------------cut here---------------end--------------->8---

Comments?

Best regards, Michael.


[-- Attachment #2: tramp-make-process.el --]
[-- Type: text/plain, Size: 7987 bytes --]

;;; tramp-make-process.el --- Tramp alternative make-process  -*- lexical-binding:t -*-

;; Copyright (C) 2020 Free Software Foundation, Inc.

;; Author: Michael Albinus <michael.albinus@gmx.de>
;; Keywords: comm, processes
;; Package: tramp

;; This file is part of GNU Emacs.

;; GNU Emacs is free software: you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; GNU Emacs is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.

;;; Commentary:

;; An alternative implementation of `make-process' for methods in
;; tramp-sh.el and tramp-adb.el.  It does not use shell commands for
;; execution of the asynchronous command.  Instead, it calls the
;; command directly.  This should result in a performance boost.
;;
;; Limitations of this approach:
;;
;; * It works only for connection methods defined in tramp-sh.el and
;;   tramp-adb.el.
;;
;; * It does not support multi-hop methods.
;;
;; * It does not support user authentication, like password handling.
;;
;; * It does not support a separated error stream.
;;
;; * It cannot be killed via `interrupt-process'.
;;
;; * It does not report the remote terminal name via `process-tty-name'.
;;
;; * It does not set environment variable "INSIDE_EMACS".
;;
;; In order to gain even more performance, it is recommended to set or
;; bind `tramp-verbose' to 0 when running `make-process'.

;;; Code:

;; We use BUFFER also as connection buffer during setup. Because of
;; this, its original contents must be saved, and restored once
;; connection has been setup.
(defun tramp-make-process (&rest args)
  "An alternative `make-process' implementation for Tramp files."
  (when args
    (with-parsed-tramp-file-name (expand-file-name default-directory) nil
      (let ((name (plist-get args :name))
	    (buffer (plist-get args :buffer))
	    (command (plist-get args :command))
	    (coding (plist-get args :coding))
	    (noquery (plist-get args :noquery))
	    (connection-type (plist-get args :connection-type))
	    (filter (plist-get args :filter))
	    (sentinel (plist-get args :sentinel))
	    (stderr (plist-get args :stderr)))
	(unless (stringp name)
	  (signal 'wrong-type-argument (list #'stringp name)))
	(unless (or (null buffer) (bufferp buffer) (stringp buffer))
	  (signal 'wrong-type-argument (list #'stringp buffer)))
	(unless (consp command)
	  (signal 'wrong-type-argument (list #'consp command)))
	(unless (or (null coding)
		    (and (symbolp coding) (memq coding coding-system-list))
		    (and (consp coding)
			 (memq (car coding) coding-system-list)
			 (memq (cdr coding) coding-system-list)))
	  (signal 'wrong-type-argument (list #'symbolp coding)))
	(unless (or (null connection-type) (memq connection-type '(pipe pty)))
	  (signal 'wrong-type-argument (list #'symbolp connection-type)))
	(unless (or (null filter) (functionp filter))
	  (signal 'wrong-type-argument (list #'functionp filter)))
	(unless (or (null sentinel) (functionp sentinel))
	  (signal 'wrong-type-argument (list #'functionp sentinel)))
	(unless (or (null stderr) (bufferp stderr) (stringp stderr))
	  (signal 'wrong-type-argument (list #'stringp stderr)))
	(when (and (stringp stderr) (tramp-tramp-file-p stderr)
		   (not (tramp-equal-remote default-directory stderr)))
	  (signal 'file-error (list "Wrong stderr" stderr)))

	(let* ((buffer
		(if buffer
		    (get-buffer-create buffer)
		  ;; BUFFER can be nil.  We use a temporary buffer.
		  (generate-new-buffer tramp-temp-buffer-name)))
	       (command (append `("cd" ,localname "&&")
				(mapcar #'tramp-shell-quote-argument command)))
	       (bmp (and (buffer-live-p buffer) (buffer-modified-p buffer)))
	       (name1 name)
	       (i 0)
	       ;; We do not want to raise an error when `make-process'
	       ;; has been started several times in `eshell' and
	       ;; friends.
	       tramp-current-connection
	       p)

	  (while (get-process name1)
	    ;; NAME must be unique as process name.
	    (setq i (1+ i)
		  name1 (format "%s<%d>" name i)))
	  (setq name name1)
	  ;; Set the new process properties.
	  (tramp-set-connection-property v "process-name" name)
	  (tramp-set-connection-property v "process-buffer" buffer)

	  (with-current-buffer (tramp-get-connection-buffer v)
	    (unwind-protect
		(let* ((login-program
			(or (tramp-get-method-parameter v 'tramp-login-program)
			    "adb"))
		       (login-args
			(or (tramp-get-method-parameter v 'tramp-login-args)
			    '(("shell"))))
		       (async-args
			(tramp-get-method-parameter v 'tramp-async-args))
		       ;; We don't create the temporary file.  In
		       ;; fact, it is just a prefix for the
		       ;; ControlPath option of ssh; the real
		       ;; temporary file has another name, and it is
		       ;; created and protected by ssh.  It is also
		       ;; removed by ssh when the connection is
		       ;; closed.  The temporary file name is cached
		       ;; in the main connection process, therefore
		       ;; we cannot use `tramp-get-connection-process'.
		       (tmpfile
			(with-tramp-connection-property
			    (tramp-get-process v) "temp-file"
			  (make-temp-name
			   (expand-file-name
			    tramp-temp-name-prefix
			    (tramp-compat-temporary-file-directory)))))
		       (options (tramp-ssh-controlmaster-options v))
		       spec)

		  ;; Replace `login-args' place holders.
		  (setq
		   spec (format-spec-make ?t tmpfile)
		   options (format-spec options spec)
		   spec (format-spec-make
			 ?h (or host "") ?u (or user "") ?p (or port "")
			 ?c options ?l "")
		   ;; Add arguments for asynchronous processes.
		   login-args (append async-args login-args)
		   ;; Expand format spec.
		   login-args
		   (tramp-compat-flatten-tree
		    (mapcar
		     (lambda (x)
		       (setq x (mapcar (lambda (y) (format-spec y spec)) x))
		       (unless (member "" x) x))
		     login-args))
		   ;; Split ControlMaster options.
		   login-args
		   (tramp-compat-flatten-tree
		    (mapcar (lambda (x) (split-string x " ")) login-args))
		   p (apply
		      #'start-process
		      name buffer login-program (append login-args command)))

		  (tramp-message v 6 "%s" (string-join (process-command p) " "))
		  ;; Set sentinel and filter.
		  (when sentinel
		    (set-process-sentinel p sentinel))
		  (when filter
		    (set-process-filter p filter))
		  ;; Set query flag and process marker for this
		  ;; process.  We ignore errors, because the
		  ;; process could have finished already.
		  (ignore-errors
		    (set-process-query-on-exit-flag p (null noquery))
		    (set-marker (process-mark p) (point)))
		  ;; We must flush them here already; otherwise
		  ;; `rename-file', `delete-file' or
		  ;; `insert-file-contents' will fail.
		  (tramp-flush-connection-property v "process-name")
		  (tramp-flush-connection-property v "process-buffer")
		  ;; Return process.
		  p)

	      ;; Save exit.
	      (if (string-match-p tramp-temp-buffer-name (buffer-name))
		  (ignore-errors
		    (set-process-buffer p nil)
		    (kill-buffer (current-buffer)))
		(set-buffer-modified-p bmp))
	      (tramp-flush-connection-property v "process-name")
	      (tramp-flush-connection-property v "process-buffer"))))))))

(with-eval-after-load 'tramp-adb
  (defalias 'tramp-adb-handle-make-process #'tramp-make-process))

(with-eval-after-load 'tramp-sh
  (defalias 'tramp-sh-handle-make-process #'tramp-make-process))

(add-hook 'tramp-unload-hook
	  (lambda ()
	    (unload-feature 'tramp-make-process 'force)))

(provide 'tramp-make-process)

;;; tramp-make-process.el ends here

Re: Remote asynchronous processes

[Philipp Stephani]

Am Mo., 13. Apr. 2020 um 12:19 Uhr schrieb Michael Albinus
<michael.albinus@gmx.de>:
>
> Hi,
>
> On the Tramp ML, there is a discussion about performance of remote
> asynchronous processes. start-file-process / make-process take too much
> time to finish.
>
> One of the reasons is, that Tramp opens first a shell on the remote
> host, performs sanity checks, and runs the command after that. Well, I
> cannot change this in general; the sanity checks have been added due to
> feedback from users.
>
> One idea to change the situation is, to remove all sanity checks from
> make-process. That is, if a user has a default directory
> "/ssh:user@host:/path/to/dir", and if he calls
>
> --8<---------------cut here---------------start------------->8---
> (make-process
>  :name "test"
>  :buffer (current-buffer)
>  :command '("cmd")
>  :file-handler t))
> --8<---------------cut here---------------end--------------->8---
>
> this is translated directly into
>
> --8<---------------cut here---------------start------------->8---
> ssh -l user -o ControlMaster=auto -o ControlPath='tramp.%C' \
>   -o ControlPersist=no host "cd /path/to/dir; cmd"
> --8<---------------cut here---------------end--------------->8---
>
> This would improve performance significantly. The drawback is, that
> Tramp does not perform convenience checks, like password handling.
>
> start-file-process would not be changed, and it behaves like before.
>
> Comments?

I think the approach is great, thanks for picking this up. A few minor comments:
- I'd change start-file-process in lockstep with make-process. Since
start-process is nowadays just a wrapper around make-process, it would
be somewhat confusing if start-file-process behaved significantly
differently.
- I'd probably leave out the ControlMaster options entirely and rely
on a working SSH config with a control master already started. That
way the control master can be reused across arbitrary processes on the
client machine, not just Emacs.

Re: Remote asynchronous processes

[Michael Albinus]

Philipp Stephani <p.stephani2@gmail.com> writes:

Hi Philipp,

> - I'd change start-file-process in lockstep with make-process. Since
> start-process is nowadays just a wrapper around make-process, it would
> be somewhat confusing if start-file-process behaved significantly
> differently.

Finally, Tramp's `start-file-process' is also just a wrapper around
`make-process', see `tramp-handle-start-file-process'.

> - I'd probably leave out the ControlMaster options entirely and rely
> on a working SSH config with a control master already started. That
> way the control master can be reused across arbitrary processes on the
> client machine, not just Emacs.

You can set `tramp-use-ssh-controlmaster-options' to nil for that
effect. Will mention it in the Tramp manual.

Best regards, Michael.

Re: Remote asynchronous processes

[Philipp Stephani]

Am Di., 4. Aug. 2020 um 19:31 Uhr schrieb Michael Albinus
<michael.albinus@gmx.de>:
>
> Philipp Stephani <p.stephani2@gmail.com> writes:
>
> Hi Philipp,
>
> > - I'd change start-file-process in lockstep with make-process. Since
> > start-process is nowadays just a wrapper around make-process, it would
> > be somewhat confusing if start-file-process behaved significantly
> > differently.
>
> Finally, Tramp's `start-file-process' is also just a wrapper around
> `make-process', see `tramp-handle-start-file-process'.

OK.

>
> > - I'd probably leave out the ControlMaster options entirely and rely
> > on a working SSH config with a control master already started. That
> > way the control master can be reused across arbitrary processes on the
> > client machine, not just Emacs.
>
> You can set `tramp-use-ssh-controlmaster-options' to nil for that
> effect. Will mention it in the Tramp manual.

Thanks, I'll set that option and see whether it works.