bug#56342: TRAMP (sh) issues way too many commands, thus being very slow over high-ping networks

[Paul Pogonyshev <pogonyshev@gmail.com>]

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

> I've worked last weeks almost full time on this.

Thank you!

> Combining stat and readlink into one remote command have even slowed
> down the machinery. Likely, because it isn't needed always, but it
> always consumes more time to compute both on the remote side, and to
> read all the results.

Well, that's hard to believe. I mean, it is understandable that `readlink'
(whatever Elisp call results in that shell command) is not often needed,
but how long can that take on a modern computer, 1 ms? It might be that we
evaluate performance changes differently. I care more about worst cases
(since they affect me directly, yeah) and not so much if Tramp becomes 1 ms
slower "per user-visible operation" on average.

> Perhaps you
give it a try to see, whether your use cases behave better.

I tested it a bit now. Initially mentioned usecase (`insert-file-contents',
but only a file part, not the whole file) appears to be considerably faster
after the changes (hard to say, feels like 30-50% faster). The connection
log says that number of commands is 10 now, with the originally mentioned
list heavily changed. However, there must still be space for improvement,
e.g. `M-x occur tramp-send RET' suggests there are outright duplicated
commands now:

    18:03:41.617738 tramp-send-command (6) # \readlink
--canonicalize-missing /tmp/tramp.H3AZYb 2>/dev/null; echo
tramp_exit_status $?
    18:03:41.650294 tramp-send-command (6) # tramp_stat_file_attributes
/tmp/tramp.H3AZYb 2>/dev/null; echo tramp_exit_status $?
    18:03:41.699627 tramp-send-command (6) # base64 -d -i
>/tmp/tramp.H3AZYb <<'9190079abe64738d52b0f040fd94461c' 2>/dev/null; echo
tramp_exit_status $?
    18:03:41.731669 tramp-send-command (6) # chown 1000:1001
/tmp/tramp.H3AZYb
    18:03:41.760145 tramp-send-command (6) # dd bs=1 skip=8166969 if=[...]
of=/tmp/tramp.H3AZYb
    18:03:41.843785 tramp-send-command (6) # \readlink
--canonicalize-missing /tmp/tramp.H3AZYb 2>/dev/null; echo
tramp_exit_status $?
    18:03:41.872072 tramp-send-command (6) # tramp_stat_file_attributes
/tmp/tramp.H3AZYb 2>/dev/null; echo tramp_exit_status $?
    18:03:41.902388 tramp-send-command (6) # (env GZIP= gzip
</tmp/tramp.H3AZYb | base64) 2>/dev/null; echo tramp_exit_status $?
    18:03:41.952283 tramp-send-command (6) # rm -f /tmp/tramp.H3AZYb
2>/dev/null; echo tramp_exit_status $?

On the other hand, refreshing a remote-tracking Magit (Emacs Git
interface) buffer doesn't feel any faster than before. Logs also suggest
that number of commands that tramp send stays as before (43). Some commands
are certainly excessive, e.g. it repeats `test -d [...] 2>/dev/null; echo
tramp_exit_status $?' 4 times for the same directory (intermixed with other
commands), suggesting that Tramp doesn't cache the result. Three times it
issues `test -r ...' for the same directory, the source checkout root. Most
other commands cannot be generically skipped by Tramp though, this would
rather depend on Magit.

Paul

On Tue, 26 Jul 2022 at 16:18, Michael Albinus <michael.albinus@gmx.de>
wrote:

> Paul Pogonyshev <pogonyshev@gmail.com> writes:
>
> > Hello,
>
> Hi Paul,
>
> > any news on this?
>
> I've worked last weeks almost full time on this. With limited success.
>
> Combining stat and readlink into one remote command have even slowed
> down the machinery. Likely, because it isn't needed always, but it
> always consumes more time to compute both on the remote side, and to
> read all the results.
>
> Moving the check of the existence of a file to the end, and let the
> commands fail in case the file doesn't exist, isn't possible. Often,
> Tramp needs to know in advance the size of a given file, for example to
> decide whether to encode/decode the file or to invoke scp, or to know
> whether the file shall be compressed prior transfer. Checking the file
> size is much the same as checking the file existence.
>
> Avoiding the "are you awake" command is still on my todo list, but it
> isn't trivial. There is a complex machinery to be invoked in case the
> command fails, in order to reestablish the connection in the
> background. It still might be possible to change this, but it isn't
> trivial.
>
> Bu there are also some results. With commit
> 9ed5c39aad09571314097be91cb28e7504614421 I've refactored major Tramp
> parts in order to have a better chance to apply performance
> changes. This includes already some changes to combine several commands
> (avoiding superfluous roundtrips) and alike. There's also commit
> dfa16cadc18930fad76fa6113750eaa27d367e72 fixing a regression which was
> introduced with the former patch.
>
> In my test environment, performance improvement of these changes is not
> overwhelming. But I sit in a LAN, with good connections. Perhaps you
> give it a try to see, whether your use cases behave better.
>
> I'll continue to work on this, but don't expect wonders.
>
> > Paul
>
> Best regards, Michael.
>

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