bug#70036: a fix that

[Theodor Thornhill]

 Eli Zaretskii <eliz@gnu.org> writes:

 

 >> 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.  It is much easier to time the

 > old and the new code doing the same jobs, and compare the times.

 >

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

 

 That's unfortunate.

 

 >

 > This new code should also be timed and compared to the other two

 > versions, before we make the final decision on this.

 

 

 This is a very unfortunate change. This completely misses the point and

 reverts any perf gains from my previous patch... I think your

 conclusions are too quick, and actions likewise.

 

 I'd argue that long paths are a way more common occurrence than

 symlinking, and the places you touched in your "better way" ignores the

 performance critical parts.

 

 The function you are suggesting isn't the hotspot. Most notable

 publishDiagnostics is, which now is killed again.

 

 Theo