From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Newsgroups: gmane.emacs.bugs Subject: bug#70036: a fix that Date: Fri, 19 Apr 2024 00:59:49 +0100 Message-ID: References: <4e670617-6483-4159-a5cf-27a921764c38@email.android.com> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000560951061667c2ac" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="30167"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Eli Zaretskii , 70036@debbugs.gnu.org, felician.nemeth@gmail.com To: Theodor Thornhill Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Apr 19 02:00:22 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rxbfx-0007l0-DB for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 19 Apr 2024 02:00:21 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rxbfT-0005si-AK; Thu, 18 Apr 2024 19:59:51 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rxbfR-0005sU-RD for bug-gnu-emacs@gnu.org; Thu, 18 Apr 2024 19:59:49 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rxbfR-0001iA-EB for bug-gnu-emacs@gnu.org; Thu, 18 Apr 2024 19:59:49 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rxbfe-0006LP-Ss for bug-gnu-emacs@gnu.org; Thu, 18 Apr 2024 20:00:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 19 Apr 2024 00:00:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 70036 X-GNU-PR-Package: emacs Original-Received: via spool by 70036-submit@debbugs.gnu.org id=B70036.171348478424194 (code B ref 70036); Fri, 19 Apr 2024 00:00:02 +0000 Original-Received: (at 70036) by debbugs.gnu.org; 18 Apr 2024 23:59:44 +0000 Original-Received: from localhost ([127.0.0.1]:55402 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rxbfI-0006HY-Ik for submit@debbugs.gnu.org; Thu, 18 Apr 2024 19:59:43 -0400 Original-Received: from mail-lj1-x22b.google.com ([2a00:1450:4864:20::22b]:43504) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rxbfG-0006GN-83 for 70036@debbugs.gnu.org; Thu, 18 Apr 2024 19:59:39 -0400 Original-Received: by mail-lj1-x22b.google.com with SMTP id 38308e7fff4ca-2dae975d0dcso25581181fa.1 for <70036@debbugs.gnu.org>; Thu, 18 Apr 2024 16:59:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713484758; x=1714089558; darn=debbugs.gnu.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=V9+W8dCveVR2yaZtCWdaxb+sNDspj5ia5ArMQ4W97mc=; b=DZBo70YCt/+M8MVFQxgY7qnehtSR1aA7rEpISd+D70fbmviqpQn//QOhKR03J5aLC8 pHigLOAT0mfEPrAAc1uY7xvQsrBKydJZhZG7NLCAACHjUPTwCJFfLY3r8u971Mnla8c4 lrrfkkWhklN4Y/JAwCSpDIsVIVI6mxPZAvJhYsAFwk6uexKZs0J1KYFyWEqQurnlRqG3 +8NJ8ViQkADDaXYCGFIZcKuYxBpmDoHlQgn6Vk7RTeXX/m6o1N0zClg9xWbHldUrM4D+ 3p60484Y14CCpmX3zXttiTNHY599vhCRCTRXt3iAz+0LWnLCJHZmBMpVTO0jw3cwk6Gd KQ+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713484758; x=1714089558; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=V9+W8dCveVR2yaZtCWdaxb+sNDspj5ia5ArMQ4W97mc=; b=OJQt8IbwzIcXW3O+ikWHN04QGUr2bXTWPutKT2bKaVASrrPzqNmjtlgOaxLGLY7qva qG9/FAr9IlLY9tXrxwhJMHCfQC80EmFJTkY2BQO+e9givTBNpjdDiu6lSXP496g9gigy KNEPPLaQUfJas8x572hZHP3TZfg990wgZKGvemU+DsTMwnl4a7uawnRn5Ix0xF1nyenc 3PzfgSJ4+nyzsELaoKaDHpsMzJ+XD5zV6XdMAwdHic+Ve6I3r06oxVzYR0on03M/YU0h gPlxFoj3nQ5Aq1xcoDgo3oEM7ZXyAiqUghXxOily9VWdgz+coNnpekfNaTKHsfo3NPzr fhFg== X-Forwarded-Encrypted: i=1; AJvYcCVqwDcBmv6Nz01plmI4llvV071v2KTHuw5VoGML5OxoPAncaQGIWJcL/HmuldxYnxm9MrXtmh3GUBp+v7Q/+RiuhsxSLgg= X-Gm-Message-State: AOJu0YwSnFJgwflgW3xDxpwRw8HZYxKfgppOYMttLPGbGuXJyHNrXmxy ulilS7bguSel9jYnTOoeqNLzT4feNJx2tVpF3vvU86eZPM3uJBMD806yE9ZAiolp1TIwsS2yYAm cqd+X+G/QIAqawUYpTYjVxn1S0cM= X-Google-Smtp-Source: AGHT+IHS+wfr2nViYy7gKSBRNNii08ESXfo1r67VTerXgZsqQmPYsbCuUB2HyWft3I/Av22c4Z03xx++fBwUvcWITKs= X-Received: by 2002:a2e:b94d:0:b0:2d8:606d:c797 with SMTP id 13-20020a2eb94d000000b002d8606dc797mr172314ljs.10.1713484758122; Thu, 18 Apr 2024 16:59:18 -0700 (PDT) In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:283620 Archived-At: --000000000000560951061667c2ac Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Apr 18, 2024 at 11:06=E2=80=AFPM Jo=C3=A3o T=C3=A1vora wrote: > Anyway is this the hotspot I should be trying to optimize? > > > 9 4% - find-buffer-visiting Alright, i've reproduced this 33 5% - eglot-handle-notification 33 5% - apply 33 5% - # 29 4% - find-buffer-visiting 29 4% - file-truename 29 4% - file-truename 25 4% - file-truename 25 4% - file-truename 25 4% - file-truename 25 4% - file-truename 25 4% + file-truename But I have to say, I wouldn't call this a severe performance penalty. I followed your instructions very closely and invoked Emacs like this: emacs -Q foo/bar/baz/foo/bar/baz/foo/bar/baz/foo/bar/baz/gin/fs.go -f go-ts-mode The directory is ~/tmp/theo/foo/bar... so it's a pretty long path with many directories all in all. But I didn't have to wait 10 seconds for the LSP to settle down! It was pretty snappy on my 2018 Lenovo T480 running Archlinux. And if I profile anything other than the initial M-x eglot (which normally happens only once in a work session), I don't find any file-truename in the profile= . So my perception is that it must have spent around 4% of 1 second in file-truename. Anyway the reason this shows in this profile is because this project with this particular server sends a lot of publishDiagnostics upfront. That's OK. Gopls is a very good server. I think I see a fix. But can you qualitatively describe the Eglot experience. Do you notice any input lag or something like that? With this project? I didn't feel _any_ lag. Super snappy. Maybe the JSON serde kicking in? Anyway, the idea I suggested earlier is in the patch after my sig. Let's think: LSP's publishDiagnostics coming from the server deals in URIs, right? And we inform the LSP server about URIs, too, right? So the URI is always LSP's idea of the resource identifier (and it likes to have truename URI). My last "better fix" commit records this URI in the buffer as a buffer local variable eglot--cached-tdi and it has to do that for every didOpen. So, to find an open buffer pertaining to a certain LSP's publishDiagnostic= s it suffices in theory to go through all the buffers that have a non-nil cached URI and compare that. No need to convert from URI to file names, not for this job at least! I tried this and it worked fine. When I do that, the profile is completely free of those 4% that bothered you. I'm still testing this for edge cases and will sleep on it, but it seems promisingly simple at least. I can't run unit tests right now, because a recent adventurous commit by Stefan Monnier broke them all :-) but I'm confident that will be fixed soon... I hope you can try this patch. Jo=C3=A3o diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index 90a607075d3..38a16b15e4c 100644 --- a/lisp/progmodes/eglot.el +++ b/lisp/progmodes/eglot.el @@ -2381,6 +2381,9 @@ eglot-handle-notification (lambda () (remhash token (eglot--progress-reporters server)))))))))) +(defvar-local eglot--cached-tdi nil + "A cached LSP TextDocumentIdentifier URI string.") + (cl-defmethod eglot-handle-notification (_server (_method (eql textDocument/publishDiagnostics)) &key uri diagnostics &allow-other-keys) ; FIXME: doesn't respect `eglot-strict-mode' @@ -2391,9 +2394,14 @@ eglot-handle-notification ((=3D sev 2) 'eglot-warning) (t 'eglot-note))) (mess (source code message) - (concat source (and code (format " [%s]" code)) ": " message))) + (concat source (and code (format " [%s]" code)) ": " message)) + (find-it (uri) + (cl-loop for b in (buffer-list) + when (with-current-buffer b + (equal eglot--cached-tdi uri)) + return b))) (if-let* ((path (expand-file-name (eglot-uri-to-path uri))) - (buffer (find-buffer-visiting path))) + (buffer (find-it uri))) (with-current-buffer buffer (cl-loop initially @@ -2518,9 +2526,6 @@ eglot-handle-request (t (setq success :json-false))) `(:success ,success))) -(defvar-local eglot--cached-tdi nil - "A cached LSP TextDocumentIdentifier URI string.") - (defun eglot--TextDocumentIdentifier () "Compute TextDocumentIdentifier object for current buffer." `(:uri ,(or eglot--cached-tdi --000000000000560951061667c2ac Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Thu, Apr 18, 2024 at 11:06=E2=80=AFPM Jo=C3=A3o T=C3=A1= vora <joaotavora@gmail.com&g= t; wrote:

> Anyway is this the hotspot I should be trying to opti= mize?
>
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A09 =C2= =A0 4% =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0- find-buffer-visiti= ng

Alright, i've reproduced this

=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 33 =C2=A0 5% =C2=A0 =C2=A0 =C2=A0 - eglot-handle-notification=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 33 =C2=A0 5% =C2=A0 =C2=A0 =C2=A0 =C2= =A0- apply
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 33 =C2=A0 5% =C2=A0 =C2=A0= =C2=A0 =C2=A0 - #<compiled-function B79>
=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 29 =C2=A0 4% =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0- find-buffer-vis= iting
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 29 =C2=A0 4% =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 - file-truename
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 29 = =C2=A0 4% =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0- file-truename
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 25 =C2=A0 4% =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 - file-truename
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 25 =C2= =A0 4% =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0- file-truename
= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 25 =C2=A0 4% =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 - file-truename
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= 25 =C2=A0 4% =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0- file= -truename
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 25 =C2=A0 4% =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 + file-truename

But I have= to say, I wouldn't call this a severe performance penalty.
I follow= ed your instructions very closely and invoked Emacs like this:

emacs= -Q foo/bar/baz/foo/bar/baz/foo/bar/baz/foo/bar/baz/gin/fs.go =C2=A0-f go-t= s-mode

The directory is ~/tmp/theo/foo/bar... so it's a pre= tty long path with
many directories all in all.

But I di= dn't have to wait 10 seconds for the LSP to settle down!=C2=A0 It waspretty snappy on my 2018 Lenovo T480 running Archlinux.=C2=A0 And if I profile anything other than the initial M-x eglot (which normally happens=
only once in a work session), I don't find any file-truename i= n the profile.

So my perception is that it must ha= ve spent around 4% of 1 second in
file-truename.
Anyway the reason this shows in this profile is because this project= =C2=A0
with this particular server sends a lot of publishDiagnost= ics upfront. =C2=A0
That's OK.=C2=A0 Gopls is a very good ser= ver.=C2=A0 I think I see a fix.=C2=A0 But can=C2=A0
you qualitati= vely describe the Eglot experience.=C2=A0 Do you notice any=C2=A0
input lag or something like that? With this project?=C2=A0 I didn't fe= el _any_ lag.
Super snappy.=C2=A0 Maybe the JSON serde kicking in= ?

Anyway, the idea I suggested earlier is in the patch af= ter=C2=A0 my sig.

Let's think:=C2=A0 LSP's= publishDiagnostics coming from the server deals=C2=A0
in URIs, r= ight?=C2=A0 And we inform the LSP server about URIs, too, right?
= So the URI is always LSP's idea of the resource identifier (and it like= s
to have truename URI).

My last &qu= ot;better fix" commit records this URI in the buffer as a buffer
=
local variable eglot--cached-tdi and it has to do that for every didOp= en.

So, to find an open buffer pertaining to= =C2=A0 a certain LSP's publishDiagnostics
it suffices in theo= ry to go through all the buffers that have a non-nil cached
URI a= nd compare that. =C2=A0

No need to convert from UR= I to file names, not for this job at least!
I tried this and it w= orked fine.

When I do that, the profile is completely free of = those 4% that
bothered you.

I'm still= testing this for edge cases and will sleep on it, but it seems=C2=A0
=
promisingly simple at least.=C2=A0 I can't run unit tests right no= w, because
a recent adventurous commit by Stefan Monnier broke th= em all :-)=C2=A0
but I'm confident that will be fixed soon...=

I hope you can try this patch.

<= div>Jo=C3=A3o

diff --git a/lisp/progmodes/eglot.el b/li= sp/progmodes/eglot.el
index 90a607075d3..38a16b15e4c 100644
--- a/lis= p/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2381,6 +2381,9= @@ eglot-handle-notification
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(lambda ()
=C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0(remhash token (eglot--progress-reporters server))))))))))
=C2= =A0
+(defvar-local eglot--cached-tdi nil
+ =C2=A0"A cached LSP T= extDocumentIdentifier URI string.")
+
=C2=A0(cl-defmethod eglot-= handle-notification
=C2=A0 =C2=A0(_server (_method (eql textDocument/pub= lishDiagnostics)) &key uri diagnostics
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 &allow-other-keys) ; FIXME: doesn't respect `eglot-st= rict-mode'
@@ -2391,9 +2394,14 @@ eglot-handle-notification
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0((= =3D sev 2) =C2=A0'eglot-warning)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(t =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0'eglot-note)))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(m= ess (source code message)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0(concat source (and code (format " [%s]" code)) ": &qu= ot; message)))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(concat= source (and code (format " [%s]" code)) ": " message))=
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(find-it (uri)
+ =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(cl-loop for b in (buffer-list)+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 when (with-current-buffer b
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(equal= eglot--cached-tdi uri))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return b)))
=C2=A0 =C2=A0 =C2=A0(if-let*= ((path (expand-file-name (eglot-uri-to-path uri)))
- =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(buffer (find-buffer-visiting path)))
+ = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(buffer (find-it uri)))
= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(with-current-buffer buffer
=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(cl-loop
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 initially
@@ -2518,9 +2526,6 @@ eglot-handle-request
=C2= =A0 =C2=A0 =C2=A0 (t (setq success :json-false)))
=C2=A0 =C2=A0 =C2=A0`(= :success ,success)))
=C2=A0
-(defvar-local eglot--cached-tdi nil
-= =C2=A0"A cached LSP TextDocumentIdentifier URI string.")
-=C2=A0(defun eglot--TextDocumentIdentifier ()
=C2=A0 =C2=A0"Comput= e TextDocumentIdentifier object for current buffer."
=C2=A0 =C2=A0`= (:uri ,(or eglot--cached-tdi
--000000000000560951061667c2ac--