bug#70036: a fix that

Im having some email client issues. I hope this mail renders readable.

João Távora <joaotavora@gmail.com> writes:

> On Thu, Apr 18, 2024 at 6:53 PM Eli Zaretskii <eliz@gnu.org> wrote:

>

>> I suggest to get an objective opinion of that from an uninvolved

>> party.

>

> I suggest you drop your sarcasm not because it hurts me or anything

> but just because you're not very good at it.

>

>> > So let's skip the morals.

>>

>> No, let's not.

[...]

I'll let most of this pass, but yikes. Let's just relax a little.

>

> It doesn't suggest anything other than it, nothing about

> textDocument/publishDiagnostics. There's 0 data about that

> and it never showed up in my profile.

I don't remember exactly right now, but I believe I mentioned many times

the performance of file-truename directly, and find-buffer-visiting

which uses file-truename indirectly.

>

> * Theodor suggests rewriting file-truneame in C, you decline or raise

> doubts. I don't follow the reasons, but fair enough, the discussion

> pivots to changing Eglot.

>

> * I reproduce Theodor's experimental findings.

>

> * I find a patch to address that hotspot that is several times

> faster than Theodor's solution (doesn't really matter though).

>

> * I ask Theodor to revert the patch and start afresh. He understands

> but declines.

Did I decline? I've lost track, but I believe I said something in the

lines of "let's discuss it first", but nevermind.

[...]

>

> Still, for the very little data that there is available, I do take care

> to put in a much simpler fix that completely fixes the issue - as far as

> I or anyone reading the available data can see it.

With this I disagree - but I guess I either have miscommunicated or have

provided unclear profiles or something inbetween.

>

> And nothing serious will happen after I reverted it, will there?

Not serious, but now we have a performance regression ;-)

>

> I have no interest in delaying a responsible decision just for the

> sake of appeasing feelings that someone else says are there. If

> Theodor is worried about a specific performance problem I have some

> time this week and the next one to help fix it, and I'm confident I will.

>

> For the record my repositories at $DAYJOB with very long path names

> _and_ very symlinks. So I'm personally interested in fixing any

> performance problems and not opening new ones.

>

>> > I'm sure Theo can understand that.

>>

>> He obviously felt hurt, so "understand" is not an appropriate word

>> here.

>

> I'm confident he will understand this, I can't be sure of course.

> But I know he wrote "could absolutely do that [revert]" though he preferred

> not to do it. That at least shows that it's not an absurd proposition.

> And _I_ understand that he doesn't want to, of course.

>

> I don't know if Theodor felt hurt, and even if I suspected something, I

> wouldn't write about it in public simply because I personally find it

> in poor taste to speculate or moralize about others people's feelings

> secondhand.

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. And I don't think Eli

moralizes on my behalf. But I don't have time to linger on that.

>

> I await Theo's reproducible experiment so that we can work on what's

> really important.

Yes, now over to my example.

I'll add two profiles, one is profile-fast and profile-slow. You can

guess which is which :)

STEPS:

I've made an intentionally broken commit into a golang repo I just

forked, to get some publishDiagnostics quickly.

(for good measure evaluate eglot.el before running profiles)

1. Install gopls

2. Make some directory you can wipe out after the test and cd into it

3. git clone git@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.

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.

Now let's stop fighting.

Theo

Profile-fast:

67 39% - command-execute

63 37% - byte-code

63 37% - read-extended-command

63 37% - read-extended-command-1

63 37% - completing-read-default

21 12% redisplay_internal (C function)

3 1% - command-execute

2 1% list

2 1% - frame-windows-min-size

2 1% window-normalize-frame

2 1% - internal-timer-start-idle

2 1% timerp

1 0% - timer-event-handler

1 0% - apply

1 0% - show-paren-function

1 0% - show-paren--default

1 0% - show-paren--locate-near-paren

1 0% back-to-indentation

1 0% #:minibuffer-setup

1 0% - undo-auto--add-boundary

1 0% - undo-auto--boundaries

1 0% - undo-auto--ensure-boundary

1 0% undo-auto--needs-boundary-p

4 2% - funcall-interactively

4 2% - execute-extended-command

3 1% - command-execute

3 1% - funcall-interactively

3 1% - eglot

3 1% - eglot--connect

3 1% - let*

2 1% - project-name

2 1% - apply

2 1% - #<compiled-function 6EA>

2 1% - apply

2 1% - #<compiled-function 2C5>

2 1% - project--value-in-dir

2 1% - hack-dir-local-variables

2 1% - hack-dir-local--get-variables

1 0% - dir-locals-find-file

1 0% - locate-dominating-file

1 0% dir-locals--all-files

1 0% - apply

1 0% - make-instance

1 0% puthash

41 24% Automatic GC

33 19% - redisplay_internal (C function)

3 1% - redisplay--pre-redisplay-functions

2 1% run-hook-with-args

18 10% - ...

10 5% - eglot--glob-parse

10 5% - let

10 5% - save-current-buffer

10 5% - unwind-protect

10 5% - progn

10 5% - let*

10 5% - while

9 5% - let

9 5% - let*

9 5% - progn

8 4% - apply

8 4% - eglot-register-capability

8 4% - apply

8 4% - #<closure E95>

8 4% - progn

8 4% - let*

8 4% - progn

8 4% - let*

7 4% - delete-dups

7 4% - mapcar

6 3% - project-files

6 3% - apply

6 3% - #<compiled-function 81C>

6 3% - mapcan

6 3% - #<compiled-function 8AE>

3 1% - project--vc-list-files

2 1% - project--value-in-dir

2 1% - hack-dir-local-variables

2 1% - hack-dir-local--get-variables

2 1% - dir-locals-find-file

2 1% - locate-dominating-file

2 1% dir-locals--all-files

1 0% - project--vc-merge-submodules-p

1 0% - project--value-in-dir

1 0% - hack-dir-local-variables

1 0% - hack-dir-local--get-variables

1 0% - dir-locals-find-file

1 0% - locate-dominating-file

1 0% dir-locals--all-files

2 1% - file-equal-p

2 1% - file-truename

1 0% - file-truename

1 0% file-truename

1 0% - project--value-in-dir

1 0% - hack-dir-local-variables

1 0% - hack-dir-local--get-variables

1 0% - dir-locals-find-file

1 0% - locate-dominating-file

1 0% dir-locals--all-files

1 0% file-name-directory

1 0% - let

1 0% - unwind-protect

1 0% - progn

1 0% - mapc

1 0% - #<closure D31>

1 0% - let*

1 0% - and

1 0% - or

1 0% - puthash

1 0% - list

1 0% file-notify-add-watch

1 0% - let*

1 0% - eglot--workspace-configuration-plist

1 0% - let

1 0% - save-current-buffer

1 0% - unwind-protect

1 0% - progn

1 0% - hack-dir-local-variables-non-file-buffer

1 0% - hack-dir-local-variables

1 0% - hack-dir-local--get-variables

1 0% - dir-locals-find-file

1 0% - locate-dominating-file

1 0% dir-locals--all-files

1 0% - setq

1 0% - cons

1 0% - let*

1 0% - while

1 0% - and

1 0% - progn

1 0% - setq

1 0% - not

1 0% setq

4 2% - #<closure 67D>

4 2% - let

4 2% - apply

4 2% - eglot-handle-notification

4 2% - apply

4 2% - #<closure A8D>

4 2% - progn

3 1% - eglot--signal-textDocument/didOpen

3 1% - jsonrpc-notify

3 1% - list

3 1% - eglot--TextDocumentItem

3 1% - append

2 1% - eglot--VersionedTextDocumentIdentifier

2 1% - append

2 1% - eglot--TextDocumentIdentifier

2 1% - list

2 1% - eglot-path-to-uri

2 1% - let

2 1% - if

2 1% - and

2 1% - let*

2 1% url-generic-parse-url

1 0% - eglot--guess-contact

1 0% - let*

1 0% - eglot--lookup-mode

1 0% - let*

1 0% - progn

1 0% - let*

1 0% - while

1 0% - and

1 0% - progn

1 0% - setq

1 0% - not

1 0% - setq

1 0% - cl-some

1 0% - function

1 0% cconv-make-interpreted-closure

1 0% - let*

1 0% - progn

1 0% - let*

1 0% - and

1 0% - expand-file-name

1 0% - eglot-uri-to-path

1 0% - let*

1 0% url-generic-parse-url

2 1% - macroexp--expand-all

2 1% - macroexp--all-forms

2 1% - macroexp--expand-all

2 1% - macroexp--all-forms

2 1% - macroexp--expand-all

2 1% - macroexp--all-forms

2 1% - macroexp--expand-all

2 1% - macroexp--all-forms

2 1% - macroexp--expand-all

2 1% - macroexp--all-forms

2 1% - macroexp--expand-all

2 1% - macroexp--all-forms

2 1% - macroexp--expand-all

2 1% - macroexp--all-forms

2 1% - macroexp--expand-all

2 1% macroexp--all-forms

1 0% - eglot-signal-didChangeConfiguration

1 0% - jsonrpc-notify

1 0% - list

1 0% - or

1 0% - eglot--workspace-configuration-plist

1 0% - let

1 0% - save-current-buffer

1 0% - unwind-protect

1 0% - progn

1 0% - hack-dir-local-variables-non-file-buffer

1 0% - hack-dir-local-variables

1 0% - hack-dir-local--get-variables

1 0% - dir-locals-find-file

1 0% - locate-dominating-file

1 0% dir-locals--all-files

1 0% - eldoc-print-current-symbol-info

1 0% - eldoc--invoke-strategy

1 0% - eldoc-documentation-compose

1 0% - eglot-signature-eldoc-function

1 0% - if

1 0% - progn

1 0% - let

1 0% - jsonrpc-async-request

1 0% - jsonrpc--async-request-1

1 0% - jsonrpc-connection-send

1 0% - apply

1 0% - #<compiled-function F08>

1 0% - jsonrpc--event

1 0% - #<compiled-function DEE>

1 0% - apply

1 0% jsonrpc--log-event

11 6% - timer-event-handler

8 4% - apply

7 4% - #<compiled-function ACE>

7 4% - execute-extended-command--shorter

7 4% - #<compiled-function 0F8>

4 2% - commandp

1 0% - interactive-form

1 0% - oclosure-interactive-form

1 0% gethash

1 0% - #<compiled-function AE6>

1 0% - jsonrpc-connection-receive

1 0% - apply

1 0% - jsonrpc--event

1 0% - #<compiled-function D93>

1 0% - apply

1 0% jsonrpc--log-event

2 1% + timer-activate

1 0% + timer-inc-time

Profile-slow

78 36% - command-execute

76 35% - byte-code

76 35% - read-extended-command

76 35% - read-extended-command-1

76 35% - completing-read-default

21 9% redisplay_internal (C function)

12 5% - command-execute

11 5% - funcall-interactively

7 3% - self-insert-command

2 0% - electric-indent-post-self-insert-function

2 0% electric--after-char-pos

1 0% undo-auto-amalgamate

4 1% - execute-extended-command

4 1% - command-execute

4 1% - funcall-interactively

4 1% - eglot

4 1% - eglot--connect

4 1% - let*

2 0% - project-name

2 0% - apply

2 0% - #<compiled-function D4A>

2 0% - apply

2 0% - #<compiled-function D8C>

2 0% - project--value-in-dir

2 0% - hack-dir-local-variables

2 0% - hack-dir-local--get-variables

2 0% - dir-locals-find-file

2 0% - locate-dominating-file

2 0% dir-locals--all-files

1 0% - apply

1 0% - make-instance

1 0% - apply

1 0% - #<compiled-function 29F>

1 0% - initialize-instance

1 0% - apply

1 0% - #<compiled-function 921>

1 0% - apply

1 0% - #<compiled-function E0C>

1 0% - #<closure F34>

1 0% - let

1 0% make-process

1 0% - unwind-protect

1 0% - condition-case

1 0% - let

1 0% - catch

1 0% - jsonrpc-async-request

1 0% - list

1 0% - file-local-name

1 0% file-remote-p

2 0% - timer-event-handler

1 0% - apply

1 0% - blink-cursor-start

1 0% - blink-cursor--start-timer

1 0% - run-with-timer

1 0% - run-at-time

1 0% timer-relative-time

1 0% - minibuffer-setup-on-screen-keyboard

1 0% - device-class

1 0% x-device-class

2 0% - funcall-interactively

2 0% execute-extended-command

64 30% Automatic GC

33 15% redisplay_internal (C function)

23 10% - ...

16 7% - #<closure DEF>

16 7% - let

16 7% - apply

16 7% - eglot-handle-notification

16 7% - apply

16 7% - #<closure 2CB>

16 7% - progn

16 7% - let*

16 7% - progn

15 7% - let*

10 4% - and

9 4% - find-buffer-visiting

9 4% - file-truename

8 3% - file-truename

6 2% - file-truename

4 1% - file-truename

2 0% - file-truename

2 0% file-truename

1 0% - expand-file-name

1 0% - eglot-uri-to-path

1 0% - let*

1 0% url-generic-parse-url

5 2% - delete-dups

5 2% - mapcar

3 1% - project-files

3 1% - apply

3 1% - #<compiled-function CE1>

3 1% - mapcan

3 1% - #<compiled-function 8CE>

2 0% - project--vc-list-files

1 0% - vc-git--run-command-string

1 0% - vc-git--out-ok

1 0% - vc-git--call

1 0% - process-file

1 0% call-process

1 0% - project--vc-merge-submodules-p

1 0% - project--value-in-dir

1 0% - hack-dir-local-variables

1 0% - hack-dir-local--get-variables

1 0% - dir-locals-find-file

1 0% - locate-dominating-file

1 0% dir-locals--all-files

1 0% - file-equal-p

1 0% - file-truename

1 0% file-truename

2 0% - function

2 0% - cconv-make-interpreted-closure

2 0% - macroexpand-all

2 0% - macroexp--expand-all

2 0% - macroexp--all-forms

2 0% - macroexp--expand-all

2 0% - macroexp--all-forms

2 0% - macroexp--expand-all

2 0% - macroexp--all-forms

2 0% - macroexp--expand-all

2 0% - macroexp--all-forms

2 0% - macroexp--expand-all

2 0% - macroexp--all-forms

2 0% - macroexp--expand-all

1 0% function-get

1 0% - macroexp--all-forms

1 0% - macroexp--expand-all

1 0% - macroexp--all-clauses

1 0% - macroexp--all-forms

1 0% - macroexp--expand-all

1 0% function-get

1 0% - if

1 0% - let

1 0% - while

1 0% - let

1 0% - save-current-buffer

1 0% - eglot--maybe-activate-editing-mode

1 0% - if

1 0% - progn

1 0% - eglot--managed-mode

1 0% - let

1 0% - cond

1 0% - if

1 0% add-hook

6 2% - #<closure 76C>

6 2% - progn

6 2% - let*

6 2% - progn

6 2% - let*

5 2% - mapcar

5 2% - #<closure 062>

5 2% - let

5 2% - let*

5 2% - progn

3 1% - let*

3 1% - eglot--workspace-configuration-plist

3 1% - let

3 1% - save-current-buffer

3 1% - unwind-protect

3 1% - progn

3 1% - hack-dir-local-variables-non-file-buffer

3 1% - hack-dir-local-variables

3 1% - hack-dir-local--get-variables

3 1% - dir-locals-find-file

3 1% - locate-dominating-file

3 1% dir-locals--all-files

2 0% - cons

2 0% - eglot--glob-compile

2 0% - let*

2 0% - eglot--glob-parse

2 0% - let

2 0% save-current-buffer

1 0% - let

1 0% - unwind-protect

1 0% - progn

1 0% - mapc

1 0% - #<closure EE0>

1 0% - let*

1 0% - and

1 0% - or

1 0% - puthash

1 0% - list

1 0% file-notify-add-watch

1 0% - eglot-signal-didChangeConfiguration

1 0% - jsonrpc-notify

1 0% - list

1 0% - or

1 0% - eglot--workspace-configuration-plist

1 0% - let

1 0% - save-current-buffer

1 0% - unwind-protect

1 0% - progn

1 0% - hack-dir-local-variables-non-file-buffer

1 0% - hack-dir-local-variables

1 0% - hack-dir-local--get-variables

1 0% - dir-locals-find-file

1 0% - locate-dominating-file

1 0% dir-locals--all-files

12 5% - timer-event-handler

12 5% - apply

7 3% - #<compiled-function B4E>

7 3% - execute-extended-command--shorter

6 2% - #<compiled-function 5ED>

4 1% commandp

5 2% - #<compiled-function 042>

4 1% - jsonrpc-connection-receive

3 1% - apply

2 0% - jsonrpc--event

2 0% - #<compiled-function 7CC>

2 0% - apply

2 0% jsonrpc--log-event

1 0% - jsonrpc--reply

1 0% - jsonrpc-connection-send

1 0% - apply

1 0% - #<compiled-function ED6>

1 0% - jsonrpc--event

1 0% - #<compiled-function 022>

1 0% - apply

1 0% jsonrpc--log-event

1 0% - jsonrpc--continue

1 0% #<closure 8C6>

2 0% - jsonrpc--process-filter

1 0% buffer-string

1 0% - #<compiled-function 32D>

1 0% - timer-activate

1 0% - timer--activate

1 0% timer--time-less-p

From:	Theodor Thornhill
Subject:	bug#70036: a fix that
Date:	Thu, 18 Apr 2024 23:32:00 +0200