[PATCH] D125675: Optimise findRefs for XRefs and docHighlights

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a reviewer: kadircet.
kadircet added a comment.

66% win sounds great, it would be nice to have some detailed numbers (but this is clearly a huge win, so no need to reperform the experiments if numbers are gone)



================
Comment at: clang-tools-extra/clangd/XRefs.cpp:895
+      : PerToken(PerToken), AST(AST) {
+    for (const Decl *D : TD) {
+      TargetDecls.insert(D->getCanonicalDecl());
----------------
nit: no need for braces

you can directly build the set with canonicaldecls and consume it here


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:951
       if (const auto *Tok = TB.spelledTokenAt(L))
-        References.push_back({*Tok, Roles, ID});
+        References.push_back({*Tok, Roles, getSymbolID(D)});
     }
----------------
we're still calling getsymbolid here lots of times, for probably the same symbol.

as discussed, what about building a lookup table on construction from canonicaldecl to symbolid and use that lookup table instead of calling getsymbolid over and over here?


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1255
+           targetDecl(N->ASTNode, Relations, AST.getHeuristicResolver())) {
+        TargetDecls.insert(D);
+      }
----------------
i mean directly inserting canonicaldecls here.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1260
         // and different kinds, deduplicate them.
         llvm::DenseSet<SymbolID> Targets;
+        for (const Decl *D : TargetDecls)
----------------
no need for this and the following loop anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125675