[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#61350: Eglot over Tramp freezes with large project
From: |
Michael Albinus |
Subject: |
bug#61350: Eglot over Tramp freezes with large project |
Date: |
Sat, 11 Mar 2023 13:44:33 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
João Távora <joaotavora@gmail.com> writes:
Hi João,
> I think the patch goes in the right direction. The force resetting of
> 'timeout' to 0, which you highlighted earlier, is odd.
Yes. But I've tried first w/o this setting, and the recipe from Thomas
still didn't work.
> + (dolist (p (delq proc (process-list)))
> + (when (tramp-file-name-equal-p
> + (process-get proc 'vector) (process-get p 'vector))
> + (setq timeout (or timeout 0))
> + (accept-process-output p 0 nil t)))
>
> That's because callers of 'tramp-accept-process-output' who call it with
> TIMEOUT=nil will now see their expectations violated depending on
> conditions they do not control. You fully control these callers, so you
> may know better, but I think this patch makes more sense, because it
> keeps the contract.
In fact, I'm even not sure that a nil timeout is still needed. This is
something from the very beginning of Tramp. Maybe I shall review the
code whether we still need a nil timeout. Because its initial idea, wait
in accept-process-output until the remote shell prompt appears, is no
longer needed, because we call tramp-accept-process-output (and
therefore accept-process-output) in a loop. This is something to be
changed later on.
A short scan over the Tramp sources shows, that there are 12 calls of
tramp-accept-process-output with a TIMEOUT of 0, and only two other
calls with a nil TIMEOUT. And these two calls are already in while loop,
so I guess it would be also OK to give them a default TIMEOUT of 0.
I will try it, but this needs lot of regression tests (I have 100+
different configurations to run tramp-tests.el).
> + (dolist (p (delq proc (process-list)))
> + (when (tramp-file-name-equal-p
> + (process-get proc 'vector) (process-get p 'vector))
> + (while (accept-process-output p 0 nil t))))
> (with-current-buffer (process-buffer proc)
> (let ((inhibit-read-only t)
> last-coding-system-used
>
> This is even more "conservative" than your patch, because it deviates
> less from the current behaviour. I've tested it (first removing the
> eglot.el workaround) with my reproduction recipe to a 100% success rate
> in 6-7 experiments (whereas without it it is 0%).
Hmm, in my experience there were still one or two blocking tests wit
Thomas' recipe, IIRC. This is because the blocking situation, data is
going through the only ssh socket for both tramp-accept-process-output
and jsonrpc, is not unblocked then.
So I would prefer to keep this (setq timeout (or timeout 0)) for
now. And I will reviw the code, whether tramp-accept-process-output
still needs a TIMEOUT ARGUMENT as sketched above.
>> I plan to install the patch next days on master, unless you oppose.
>
> I don't, but consider my alternative.
I consider it by removing the TIMEOUT argument, if possible.
> João
>
> PS: And I also think that Eglot over Tramp does seem to have become
> faster with ControlMaster, at least in a subjective evaluation.
Of course. That is the reason behind using it.
Best regards, Michael.
- bug#61350: Eglot over Tramp freezes with large project, (continued)
- bug#61350: Eglot over Tramp freezes with large project, Thomas Koch, 2023/03/07
- bug#61350: Eglot over Tramp freezes with large project, João Távora, 2023/03/07
- bug#61350: Eglot over Tramp freezes with large project, Michael Albinus, 2023/03/07
- bug#61350: Eglot over Tramp freezes with large project, João Távora, 2023/03/07
- bug#61350: Eglot over Tramp freezes with large project, Michael Albinus, 2023/03/07
- bug#61350: Eglot over Tramp freezes with large project, Michael Albinus, 2023/03/11
- bug#61350: Eglot over Tramp freezes with large project, Thomas Koch, 2023/03/11
- bug#61350: Eglot over Tramp freezes with large project, João Távora, 2023/03/11
- bug#61350: Eglot over Tramp freezes with large project, Michael Albinus, 2023/03/11
- bug#61350: Eglot over Tramp freezes with large project, João Távora, 2023/03/11
- bug#61350: Eglot over Tramp freezes with large project,
Michael Albinus <=
- bug#61350: Eglot over Tramp freezes with large project, João Távora, 2023/03/11
- bug#61350: Eglot over Tramp freezes with large project, Michael Albinus, 2023/03/11
- bug#61350: Eglot over Tramp freezes with large project, João Távora, 2023/03/11
- bug#61350: Eglot over Tramp freezes with large project, Michael Albinus, 2023/03/12
- bug#61350: Eglot over Tramp freezes with large project, Michael Albinus, 2023/03/14
- bug#61350: Eglot over Tramp freezes with large project, Michael Albinus, 2023/03/14
- bug#61350: Eglot over Tramp freezes with large project, João Távora, 2023/03/14
- bug#61350: Eglot over Tramp freezes with large project, Michael Albinus, 2023/03/14
- bug#61350: Eglot over Tramp freezes with large project, Stefan Monnier, 2023/03/15
- bug#61350: Eglot over Tramp freezes with large project, João Távora, 2023/03/15