From d19f0666bcd8f7d26aaf4019244c3ed91e47b1b7 Mon Sep 17 00:00:00 2001 From: Aleksandr Platonov Date: Fri, 17 Jul 2020 18:48:56 +0200 Subject: [PATCH] [clang][Tooling] Try to avoid file system access if there is no record 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 --- clang/lib/Tooling/FileMatchTrie.cpp | 14 +++++++++++++- clang/unittests/Tooling/CompilationDatabaseTest.cpp | 9 +++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/clang/lib/Tooling/FileMatchTrie.cpp b/clang/lib/Tooling/FileMatchTrie.cpp index 88dea6b..3b02405 100644 --- a/clang/lib/Tooling/FileMatchTrie.cpp +++ b/clang/lib/Tooling/FileMatchTrie.cpp @@ -105,8 +105,13 @@ public: 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 {}; } @@ -121,6 +126,13 @@ public: 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 AllChildren; getAll(AllChildren, MatchingChild); StringRef Result; diff --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp b/clang/unittests/Tooling/CompilationDatabaseTest.cpp index cc948b8..3bfb0ec 100644 --- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp +++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp @@ -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( -- 2.7.4