sammccall added inline comments.

================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:616
+    Position HintStart = sourceLocToPosition(SM, BraceRange.getEnd());
+    Position HintEnd = {HintStart.line,
+                        HintStart.character +
----------------
daiyousei-qz wrote:
> sammccall wrote:
> > please don't do this, call sourceLocToPosition unless you have benchmarks 
> > showing this is a bottleneck on real-world code.
> > 
> > `lspLength` and direct manipulation of line/character is unneccesarily 
> > subtle.
> > (If the performance matters, there's no need to be computing the LSP line 
> > of the lbrace at all - we never use it - this is one reason I think this 
> > function has the wrong signature)
> I argue performance does matter here. I eye-balled inlay hint compute time in 
> clangd's output window from vscode. On "./clang/lib/Sema/SemaOpenMP.cpp" 
> which is around 1MB, using `sourceLocToPosition` roughly takes around 
> 1250~1350ms. However, using two `offsetLocToPosition` usually use 
> 1400~1500ms.  I think 100ms delta is already a noticeable difference.
I'm unable to reproduce these results. After verifying EndBlock hints work in 
an editor:

```
$ bin/clangd --check=../clang/lib/Sema/SemaOpenMP.cpp
I[15:50:26.357] clangd version 17.0.0
I[15:50:26.358] Features: linux+debug-tidy
...
 I[15:50:26.358] Testing on source file 
/usr/local/google/home/sammccall/src/llvm-project/clang/lib/Sema/SemaOpenMP.cpp
...
I[15:50:26.428] Building preamble...
I[15:50:31.097] Indexing headers...
I[15:50:32.027] Built preamble of size 53468188 for file 
/usr/local/google/home/sammccall/src/llvm-project/clang/lib/Sema/SemaOpenMP.cpp 
version null in 5.60 seconds
I[15:50:32.032] Building AST...
I[15:50:38.857] Indexing AST...
I[15:50:38.975] Building inlay hints
I[15:50:38.993] Building semantic highlighting
I[15:50:39.050] Testing features at each token (may be slow in large files)
...
```

So that's 18ms for inlay hints after 5.6 seconds to parse the preamble and 6 
seconds to parse the main file. (This is a debug build, opt must be faster).

This is on a fairly powerful machine, but surely it can't be 1000x faster than 
yours - something else must be going on.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150635/new/

https://reviews.llvm.org/D150635

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to