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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
