From: Theodor Thornhill <theo@thornhill.no>
To: "João Távora" <joaotavora@gmail.com>
Cc: emacs-devel@gnu.org
Subject: Re: master 08c80c45dde: Don't use file-truepath in Eglot (bug#70036)
Date: Thu, 18 Apr 2024 08:02:42 +0200 [thread overview]
Message-ID: <87edb3rywt.fsf@thornhill.no> (raw)
In-Reply-To: <CALDnm52b9ipvv-CB-g5Ga71b3ubQf7jAGiLc505yAJYn7mTnKg@mail.gmail.com>
João Távora <joaotavora@gmail.com> writes:
> On Wed, Apr 17, 2024 at 7:41 PM Theodor Thornhill <theo@thornhill.no> wrote:
> have been looking for an Eglot maintainer but everyone I contacted
>
>> If you want I can do this - so that you don't have to. I've been
>> considering it for some time after I responded to your call for
>> maintainers.
>
> OK, I've added you to the GitHub repo as a collaborator. Let me know
> off-list what your intentions are (to phase it out, keep it and take
> it over, or whatever) and let's see how it goes.
>
Thanks, will do.
>> > The change seems well structured, well coded, and well described in the
>> > commit message, so I could understand it easily. Do keep that up.
>> >
>> > But of course going from file-buffer-visiting to something else
>> > whose underlying implementation is faster but doesn't chase
>> > symlinks is probably going to have some kind of functional implication
>> > right? I wonder if (or rather "I hope that") you guys considered it.
>>
>> I did - and afaict it has no such implications. It
>
> I've identified at least one implication. A rather obvious one, given
> that you've effectively removed symlink-awareness from Eglot.
>
> When working in a project with symlinks and visiting such symlinks,
> the LSP server is now informed (via LSP 'didOpen') of the symlinked
> file as if it was its own file, whereas before your changes Eglot
> sent over the true name of the file. You can see this easily
> from M-x eglot-events-buffer.
>
> What does this mean in practice? Many things potentially, but
> at least this one: I've run this experiment: In a new dir, create
> a lib.h file with this silly content:
>
> int foo();
>
> Then create a main.cpp like this:
>
> #include "lib.h"
> int main() { return foo(); }
>
> Then create a mainlink.cpp symbolic link to main.cpp
>
> Then M-x eglot (launches clangd server to manage the new directory
> as a project).
>
> Let's say that during the Eglot session I visit both main.cpp
> and mainlink.cpp in different buffers (either because I don't visit
> them at the same time or because find-file-existing-other-name is
> nil). Then I press M-? on lib.h's foo() to tell me who references it.
>
> Before you change, Eglot will -- correctly -- tell me there is a single
> user of lib.h's foo() function in my project.
>
> After your change, it tells me there are two users. This is wrong,
> there is only one.
>
> It could be that some servers with direct access to the file system
> can deduplicate the information and add back the symlink smarts.
>
> But clangd doesn't do this, and in general servers _can't_ do
> this because LSP models a virtual file system.
From what I can see in other servers this is recognized as a server
problem, and not a client problem [1], [2], [3].
>
> And for symlinks to large enough files, I'd be surprised if this
> doesn't slow down the performance of the server because it has to
> analyse what it is told is a completely new file.
>
> So this seems like a pretty big flaw to me after just minimal
> surface scratching. Please reinstate the previous code.
>
I can absolutely do this, but I'd rather we consider it a bit more
before I do. On my system there is a real, tangible performance increase
with this change, and the symlink one seems more like a server edge case
issue. Maybe we could check for file-symlink-p rather than runthe whole
file-truepath?
> We can resume discussion of your performance problems in the
> bug report. I've seen the profile reports and file-truename is
> not thaat big of a problem. But nevertheless I think I can
> help you think of other places to cache things. Caching
> eglot--TextDocumentIdentifier per-buffer seems like a much
> easier way to solve your problems.
I disagree - it really makes a huge difference in felt latency.
Theo
[1]: https://github.com/clangd/clangd/issues/124
[2]: https://github.com/microsoft/python-language-server/issues/181
[3]: https://github.com/microsoft/vscode-python/issues/2613
next prev parent reply other threads:[~2024-04-18 6:02 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <171215083924.12380.5369373861551158668@vcs2.savannah.gnu.org>
[not found] ` <20240403132719.A18EFC12C28@vcs2.savannah.gnu.org>
2024-04-17 15:35 ` master 08c80c45dde: Don't use file-truepath in Eglot (bug#70036) João Távora
2024-04-17 18:41 ` Theodor Thornhill
2024-04-18 0:24 ` João Távora
2024-04-18 5:49 ` Eli Zaretskii
2024-04-18 6:16 ` Theodor Thornhill
2024-04-18 8:34 ` João Távora
2024-04-18 9:57 ` Theodor Thornhill
2024-04-18 15:00 ` João Távora
2024-04-18 15:44 ` Eli Zaretskii
2024-04-18 16:22 ` João Távora
2024-04-18 16:30 ` Theodor Thornhill
2024-04-18 17:12 ` João Távora
2024-04-18 16:35 ` Eli Zaretskii
2024-04-18 6:02 ` Theodor Thornhill [this message]
2024-04-18 14:49 ` João Távora
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87edb3rywt.fsf@thornhill.no \
--to=theo@thornhill.no \
--cc=emacs-devel@gnu.org \
--cc=joaotavora@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.