all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "João Távora" <joaotavora@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: felician.nemeth@gmail.com, 70036@debbugs.gnu.org, theo@thornhill.no
Subject: bug#70036: a fix that
Date: Thu, 18 Apr 2024 21:21:25 +0100	[thread overview]
Message-ID: <CALDnm517N=DM+0tqBpHCGiHenPMirq2eFJF+Gk0ePO+W6WO-Ug@mail.gmail.com> (raw)
In-Reply-To: <86jzkud0be.fsf@gnu.org>

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.

Really, you want to go at it again?  Well let me just add a pinch of
salt to your typical condescension lecture, getting some facts straight
about what happened here.

* I had to take a long timeout from Eglot maintenance for personal
reasons managed to get maintainers for Flymake and Jsonrpc, but I am
still Eglot maintainer.

* I notice some discussions going on and I let them proceed with
little input.

* I regain some capacity some months later, notice a user report
on GitHub alerts that there are some very new things in Eglot that
didn't ask for explicit greenlighting as was usual during the last 7
years I'm maintaining Eglot.  Fair enough, life went on without me.

* I post to Emacs-devel about these.  I take care to thank everyone
for their efforts.

* One of the changes is Stefan Monnier's.  It's broken, he fixes it
immediately in the usual good spirits.

* Another one of the changes is Theodor's.  It's a good change and
I highlight it as such, merely request a change to the doc.

* The other change is also Theodor's.  I commend it for its clarity
and implementation, but ultimately notice a serious bug that affects
me and anyone else working a super-common C/C++ servers and symlinks.

* I go and read the full bug report and notice that Theodor has been
"studying Eglots performance" and has come to the conclusion that
there is a hotspot of performance problems.  He posts in that email
what to this time is the only actual hard data we have.  That
data suggests that file-truename is slow, and that Eglot uses it
a lot in eglot--TextDocumentIdentifier.

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.

* 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.

* I weigh the pros and cons, especially the time I have to address this
and other Eglot recent problems (tests are borked, have to see what happened)
Since I am Eglot maintainer and revert it myself, just like I've
reverted my and others own perfectly well-intentioned and laboriously
crafted  commits  many times and it hurts a bit to do it, but not that much.

It's a no-brainer to revert something like this to me. There is so far
very little hard evidence of the effect this is having on the general
population, it's a very uncommon report.  This code has been there
for 7 years and while there have been performance bugs, this was never
one of it.  This is why I write "one-off" and "uncommon".  Absolutely
accurate.  Ultimately, I see that very little testing has been done around some
rather obvious use cases of removing symlink-smarts from Eglot.  The "my
servers happen not to have this" just isn't an acceptable argument to me.

Sure if Eglot had behaved like this all along from the beginning, displaying
duplicated references in some servers, maybe it would be different, but
that's a different world.  Bottom line, Eglot worked correctly three weeks
ago and now has a new bug introduced by a performance fix based on very
little hard data, and a single very scarse report.  It's a no brainer
to revert.

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.

Your last question to Theodor on this thread before the change was merge
good but the answer was completely misinterpreted.   You ask if the servers
resolve symlinks and Theodor answers this:

  > The LSP specification does not talk about symlinks.  The
  > servers I used let the operating system resolve symlinks for them.

Well, maybe at that point if one would have noticed that there are
hundreds of servers arounds, this could have taken a different turn.

> > I've told you I'm invested in fixing Theo's performance problems,
> > but I must learn more about them obviously.  But I'm not going to
> > keep a regression in just to be nice to people.
>
> If -- and it's stil an "if" -- it was a regression,

Did you not read my experiment with the three-file project and the
clangd server?

https://lists.gnu.org/archive/html/emacs-devel/2024-04/msg00350.html

Is anything unclear?  If you've seen it but don't believe my results
maybe you should try Eglot yourself: seeing is believing, they say.

> it was there for quite some time

So was the supposed relevant performance problem.  Only that time is
measured in years and this is measured in weeks.

> , so nothing serious would have happened if we'd leave
> it there for a few more hours or days.

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

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 await Theo's reproducible experiment so that we can work on what's
really important.

João





  reply	other threads:[~2024-04-18 20:21 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-27 19:08 bug#70036: 30.0.50; Move file-truename to the C level Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-27 19:44 ` Eli Zaretskii
2024-03-27 21:56   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28  1:14     ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28  3:05       ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28  7:04         ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28  7:03       ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28  6:22     ` Eli Zaretskii
2024-03-28  7:03       ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-27 20:12 ` Felician Nemeth
2024-03-27 21:43   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28  6:03     ` Eli Zaretskii
2024-03-28  7:10       ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28  8:52         ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28 11:55         ` Felician Nemeth
2024-03-28 12:08           ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-30  9:46             ` Felician Nemeth
2024-03-30 11:18               ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-30 12:45               ` Eli Zaretskii
2024-03-31 12:57                 ` Felician Nemeth
2024-03-31 13:32                   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28  9:22 ` Ihor Radchenko
2024-03-28 10:59   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28 11:18     ` Ihor Radchenko
2024-03-28 11:41       ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28 11:51         ` Ihor Radchenko
2024-03-28 12:47           ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-28 13:52           ` Eli Zaretskii
2024-04-18 15:32 ` bug#70036: a fix that João Távora
2024-04-18 15:39   ` João Távora
2024-04-18 15:40   ` Ihor Radchenko
2024-04-18 15:45     ` João Távora
2024-04-18 15:49   ` Eli Zaretskii
2024-04-18 16:11     ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-18 16:15       ` João Távora
2024-04-18 16:29         ` Eli Zaretskii
2024-04-18 17:22           ` João Távora
2024-04-18 17:53             ` Eli Zaretskii
2024-04-18 20:21               ` João Távora [this message]
     [not found]                 ` <874jbycrd7.fsf@dick>
2024-04-18 21:26                   ` João Távora
2024-04-18 21:37                     ` João Távora
2024-04-19  9:17                       ` Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-18 21:32                 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-18 22:06                   ` João Távora
2024-04-18 23:59                     ` João Távora
2024-04-19  6:09                       ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19  6:26                         ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19  8:06                           ` João Távora
2024-04-19  9:05                             ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19  8:01                         ` João Távora
2024-04-19  9:10                           ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19  9:22                             ` João Távora
2024-04-19  5:58                     ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19  7:52                       ` João Távora
2024-04-19  9:14                         ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19  6:56                     ` Eli Zaretskii
2024-04-19  7:51                       ` Ihor Radchenko
2024-04-19 10:51                         ` Eli Zaretskii
2024-04-30 11:30                           ` Ihor Radchenko
2024-05-02  9:40                             ` Eli Zaretskii
2024-04-19  8:27                       ` João Távora
2024-04-19  8:49                         ` João Távora
2024-04-19 11:12                           ` Eli Zaretskii
2024-04-19 11:34                             ` João Távora
2024-04-19 18:13                               ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19 18:59                                 ` João Távora
2024-04-19 19:42                                   ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19 11:01                         ` Eli Zaretskii
2024-04-19 11:32                           ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19 11:40                             ` João Távora
2024-04-19 11:47                               ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19 11:51                                 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19 12:01                                   ` João Távora
2024-04-19 11:51                                 ` João Távora
2024-04-19 20:23                               ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19 21:32                                 ` João Távora
2024-04-19 11:53                             ` Eli Zaretskii
2024-04-19 11:59                               ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19 12:03                               ` João Távora
2024-04-19 12:00                           ` João Távora
2024-04-19 12:13                             ` Eli Zaretskii
2024-04-19 12:20                               ` João Távora
2024-04-19  6:45                   ` Eli Zaretskii
2024-04-19  7:38                     ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-19 12:54                     ` João Távora
2024-04-19 14:32                       ` Eli Zaretskii
2024-04-19  0:57                 ` Yuan Fu
2024-04-19  1:20                   ` João Távora
2024-04-22 22:11                 ` Dmitry Gutov
2024-04-18 16:21       ` Eli Zaretskii
2024-04-18 16:12     ` João Távora
2024-04-18 16:24       ` Eli Zaretskii
2024-04-18 16:33         ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-18 16:36           ` Eli Zaretskii
2024-04-18 17:26           ` João Távora
2024-04-18 17:27         ` 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='CALDnm517N=DM+0tqBpHCGiHenPMirq2eFJF+Gk0ePO+W6WO-Ug@mail.gmail.com' \
    --to=joaotavora@gmail.com \
    --cc=70036@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=felician.nemeth@gmail.com \
    --cc=theo@thornhill.no \
    /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.