Author: sammccall Date: Tue Jul 16 03:17:06 2019 New Revision: 366199 URL: http://llvm.org/viewvc/llvm-project?rev=366199&view=rev Log: [clangd] Don't rebuild background index until we indexed one TU per thread.
Summary: This increases the odds that the boosted file (cpp file matching header) will be ready. (It always enqueues first, so it'll be present unless another thread indexes *two* files before the first thread indexes one.) Reviewers: kadircet Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D64682 Modified: clang-tools-extra/trunk/clangd/index/Background.cpp clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp 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=366199&r1=366198&r2=366199&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Background.cpp (original) +++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Jul 16 03:17:06 2019 @@ -127,7 +127,7 @@ BackgroundIndex::BackgroundIndex( BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize) : SwapIndex(llvm::make_unique<MemIndex>()), FSProvider(FSProvider), CDB(CDB), BackgroundContext(std::move(BackgroundContext)), - Rebuilder(this, &IndexedSymbols), + Rebuilder(this, &IndexedSymbols, ThreadPoolSize), IndexStorageFactory(std::move(IndexStorageFactory)), CommandsChanged( CDB.watch([&](const std::vector<std::string> &ChangedFiles) { Modified: clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h?rev=366199&r1=366198&r2=366199&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h (original) +++ clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h Tue Jul 16 03:17:06 2019 @@ -16,6 +16,7 @@ #include "index/FileIndex.h" #include "index/Index.h" +#include "llvm/Support/Threading.h" namespace clang { namespace clangd { @@ -45,12 +46,9 @@ namespace clangd { // This class is exposed in the header so it can be tested. class BackgroundIndexRebuilder { public: - // Thresholds for rebuilding as TUs get indexed. - static constexpr unsigned TUsBeforeFirstBuild = 5; - static constexpr unsigned TUsBeforeRebuild = 100; - - BackgroundIndexRebuilder(SwapIndex *Target, FileSymbols *Source) - : Target(Target), Source(Source) {} + BackgroundIndexRebuilder(SwapIndex *Target, FileSymbols *Source, + unsigned Threads) + : TUsBeforeFirstBuild(Threads), Target(Target), Source(Source) {} // Called to indicate a TU has been indexed. // May rebuild, if enough TUs have been indexed. @@ -71,6 +69,10 @@ public: // Ensures we won't start any more rebuilds. void shutdown(); + // Thresholds for rebuilding as TUs get indexed. + const unsigned TUsBeforeFirstBuild; // Typically one per worker thread. + const unsigned TUsBeforeRebuild = 100; + private: // Run Check under the lock, and rebuild if it returns true. void maybeRebuild(const char *Reason, std::function<bool()> Check); Modified: clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp?rev=366199&r1=366198&r2=366199&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp Tue Jul 16 03:17:06 2019 @@ -575,7 +575,8 @@ TEST_F(BackgroundIndexTest, CmdLineHash) class BackgroundIndexRebuilderTest : public testing::Test { protected: BackgroundIndexRebuilderTest() - : Target(llvm::make_unique<MemIndex>()), Rebuilder(&Target, &Source) { + : Target(llvm::make_unique<MemIndex>()), + Rebuilder(&Target, &Source, /*Threads=*/10) { // Prepare FileSymbols with TestSymbol in it, for checkRebuild. TestSymbol.ID = SymbolID("foo"); } @@ -610,11 +611,10 @@ protected: }; TEST_F(BackgroundIndexRebuilderTest, IndexingTUs) { - for (unsigned I = 0; I < BackgroundIndexRebuilder::TUsBeforeFirstBuild - 1; - ++I) + for (unsigned I = 0; I < Rebuilder.TUsBeforeFirstBuild - 1; ++I) EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); })); EXPECT_TRUE(checkRebuild([&] { Rebuilder.indexedTU(); })); - for (unsigned I = 0; I < BackgroundIndexRebuilder::TUsBeforeRebuild - 1; ++I) + for (unsigned I = 0; I < Rebuilder.TUsBeforeRebuild - 1; ++I) EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); })); EXPECT_TRUE(checkRebuild([&] { Rebuilder.indexedTU(); })); } @@ -640,7 +640,7 @@ TEST_F(BackgroundIndexRebuilderTest, Loa // No rebuilding for indexed files while loading. Rebuilder.startLoading(); - for (unsigned I = 0; I < 3 * BackgroundIndexRebuilder::TUsBeforeRebuild; ++I) + for (unsigned I = 0; I < 3 * Rebuilder.TUsBeforeRebuild; ++I) EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); })); // But they get indexed when we're done, even if no shards were loaded. EXPECT_TRUE(checkRebuild([&] { Rebuilder.doneLoading(); })); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits