Package: emacs;
Reported by: Theodor Thornhill <theo <at> thornhill.no>
Date: Wed, 27 Mar 2024 19:10:02 UTC
Severity: normal
Found in version 30.0.50
Message #140 received at 70036 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: felician.nemeth <at> gmail.com, 70036 <at> debbugs.gnu.org, theo <at> thornhill.no Subject: Re: bug#70036 a fix that Date: Thu, 18 Apr 2024 21:21:25 +0100
On Thu, Apr 18, 2024 at 6:53 PM Eli Zaretskii <eliz <at> 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.