bug#70036: a fix that

["João Távora" <joaotavora@gmail.com>]

On Thu, Apr 18, 2024 at 4:49 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Date: Thu, 18 Apr 2024 16:32:33 +0100
> > Cc: Eli Zaretskii <eliz@gnu.org>, 70036@debbugs.gnu.org
> >
> > So I've read up on the bug report and I had a close look at the Eglot
> > usage profiles (not the micro-benchmarks, those are reasonably
> > irrelevant in what concerns Eglot).  I see these kinds of things in
> > Theodor's profiles
> >
> >           18   8%           - eglot--TextDocumentPositionParams
> >           18   8%            - eglot--TextDocumentIdentifier
> >           18   8%             - eglot--path-to-uri
> >           15   7%              - file-truename
> >           14   6%               - file-truename
> >           14   6%                - file-truename
> >           11   5%                 - file-truename
> >           11   5%                  - file-truename
> >           11   5%                   - file-truename
> >           10   4%                    - file-truename
> >           10   4%                     - file-truename
> >            8   3%                      - file-truename
> >            8   3%                       - file-truename
> >            8   3%                        - file-truename
> >            5   2%                         - file-truename
> >            3   1%                          - file-truename
> >            2   0%                           - file-truename
> >            1   0%                              file-truename
> >
> >
> > I could reproduce this, but never even close to the amount of ~7-8%.
> > Best I could get was 1% and I had to work pretty hard for it.  If I
> > invoke completion or something heavier like that, it completely
> > dominates the profile.
> >
> >           25   1%         - eglot--TextDocumentPositionParams
> >           23   1%          - eglot--TextDocumentIdentifier
> >           23   1%           - eglot-path-to-uri
> >           13   0%            - file-truename
> >           13   0%             - file-truename
> >           13   0%              - file-truename
> >           13   0%                 file-truename
> >
> > Maybe that's because file-truename is a recursive function that becomes
> > slower as the path it's asked to analyse becomes longer (obviously,
> > there can be a symlink at every junction).
>
> Profiles can mislead and they can lie.

Theo's profiles attached to this issue were the best I could find.
If the manner in which they were collected was enough to make a
decision in a certainway, they should be good enough to make a decision
in another way.

But I fully agree we should have a more constrained test case.

> It is much easier to time the
> old and the new code doing the same jobs, and compare the times.

So... that's what I did. As I wrote, I first reproduced (a fraction of)
Theo's findings with the old code (before his patch). Then in the latest
master his patch, added my new patch, and  verified that the newest code
no longer reproduces those findings.

Do you have a concrete idea of what this "job" is?   I could only gather
a moderately useful idea from Theo's profiles, but it wasn't vague
by any means.  It seems his "timer-event-handler" which is usually
doing the work for Eglot's at-point documentation job, is spending
about a quarter of its time in file-truename.  That's typical
when just browsing code with the cursor and reading documentation.

> > If so, I think this simpler patch after my sig is all we need, as it
> > completely clears the profile of any "file-truename".  I have reverted
> > the earlier patch and pushed a patch very similar to the one after my sig.
>
> This new code should also be timed and compared to the other two
> versions, before we make the final decision on this.

Of course.  Let's have Theo do this comparison and perhaps describe
in more detail the conditions in which he collected his profiles