bug#70036: 30.0.50; Move file-truename to the C level

[Theodor Thornhill]

Felician Nemeth <felician.nemeth@gmail.com> writes:

> Theodor Thornhill <theo@thornhill.no> writes:
>> Felician Nemeth <felician.nemeth@gmail.com> writes:
>
>>> Theo, can you email me the relevant messages that your server sends
>>> to Emacs?  Does the server send lots of similar diagnostics messages
>>> frequently?
>
>> I'll try to include such a report a little later today. 
>
> Thanks, that would be helpful.
>

It's a little hard scraping the logs for company related data, so I
haven't done this yet, but the profiles should be pretty expanatory if
you do some edits after applying the below patch.

>> [2. text/x-diff; 0001-Don-t-use-file-truepath-in-Eglot-bug-70036.patch]...
>
> I think using find-buffer-visiting instead of get-file-buffer and
> file-truename instead of expand-file-name in Eglot is problematic.
> Let's say we have these files:
>
> /project/a.c
> /project/a.h -> /other/a.h
>
> Eglot will communicate these file names to the LSP server: /project/a.c
> and /other/a.h.  Then the server cannot "associate" a.h with a.c.
> Additionally, a.h will be outside of the LSP workspace.
>
> This indeed confuses clangd a bit: it only takes into account the
> changes of buffer a.h when it is saved.  (Because it assumes
> /project/a.h is not opened in the editor.)

So in other words this is already a bug in eglot? 

>
> ------
>
> Regarding the patch itself, cache invalidation is missing from it.  The
> user might kill a buffer or save it under a different name with
> write-file.  Changing (PATH -> BUFFER) to (PATH -> (BUFFER, FILENAME))
> would probably work.  Eglot should save the current buffer-file-name
> when it inserts a new item into the hash of managed-buffers.  And when
> it retrieves an item, it should verify whether the buffer-file-name is
> the same as the saved file-name.
>
> Can file-truepath change while buffer-file-name remains the same?  Yes,
> but I think Eglot could ignore those rare cases, or handle it elsewhere.
> (For example, it could update the cache entry after a buffer is saved.)

Actually, cache invalidation is there, at least for killing a
buffer. Major modes are disabled on killing buffer, and it is removed as
a part of the teardown. Saving to a new buffer isn't handled properly
yet, but this looks like something not really supported by Eglot yet
anyway. If I resave a buffer with C-x C-w its content will be placed in
a new buffer, but old one will not be deleted, and the new one will not
be registered with the eglot server before running M-x revert-buffer
anyways. So that seems like a different issue, really.

Please check out the new patch. I find no issues diverging from the
current behavior using this one. It seems to solve all the performance
issues I stated, and now completely new stuff shows up in the profile,
which to me sounds like the bottlenecks have moved, suggesting we get a
nice performance upgrade.

What do you think? I'll test this for a while and install to master in a
few days if nothing comes up :-)

Theo

0001-Don-t-use-file-truepath-in-Eglot-bug-70036.patch
Description: Text Data