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