GNU bug report logs - #70036
30.0.50; Move file-truename to the C level

Previous Next

Full log

View this message in rfc822 format

From: Theodor Thornhill <theo <at> thornhill.no>
To: João Távora <joaotavora <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 70036 <at> debbugs.gnu.org, felician.nemeth <at> gmail.com
Subject: bug#70036: a fix that
Date: Fri, 19 Apr 2024 07:58:57 +0200

>> I'm fine. I do think you do come off strongly though, and it is not
>> always easy to judge if it is of good faith.
>
> Just think: what reason do I have to be in "bad faith"?  I just invited
> you to collaborate in the GitHub repo and I want you to keep learning
> and improving Eglot.  I just think you did a commit -- in perfectly
> good faith -- that clashes directly with what is a tenet of Eglot:
> correctness and backward compatibility.  I also think there are other
> much better ways to fix the problem (the greatest of these ideas is
> yours!!! rewrite it is C).
>

I agree - that's why I want to focus on improving the code, same as
you. I just want you to understand that being mindful on how you come
across can't really hurt. There are enough conflicts around these days,
let's not make code be one.
>> 1. Install gopls
>> 2. Make some directory you can wipe out after the test and cd into it
>> 3. git clone git <at> github.com:theothornhill/gin.git foo/bar/baz/foo/bar/baz/foo/bar/baz/foo/bar/baz/gin
>> 4. cd foo/bar/baz/foo/bar/baz/foo/bar/baz/foo/bar/baz/gin
>> 5. open fs.go in emacs and make sure some go mode is available. Go-ts-mode for example
>> 6. M-x profiler-start
>> 7. M-x eglot
>> 8. Wait 10-20 seconds. Do no actions other than let the lsp settle.
>> 9. M-x profiler-stop
>> 10. M-x profiler-report
>> 11. Rinse repeat with both or all variants of emacs with/without the
>> latest eglot changes.
>> I'll add my profiles, and let some metrics talk. This is from emacs -Q
>> from your "better fix" and the commit before your revert. Please try on
>> your systems and report back.
>
> Great recipe.
>

Good!

>> By the way. I have a hunch the reason this is so apparent now is because
>> of the faster json serde recently added to emacs. Not sure, but feels
>> like it. Json serde is almost gone from the profiles.
>
> What is Json serde?  Is it Json SERialization/DEserialization?

That's right

> That commit by Hermann made it in? That's great!!  So Eglot is probably
> faster overall. So I guess your bug report is about not missing
> a further optimization opportunity.  Fine.

Yeah, that's right. I assume that when json serde disappeared as a
bottleneck others emerge. This is the obvious one to me right now.

>
> Anyway is this the hotspot I should be trying to optimize?
>
>>            9   4%              - find-buffer-visiting
>
> I still think the cleanest solution is to write file-truename
> in C.  But if that can't be done, it doesn't seem terribly hard
> to get rid of find-buffer-visiting in publishDiagnostics and
> still remain symlink-correct.

I agree, and I'm still experimenting on that. But I need to make sure I
have good test coverage on the current behavior before I start making
the C one. The initial version, while fast, was just to prove that
file-truename should be improved.

>
> That's because every interesting result of find-buffer-visiting
> is a buffer for which we've already issued a 'didOpen', which
> in turn means we've already called file-truename once for it.
> If we cache that result (the basic idea behind the "better fix")
> it should be possible to find the buffer quicker just by iterating
> the list.  That's what I will try to do, using your recipe as
> guide.
>
> João

Sure. That will make the change you make similar in spirit to mine, yet
with a different implementation. I'm fine with that.

Theo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.