process-file and Tramp's file cache (was: bug#1741: 23.0.60; vc fail to register file while logged as /su:: with tramp)

process-file and Tramp's file cache (was: bug#1741: 23.0.60; vc fail to register file while logged as /su:: with tramp)

[Michael Albinus]

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

> BTW, to improve the behavior in case of things like "git status", it
> might be worthwhile to add some way to indicate that a particular use of
> process-file is (supposed to be) free of side-effects.

I would like to introduce a new variable `process-file-modification-list'.
It's default value shall be `t', which means that any file on the remote
host could have been modified by the corresponding `process-file' call.
`nil' means that no file is modified during `process-file'. A list of
strings (remote file names) are indication, that exactly those files are
modified, when `process-file' runs.

This would allow Tramp to optimize its file cache. Currently, it assumes
after every `process-file' call, that all cached values are invalid.
This results in unnecessary check operations on the remote side.

A typical use case in `vc-registered' would look like this:

  (let (handler process-file-modification-list)
    (cond ...

Opinions?

>         Stefan

Best regards, Michael.

Re: process-file and Tramp's file cache

[Stefan Monnier]

>> BTW, to improve the behavior in case of things like "git status", it
>> might be worthwhile to add some way to indicate that a particular use of
>> process-file is (supposed to be) free of side-effects.

> I would like to introduce a new variable `process-file-modification-list'.
> It's default value shall be `t', which means that any file on the remote
> host could have been modified by the corresponding `process-file' call.
> `nil' means that no file is modified during `process-file'.  A list of
> strings (remote file names) are indication, that exactly those files are
> modified, when `process-file' runs.

> This would allow Tramp to optimize its file cache. Currently, it assumes
> after every `process-file' call, that all cached values are invalid.
> This results in unnecessary check operations on the remote side.

> A typical use case in `vc-registered' would look like this:

>   (let (handler process-file-modification-list)
>     (cond ...

> Opinions?

How 'bout changing Tramp's process-file to compare the command (and
args) against a list of "known pure commands"?  This would keep it all
inside Tramp, which I think I like better (tho I also agree that this
information is not fundamentally specific to Tramp, although currently
Tramp is the only package able to make use of it).


        Stefan

Re: process-file and Tramp's file cache

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> How 'bout changing Tramp's process-file to compare the command (and
> args) against a list of "known pure commands"?  This would keep it all
> inside Tramp, which I think I like better (tho I also agree that this
> information is not fundamentally specific to Tramp, although currently
> Tramp is the only package able to make use of it).

That is also Julian's proposal. I dislike it, because there might be
many packages Tramp has never heard about. And even for the packages
contained in core Emacs, it will be an endless story: think about all the
vc-BACKEND calls of list whatever.

The charme is also, that packages can precisely say, which file has been
changed. Tramp could only guess `t' or `nil'. Parsing `args' of
`process-file' might also not a good idea, I believe. You cannot not
know for sure, what is a filename, and what is something else.

>         Stefan

Best regards, Michael.

Re: process-file and Tramp's file cache

[Stefan Monnier]

>> How 'bout changing Tramp's process-file to compare the command (and
>> args) against a list of "known pure commands"?  This would keep it all
>> inside Tramp, which I think I like better (tho I also agree that this
>> information is not fundamentally specific to Tramp, although currently
>> Tramp is the only package able to make use of it).

> That is also Julian's proposal. I dislike it, because there might be
> many packages Tramp has never heard about. And even for the packages
> contained in core Emacs, it will be an endless story: think about all the
> vc-BACKEND calls of list whatever.

OTOH, this list could be kept in a variable, so external packages could
easily add to it.

> The charme is also, that packages can precisely say, which file has been
> changed.

Clearly, each option has its advantages.  I'd mostly like to avoid
overengineering it, so for now I think a boolean is plenty (after all,
we currently live fairly happily with a worst-case behavior).

> Parsing `args' of `process-file' might also not a good idea,
> I believe.

It could prove problematic, indeed (I'm thinking mostly of having to
search for "status" in the argument of "cvs", having to skip the options
that might come before).

Let's go with your suggestion, but limited to a boolean for now (so the
name shouldn't end with "-list").


        Stefan

Re: process-file and Tramp's file cache

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Let's go with your suggestion, but limited to a boolean for now (so the
> name shouldn't end with "-list").

Shouldn't it `process-file-modification-p' then?

>         Stefan

Best regards, Michael.

Re: process-file and Tramp's file cache

[Stefan Monnier]

>> Let's go with your suggestion, but limited to a boolean for now (so the
>> name shouldn't end with "-list").

> Shouldn't it `process-file-modification-p' then?

No: the "-p" is for functions returning booleans, not for variables.
Note that "-flag" is not a good idea either, because it's incompatible
with any future extension to additional values (e.g. a list of files).
I suggest `process-file-side-effects'.


        Stefan

Re: process-file and Tramp's file cache

[Michael Albinus]

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

> No: the "-p" is for functions returning booleans, not for variables.
> Note that "-flag" is not a good idea either, because it's incompatible
> with any future extension to additional values (e.g. a list of files).
> I suggest `process-file-side-effects'.

OK, we'll do so. When Julian's papers have arrived FSF, I'll commit it
to the trunk (he has written most of the optimizations we're testing
these days).

>         Stefan

Best regards, Michael.