Recent diff-mode changes feel slow with TRAMP

Recent diff-mode changes feel slow with TRAMP

[Dima Kogan]

Hi.

There were some diff-mode updates recently that (I think) are making
emacs really slow under some conditions. Scenario:

- I'm working on a remote project via TRAMP and SSH
- There're a number of local changes
- I hit C-x v D    (vc-root-diff)

This runs something like "git diff" remotely. That command constructs a
patch. Previously emacs would simply load this patch into a buffer,
fontify it, and call it done. I THINK what's happening now is that in
addition, each file mentioned in the patch is loaded by emacs. I don't
know what emacs is doing with those files, but file loading is
potentially very slow with TRAMP, and we shouldn't be doing this by
default.

I haven't dug deeply here at all. I just see that after hitting C-x v D
I now have to sit here for a few minutes while emacs figures out what
it's doing. Is it loading all the files? Why?

I'm not subscribed to the list right now, so please Cc me in replies.

Thanks!
dima

Re: Recent diff-mode changes feel slow with TRAMP

[Dima Kogan]

Playing with it a bit more, it feels really slow just moving around,
deleting hunks, reversing the patch, etc, etc. One clear bug I just
noticed is that turning off diff-auto-refine-mode no longer has an
effect: diff-auto-refine-mode still evaluates to nil, but the hunks are
being automatically refined.

Thanks.

Re: Recent diff-mode changes feel slow with TRAMP

[Charles A. Roelli]

> From: Dima Kogan <lists@dima.secretsauce.net>
> Date: Fri, 11 Jan 2019 22:57:19 -0800
> 
> Playing with it a bit more, it feels really slow just moving around,
> deleting hunks, reversing the patch, etc, etc. One clear bug I just
> noticed is that turning off diff-auto-refine-mode no longer has an
> effect: diff-auto-refine-mode still evaluates to nil, but the hunks are
> being automatically refined.

There is some discussion on this in Bug#32991.

Re: Recent diff-mode changes feel slow with TRAMP

[Dima Kogan]

Charles A. Roelli <charles@aurox.ch> writes:

> There is some discussion on this in Bug#32991.

Thanks. I subscribed to the bug. Is there an easy way to turn off hunk
refinement with the latest code? If nothing else, I'd like to be able to
compare the old and new behaviors side by side.

Re: Recent diff-mode changes feel slow with TRAMP

[Charles A. Roelli]

> From: Dima Kogan <dima@secretsauce.net>
> Date: Fri, 11 Jan 2019 20:16:04 -0800
>
> 				I THINK what's happening now is that in
> addition, each file mentioned in the patch is loaded by emacs. I don't
> know what emacs is doing with those files, but file loading is
> potentially very slow with TRAMP, and we shouldn't be doing this by
> default.

Did you try turning off "diff-font-lock-syntax"?  This new feature
will find each file in the diff to provide correct font-locking.
Maybe it should be disabled by default in remote buffers.

Re: Recent diff-mode changes feel slow with TRAMP

[Charles A. Roelli]

> From: Dima Kogan <lists@dima.secretsauce.net>
> Date: Sat, 12 Jan 2019 11:48:37 -0800
> 
> Thanks. I subscribed to the bug. Is there an easy way to turn off hunk
> refinement with the latest code?

This should soon be possible again (see the patch posted in the bug report).

Re: Recent diff-mode changes feel slow with TRAMP

[Juri Linkov]

> Maybe it should be disabled by default in remote buffers.

When a remote host is fast, it's a big advantage that TRAMP supports
this feature without any efforts.  But when a remote host is slow,
it should be disabled indeed.

I can't confirm what effect is has on slow connections because all my
remote connections are so fast that I don't notice any difference
between remote and local.

Re: Recent diff-mode changes feel slow with TRAMP

[Stefan Monnier]

>> Maybe it should be disabled by default in remote buffers.
>
> When a remote host is fast, it's a big advantage that TRAMP supports
> this feature without any efforts.  But when a remote host is slow,
> it should be disabled indeed.
>
> I can't confirm what effect is has on slow connections because all my
> remote connections are so fast that I don't notice any difference
> between remote and local.

Do you think it would be possible to provide a fast/local option for
diff-font-lock-syntax which doesn't fetch the base file but still
performs some syntax-highlighting (e.g. just take the hunk's text into
a temp buffer, put it in the appropriate major mode and
call font-lock-ensure; maybe bailing out if the end of the hunk is
taken to be within a string, in which case maybe the whole highlighting
was done wrong because maybe it was the beginning of the hunk which was
inside a string)?

It could also be used in those cases where you're looking at a diff "out
of context" (so the base file can't be fetched)?


        Stefan

Re: Recent diff-mode changes feel slow with TRAMP

[Dima Kogan]

Charles A. Roelli <charles@aurox.ch> writes:

> Did you try turning off "diff-font-lock-syntax"? This new feature will
> find each file in the diff to provide correct font-locking. Maybe it
> should be disabled by default in remote buffers.

I did not know this setting exists. It's likely the thing that's
breaking me (will test that out later)

Re: Recent diff-mode changes feel slow with TRAMP

[Dima Kogan]

Charles A. Roelli <charles@aurox.ch> writes:

> Did you try turning off "diff-font-lock-syntax"? This new feature will
> find each file in the diff to provide correct font-locking. Maybe it
> should be disabled by default in remote buffers.

Gah. I sent the email before I was done.

OK. Yes. Setting diff-font-lock-syntax to nil makes it fast again.
Thanks a lot for pointing me to that.

The docs for diff-font-lock-syntax say that 'hunk-only is the fastest
setting here, but that's not true, right? Isn't nil the fastest one?

For my use cases, I don't think I want this enabled at all, TRAMP or
not. Combined with refinement, this makes the diffs more difficult to
interpret, at least to me. And even without TRAMP, this makes it slower.
Do we really want both refinement and this font-locking enabled by
default?

There's also a visual issue with auto-refinement. If I load a patch file
with 'emacs -Q -rv' (no user settings, but reverse video), it looks like
this:

  http://notes.secretsauce.net/emacsdiff.png

Note that the bright green refined face makes the leading + invisible.
There's an identical problem with the bright red making the leading -
invisible.

Thanks for the work. I'll try to help out more when I get more time.

dima

Re: Recent diff-mode changes feel slow with TRAMP

[Juri Linkov]

>> When a remote host is fast, it's a big advantage that TRAMP supports
>> this feature without any efforts.  But when a remote host is slow,
>> it should be disabled indeed.
>>
>> I can't confirm what effect is has on slow connections because all my
>> remote connections are so fast that I don't notice any difference
>> between remote and local.
>
> Do you think it would be possible to provide a fast/local option for
> diff-font-lock-syntax which doesn't fetch the base file but still
> performs some syntax-highlighting (e.g. just take the hunk's text into
> a temp buffer, put it in the appropriate major mode and
> call font-lock-ensure;

There is already an option `hunk-only' but I don't know how
TRAMP could detect a slow connection and set diff-font-lock-syntax
to `hunk-only' in this case.

>                        maybe bailing out if the end of the hunk is
> taken to be within a string, in which case maybe the whole highlighting
> was done wrong because maybe it was the beginning of the hunk which was
> inside a string)?

Also I don't know how to detect a partial string because if there are
two unterminated strings (i.e. two quotes) in the same hunk
they might be interpreted as one "inside-out" string.

Re: Recent diff-mode changes feel slow with TRAMP

[Juri Linkov]

> OK. Yes. Setting diff-font-lock-syntax to nil makes it fast again.
> Thanks a lot for pointing me to that.
>
> The docs for diff-font-lock-syntax say that 'hunk-only is the fastest
> setting here, but that's not true, right? Isn't nil the fastest one?

`hunk-only' is the fastest non-nil option.

> For my use cases, I don't think I want this enabled at all, TRAMP or
> not. Combined with refinement, this makes the diffs more difficult to
> interpret, at least to me. And even without TRAMP, this makes it slower.
> Do we really want both refinement and this font-locking enabled by
> default?

Modern version control systems like GitHub and GitLab enable both by default.

> There's also a visual issue with auto-refinement. If I load a patch file
> with 'emacs -Q -rv' (no user settings, but reverse video), it looks like
> this:
>
>   http://notes.secretsauce.net/emacsdiff.png
>
> Note that the bright green refined face makes the leading + invisible.
> There's an identical problem with the bright red making the leading -
> invisible.

It's easy to improve auto-refinement to not put highlighting on diff
indicators like it's already implemented in line-based diff-font-lock-syntax.

BTW, there is another issue that could be improved in auto-refinement:
when changes are only in whitespace it highlights nothing because it's
word-based.  I think in this case it could fall back to the default mode
that doesn't ignore differences in whitespace, thus highlighting whitespace.

Re: Recent diff-mode changes feel slow with TRAMP

[Stefan Monnier]

> Also I don't know how to detect a partial string

Reliably you can't.

> because if there are two unterminated strings (i.e. two quotes) in the
> same hunk they might be interpreted as one "inside-out" string.

I was thinking of a heuristic to try and catch common cases.


        Stefan