Skip to content

Commit

Permalink
[clang][Tooling] Try to avoid file system access if there is no recor…
Browse files Browse the repository at this point in the history
…d for the file in compile_commads.json

Summary:
If there is no record in compile_commands.json, we try to find suitable record with `MatchTrie.findEquivalent()` call.
This is very expensive operation with a lot of `llvm::sys::fs::equivalent()` calls in some cases.

This patch disables file symlinks for performance reasons.

Example scenario without this patch:
- compile_commands.json generated at clangd build (contains ~3000 files).
- it tooks more than 1 second to get compile command for newly created file in the root folder of LLVM project.
- we wait for 1 second every time when clangd requests compile command for this file (at file change).

Reviewers: sammccall, kadircet, hokein

Reviewed By: sammccall

Subscribers: chandlerc, djasper, klimek, ilya-biryukov, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D83621
  • Loading branch information
ArcsinX authored and sam-mccall committed Jul 17, 2020
1 parent 0e347c0 commit d19f066
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
14 changes: 13 additions & 1 deletion clang/lib/Tooling/FileMatchTrie.cpp
Expand Up @@ -105,8 +105,13 @@ class FileMatchTrieNode {
StringRef FileName,
bool &IsAmbiguous,
unsigned ConsumedLength = 0) const {
// Note: we support only directory symlinks for performance reasons.
if (Children.empty()) {
if (Comparator.equivalent(StringRef(Path), FileName))
// As far as we do not support file symlinks, compare
// basenames here to avoid request to file system.
if (llvm::sys::path::filename(Path) ==
llvm::sys::path::filename(FileName) &&
Comparator.equivalent(StringRef(Path), FileName))
return StringRef(Path);
return {};
}
Expand All @@ -121,6 +126,13 @@ class FileMatchTrieNode {
if (!Result.empty() || IsAmbiguous)
return Result;
}

// If `ConsumedLength` is zero, this is the root and we have no filename
// match. Give up in this case, we don't try to find symlinks with
// different names.
if (ConsumedLength == 0)
return {};

std::vector<StringRef> AllChildren;
getAll(AllChildren, MatchingChild);
StringRef Result;
Expand Down
9 changes: 9 additions & 0 deletions clang/unittests/Tooling/CompilationDatabaseTest.cpp
Expand Up @@ -281,6 +281,15 @@ TEST_F(FileMatchTrieTest, CannotResolveRelativePath) {
EXPECT_EQ("Cannot resolve relative paths", Error);
}

TEST_F(FileMatchTrieTest, SingleFile) {
Trie.insert("/root/RootFile.cc");
EXPECT_EQ("", find("/root/rootfile.cc"));
// Add subpath to avoid `if (Children.empty())` special case
// which we hit at previous `find()`.
Trie.insert("/root/otherpath/OtherFile.cc");
EXPECT_EQ("", find("/root/rootfile.cc"));
}

TEST(findCompileArgsInJsonDatabase, FindsNothingIfEmpty) {
std::string ErrorMessage;
CompileCommand NotFound = findCompileArgsInJsonDatabase(
Expand Down

0 comments on commit d19f066

Please sign in to comment.