Author: ioeric Date: Tue Dec 18 07:39:33 2018 New Revision: 349496 URL: http://llvm.org/viewvc/llvm-project?rev=349496&view=rev Log: [clangd] BackgroundIndex rebuilds symbol index periodically.
Summary: Currently, background index rebuilds symbol index on every indexed file, which can be inefficient. This patch makes it only rebuild symbol index periodically. As the rebuild no longer happens too often, we could also build more efficient dex index. Reviewers: ilya-biryukov, kadircet Reviewed By: kadircet Subscribers: dblaikie, MaskRay, jkorous, arphaman, jfb, cfe-commits Differential Revision: https://reviews.llvm.org/D55770 Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/clangd/index/Background.cpp clang-tools-extra/trunk/clangd/index/Background.h clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp clang-tools-extra/trunk/test/clangd/background-index.test clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=349496&r1=349495&r2=349496&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Dec 18 07:39:33 2018 @@ -139,7 +139,8 @@ ClangdServer::ClangdServer(const GlobalC if (Opts.BackgroundIndex) { BackgroundIdx = llvm::make_unique<BackgroundIndex>( Context::current().clone(), ResourceDir, FSProvider, CDB, - BackgroundIndexStorage::createDiskBackedStorageFactory()); + BackgroundIndexStorage::createDiskBackedStorageFactory(), + Opts.BackgroundIndexRebuildPeriodMs); AddIndex(BackgroundIdx.get()); } if (DynamicIdx) Modified: clang-tools-extra/trunk/clangd/ClangdServer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=349496&r1=349495&r2=349496&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Dec 18 07:39:33 2018 @@ -85,6 +85,10 @@ public: /// If true, ClangdServer automatically indexes files in the current project /// on background threads. The index is stored in the project root. bool BackgroundIndex = false; + /// If set to non-zero, the background index rebuilds the symbol index + /// periodically every BuildIndexPeriodMs milliseconds; otherwise, the + /// symbol index will be updated for each indexed file. + size_t BackgroundIndexRebuildPeriodMs = 0; /// If set, use this index to augment code completion results. SymbolIndex *StaticIndex = nullptr; Modified: clang-tools-extra/trunk/clangd/index/Background.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=349496&r1=349495&r2=349496&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Background.cpp (original) +++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Dec 18 07:39:33 2018 @@ -27,11 +27,13 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/SHA1.h" +#include <chrono> #include <memory> #include <numeric> #include <queue> #include <random> #include <string> +#include <thread> using namespace llvm; namespace clang { @@ -125,10 +127,13 @@ createFileFilter(const llvm::StringMap<F BackgroundIndex::BackgroundIndex( Context BackgroundContext, StringRef ResourceDir, const FileSystemProvider &FSProvider, const GlobalCompilationDatabase &CDB, - BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize) + BackgroundIndexStorage::Factory IndexStorageFactory, + size_t BuildIndexPeriodMs, size_t ThreadPoolSize) : SwapIndex(make_unique<MemIndex>()), ResourceDir(ResourceDir), FSProvider(FSProvider), CDB(CDB), BackgroundContext(std::move(BackgroundContext)), + BuildIndexPeriodMs(BuildIndexPeriodMs), + SymbolsUpdatedSinceLastIndex(false), IndexStorageFactory(std::move(IndexStorageFactory)), CommandsChanged( CDB.watch([&](const std::vector<std::string> &ChangedFiles) { @@ -138,6 +143,11 @@ BackgroundIndex::BackgroundIndex( assert(this->IndexStorageFactory && "Storage factory can not be null!"); while (ThreadPoolSize--) ThreadPool.emplace_back([this] { run(); }); + if (BuildIndexPeriodMs > 0) { + log("BackgroundIndex: build symbol index periodically every {0} ms.", + BuildIndexPeriodMs); + ThreadPool.emplace_back([this] { buildIndex(); }); + } } BackgroundIndex::~BackgroundIndex() { @@ -148,10 +158,12 @@ BackgroundIndex::~BackgroundIndex() { void BackgroundIndex::stop() { { - std::lock_guard<std::mutex> Lock(QueueMu); + std::lock_guard<std::mutex> QueueLock(QueueMu); + std::lock_guard<std::mutex> IndexLock(IndexMu); ShouldStop = true; } QueueCV.notify_all(); + IndexCV.notify_all(); } void BackgroundIndex::run() { @@ -332,6 +344,30 @@ void BackgroundIndex::update(StringRef M } } +void BackgroundIndex::buildIndex() { + assert(BuildIndexPeriodMs > 0); + while (true) { + { + std::unique_lock<std::mutex> Lock(IndexMu); + if (ShouldStop) // Avoid waiting if stopped. + break; + // Wait until this is notified to stop or `BuildIndexPeriodMs` has past. + IndexCV.wait_for(Lock, std::chrono::milliseconds(BuildIndexPeriodMs)); + if (ShouldStop) // Avoid rebuilding index if stopped. + break; + } + if (!SymbolsUpdatedSinceLastIndex.exchange(false)) + continue; + // There can be symbol update right after the flag is reset above and before + // index is rebuilt below. The new index would contain the updated symbols + // but the flag would still be true. This is fine as we would simply run an + // extra index build. + reset( + IndexedSymbols.buildIndex(IndexType::Heavy, DuplicateHandling::Merge)); + log("BackgroundIndex: rebuilt symbol index."); + } +} + Error BackgroundIndex::index(tooling::CompileCommand Cmd, BackgroundIndexStorage *IndexStorage) { trace::Span Tracer("BackgroundIndex"); @@ -362,7 +398,7 @@ Error BackgroundIndex::index(tooling::Co DigestsSnapshot = IndexedFileDigests; } - log("Indexing {0}", Cmd.Filename, toHex(Hash)); + log("Indexing {0} (digest:={1})", Cmd.Filename, toHex(Hash)); ParseInputs Inputs; Inputs.FS = std::move(FS); Inputs.FS->setCurrentWorkingDirectory(Cmd.Directory); @@ -424,10 +460,11 @@ Error BackgroundIndex::index(tooling::Co IndexedFileDigests[AbsolutePath] = Hash; } - // FIXME: this should rebuild once-in-a-while, not after every file. - // At that point we should use Dex, too. - vlog("Rebuilding automatic index"); - reset(IndexedSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge)); + if (BuildIndexPeriodMs > 0) + SymbolsUpdatedSinceLastIndex = true; + else + reset( + IndexedSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge)); return Error::success(); } Modified: clang-tools-extra/trunk/clangd/index/Background.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.h?rev=349496&r1=349495&r2=349496&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Background.h (original) +++ clang-tools-extra/trunk/clangd/index/Background.h Tue Dec 18 07:39:33 2018 @@ -21,8 +21,10 @@ #include "llvm/ADT/StringMap.h" #include "llvm/Support/SHA1.h" #include "llvm/Support/Threading.h" +#include <atomic> #include <condition_variable> #include <deque> +#include <mutex> #include <string> #include <thread> #include <vector> @@ -63,11 +65,15 @@ public: // FIXME: it should watch for changes to files on disk. class BackgroundIndex : public SwapIndex { public: + /// If BuildIndexPeriodMs is greater than 0, the symbol index will only be + /// rebuilt periodically (one per \p BuildIndexPeriodMs); otherwise, index is + /// rebuilt for each indexed file. // FIXME: resource-dir injection should be hoisted somewhere common. BackgroundIndex(Context BackgroundContext, llvm::StringRef ResourceDir, const FileSystemProvider &, const GlobalCompilationDatabase &CDB, BackgroundIndexStorage::Factory IndexStorageFactory, + size_t BuildIndexPeriodMs = 0, size_t ThreadPoolSize = llvm::hardware_concurrency()); ~BackgroundIndex(); // Blocks while the current task finishes. @@ -101,6 +107,11 @@ private: // index state llvm::Error index(tooling::CompileCommand, BackgroundIndexStorage *IndexStorage); + void buildIndex(); // Rebuild index periodically every BuildIndexPeriodMs. + const size_t BuildIndexPeriodMs; + std::atomic<bool> SymbolsUpdatedSinceLastIndex; + std::mutex IndexMu; + std::condition_variable IndexCV; FileSymbols IndexedSymbols; llvm::StringMap<FileDigest> IndexedFileDigests; // Key is absolute file path. Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=349496&r1=349495&r2=349496&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original) +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Dec 18 07:39:33 2018 @@ -170,6 +170,14 @@ static cl::opt<bool> EnableBackgroundInd "Experimental"), cl::init(false), cl::Hidden); +static cl::opt<int> BackgroundIndexRebuildPeriod( + "background-index-rebuild-period", + cl::desc( + "If set to non-zero, the background index rebuilds the symbol index " + "periodically every X milliseconds; otherwise, the " + "symbol index will be updated for each indexed file."), + cl::init(5000), cl::Hidden); + enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs }; static cl::opt<CompileArgsFrom> CompileArgsFrom( "compile_args_from", cl::desc("The source of compile commands"), @@ -352,6 +360,7 @@ int main(int argc, char *argv[]) { Opts.BuildDynamicSymbolIndex = EnableIndex; Opts.HeavyweightDynamicSymbolIndex = UseDex; Opts.BackgroundIndex = EnableBackgroundIndex; + Opts.BackgroundIndexRebuildPeriodMs = BackgroundIndexRebuildPeriod; std::unique_ptr<SymbolIndex> StaticIdx; std::future<void> AsyncIndexLoad; // Block exit while loading the index. if (EnableIndex && !IndexFile.empty()) { Modified: clang-tools-extra/trunk/test/clangd/background-index.test URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/background-index.test?rev=349496&r1=349495&r2=349496&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clangd/background-index.test (original) +++ clang-tools-extra/trunk/test/clangd/background-index.test Tue Dec 18 07:39:33 2018 @@ -10,7 +10,7 @@ # We're editing bar.cpp, which includes foo.h. # foo() is declared in foo.h and defined in foo.cpp. # The background index should allow us to go-to-definition on foo(). -# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc +# RUN: clangd -background-index -background-index-rebuild-period=0 -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc # Test that the index is writing files in the expected location. # RUN: ls %t/.clangd-index/foo.cpp.*.idx Modified: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp?rev=349496&r1=349495&r2=349496&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Tue Dec 18 07:39:33 2018 @@ -2,11 +2,14 @@ #include "TestFS.h" #include "index/Background.h" #include "llvm/Support/ScopedPrinter.h" +#include "llvm/Support/Threading.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include <thread> using testing::_; using testing::AllOf; +using testing::ElementsAre; using testing::Not; using testing::UnorderedElementsAre; @@ -240,5 +243,39 @@ TEST_F(BackgroundIndexTest, DirectInclud EmptyIncludeNode()); } +TEST_F(BackgroundIndexTest, PeriodicalIndex) { + MockFSProvider FS; + llvm::StringMap<std::string> Storage; + size_t CacheHits = 0; + MemoryShardStorage MSS(Storage, CacheHits); + OverlayCDB CDB(/*Base=*/nullptr); + BackgroundIndex Idx( + Context::empty(), "", FS, CDB, [&](llvm::StringRef) { return &MSS; }, + /*BuildIndexPeriodMs=*/10); + + FS.Files[testPath("root/A.cc")] = "#include \"A.h\""; + + tooling::CompileCommand Cmd; + FS.Files[testPath("root/A.h")] = "class X {};"; + Cmd.Filename = testPath("root/A.cc"); + Cmd.CommandLine = {"clang++", Cmd.Filename}; + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); + + ASSERT_TRUE(Idx.blockUntilIdleForTest()); + EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre()); + std::this_thread::sleep_for(std::chrono::milliseconds(15)); + EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("X"))); + + FS.Files[testPath("root/A.h")] = "class Y {};"; + FS.Files[testPath("root/A.cc")] += " "; // Force reindex the file. + Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")}; + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); + + ASSERT_TRUE(Idx.blockUntilIdleForTest()); + EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("X"))); + std::this_thread::sleep_for(std::chrono::milliseconds(15)); + EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("Y"))); +} + } // namespace clangd } // namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
