Index: clangd/index/Background.h =================================================================== --- clangd/index/Background.h +++ clangd/index/Background.h @@ -91,10 +91,11 @@ blockUntilIdleForTest(llvm::Optional TimeoutSeconds = 10); private: - /// Given index results from a TU, only update files in \p FilesToUpdate. - /// Also stores new index information on IndexStorage. + /// Given index results from a TU, only update symbols coming from files with + /// different digests than \p DigestsSnapshot. Also stores new index + /// information on IndexStorage. void update(llvm::StringRef MainFile, IndexFileIn Index, - const llvm::StringMap &FilesToUpdate, + const llvm::StringMap &DigestsSnapshot, BackgroundIndexStorage *IndexStorage); // configuration Index: clangd/index/Background.cpp =================================================================== --- clangd/index/Background.cpp +++ clangd/index/Background.cpp @@ -91,12 +91,10 @@ // Creates a filter to not collect index results from files with unchanged // digests. -// \p FileDigests contains file digests for the current indexed files, and all -// changed files will be added to \p FilesToUpdate. +// \p FileDigests contains file digests for the current indexed files. decltype(SymbolCollector::Options::FileFilter) -createFileFilter(const llvm::StringMap &FileDigests, - llvm::StringMap &FilesToUpdate) { - return [&FileDigests, &FilesToUpdate](const SourceManager &SM, FileID FID) { +createFileFilter(const llvm::StringMap &FileDigests) { + return [&FileDigests](const SourceManager &SM, FileID FID) { const auto *F = SM.getFileEntryForID(FID); if (!F) return false; // Skip invalid files. @@ -109,8 +107,6 @@ auto D = FileDigests.find(*AbsPath); if (D != FileDigests.end() && D->second == Digest) return false; // Skip files that haven't changed. - - FilesToUpdate[*AbsPath] = *Digest; return true; }; } @@ -264,22 +260,34 @@ QueueCV.notify_all(); } -/// Given index results from a TU, only update files in \p FilesToUpdate. +/// Given index results from a TU, only update symbols coming from files that +/// are different or missing from than \p DigestsSnapshot. Also stores new index +/// information on IndexStorage. void BackgroundIndex::update(llvm::StringRef MainFile, IndexFileIn Index, - const llvm::StringMap &FilesToUpdate, + const llvm::StringMap &DigestsSnapshot, BackgroundIndexStorage *IndexStorage) { // Partition symbols/references into files. struct File { llvm::DenseSet Symbols; llvm::DenseSet Refs; + FileDigest Digest; }; llvm::StringMap Files; URIToFileCache URICache(MainFile); + for (const auto &IndexIt : *Index.Sources) { + const auto &IGN = IndexIt.getValue(); + const auto AbsPath = URICache.resolve(IGN.URI); + const auto DigestIt = DigestsSnapshot.find(AbsPath); + // File has different contents. + if (DigestIt == DigestsSnapshot.end() || DigestIt->getValue() != IGN.Digest) + Files.try_emplace(AbsPath).first->getValue().Digest = IGN.Digest; + } for (const auto &Sym : *Index.Symbols) { if (Sym.CanonicalDeclaration) { auto DeclPath = URICache.resolve(Sym.CanonicalDeclaration.FileURI); - if (FilesToUpdate.count(DeclPath) != 0) - Files[DeclPath].Symbols.insert(&Sym); + const auto FileIt = Files.find(DeclPath); + if (FileIt != Files.end()) + FileIt->second.Symbols.insert(&Sym); } // For symbols with different declaration and definition locations, we store // the full symbol in both the header file and the implementation file, so @@ -288,16 +296,18 @@ if (Sym.Definition && Sym.Definition.FileURI != Sym.CanonicalDeclaration.FileURI) { auto DefPath = URICache.resolve(Sym.Definition.FileURI); - if (FilesToUpdate.count(DefPath) != 0) - Files[DefPath].Symbols.insert(&Sym); + const auto FileIt = Files.find(DefPath); + if (FileIt != Files.end()) + FileIt->second.Symbols.insert(&Sym); } } llvm::DenseMap RefToIDs; for (const auto &SymRefs : *Index.Refs) { for (const auto &R : SymRefs.second) { auto Path = URICache.resolve(R.Location.FileURI); - if (FilesToUpdate.count(Path) != 0) { - auto &F = Files[Path]; + const auto FileIt = Files.find(Path); + if (FileIt != Files.end()) { + auto &F = FileIt->getValue(); RefToIDs[&R] = SymRefs.first; F.Refs.insert(&R); } @@ -305,18 +315,14 @@ } // Build and store new slabs for each updated file. - for (const auto &I : *Index.Sources) { - std::string Path = URICache.resolve(I.first()); + for (const auto &FileIt : Files) { + llvm::StringRef Path = FileIt.getKey(); SymbolSlab::Builder Syms; RefSlab::Builder Refs; - auto FileIt = Files.find(Path); - if (FileIt != Files.end()) { - auto &F = *FileIt; - for (const auto *S : F.second.Symbols) - Syms.insert(*S); - for (const auto *R : F.second.Refs) - Refs.insert(RefToIDs[R], *R); - } + for (const auto *S : FileIt.second.Symbols) + Syms.insert(*S); + for (const auto *R : FileIt.second.Refs) + Refs.insert(RefToIDs[R], *R); auto SS = llvm::make_unique(std::move(Syms).build()); auto RS = llvm::make_unique(std::move(Refs).build()); auto IG = llvm::make_unique( @@ -335,7 +341,7 @@ } { std::lock_guard Lock(DigestsMu); - auto Hash = I.second.Digest; + auto Hash = FileIt.second.Digest; // Skip if file is already up to date. auto DigestIt = IndexedFileDigests.try_emplace(Path); if (!DigestIt.second && DigestIt.first->second == Hash) @@ -410,8 +416,7 @@ "Couldn't build compiler instance"); SymbolCollector::Options IndexOpts; - llvm::StringMap FilesToUpdate; - IndexOpts.FileFilter = createFileFilter(DigestsSnapshot, FilesToUpdate); + IndexOpts.FileFilter = createFileFilter(DigestsSnapshot); IndexFileIn Index; auto Action = createStaticIndexingAction( IndexOpts, [&](SymbolSlab S) { Index.Symbols = std::move(S); }, @@ -448,7 +453,7 @@ SPAN_ATTACH(Tracer, "refs", int(Index.Refs->numRefs())); SPAN_ATTACH(Tracer, "sources", int(Index.Sources->size())); - update(AbsolutePath, std::move(Index), FilesToUpdate, IndexStorage); + update(AbsolutePath, std::move(Index), DigestsSnapshot, IndexStorage); if (BuildIndexPeriodMs > 0) SymbolsUpdatedSinceLastIndex = true; Index: unittests/clangd/BackgroundIndexTests.cpp =================================================================== --- unittests/clangd/BackgroundIndexTests.cpp +++ unittests/clangd/BackgroundIndexTests.cpp @@ -351,11 +351,82 @@ EXPECT_EQ(CacheHits, 2U); // Check both A.cc and A.h loaded from cache. // Check if the new symbol has arrived. - auto ShardSource = MSS.loadShard(testPath("root/A.cc")); + ShardHeader = MSS.loadShard(testPath("root/A.h")); EXPECT_NE(ShardHeader, nullptr); + EXPECT_THAT(*ShardHeader->Symbols, Contains(Named("A_CCnew"))); + auto ShardSource = MSS.loadShard(testPath("root/A.cc")); + EXPECT_NE(ShardSource, nullptr); EXPECT_THAT(*ShardSource->Symbols, Contains(AllOf(Named("f_b"), Declared(), Defined()))); } +TEST_F(BackgroundIndexTest, ShardStorageEmptyFile) { + MockFSProvider FS; + FS.Files[testPath("root/A.h")] = R"cpp( + void common(); + void f_b(); + class A_CC {}; + )cpp"; + FS.Files[testPath("root/B.h")] = R"cpp( + #include "A.h" + )cpp"; + FS.Files[testPath("root/A.cc")] = + "#include \"B.h\"\nvoid g() { (void)common; }"; + + llvm::StringMap Storage; + size_t CacheHits = 0; + MemoryShardStorage MSS(Storage, CacheHits); + + tooling::CompileCommand Cmd; + Cmd.Filename = testPath("root/A.cc"); + Cmd.Directory = testPath("root"); + Cmd.CommandLine = {"clang++", testPath("root/A.cc")}; + // Check that A.cc, A.h and B.h has been stored. + { + OverlayCDB CDB(/*Base=*/nullptr); + BackgroundIndex Idx(Context::empty(), "", FS, CDB, + [&](llvm::StringRef) { return &MSS; }); + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); + ASSERT_TRUE(Idx.blockUntilIdleForTest()); + } + EXPECT_THAT(Storage.keys(), + UnorderedElementsAre(testPath("root/A.cc"), testPath("root/A.h"), + testPath("root/B.h"))); + auto ShardHeader = MSS.loadShard(testPath("root/B.h")); + EXPECT_NE(ShardHeader, nullptr); + EXPECT_TRUE(ShardHeader->Symbols->empty()); + + // Check that A.cc, A.h and B.h has been loaded. + { + CacheHits = 0; + OverlayCDB CDB(/*Base=*/nullptr); + BackgroundIndex Idx(Context::empty(), "", FS, CDB, + [&](llvm::StringRef) { return &MSS; }); + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); + ASSERT_TRUE(Idx.blockUntilIdleForTest()); + } + EXPECT_EQ(CacheHits, 3U); + + // Update B.h to contain some symbols. + FS.Files[testPath("root/B.h")] = R"cpp( + #include "A.h" + void new_func(); + )cpp"; + // Check that B.h has been stored with new contents. + { + CacheHits = 0; + OverlayCDB CDB(/*Base=*/nullptr); + BackgroundIndex Idx(Context::empty(), "", FS, CDB, + [&](llvm::StringRef) { return &MSS; }); + CDB.setCompileCommand(testPath("root/A.cc"), Cmd); + ASSERT_TRUE(Idx.blockUntilIdleForTest()); + } + EXPECT_EQ(CacheHits, 3U); + ShardHeader = MSS.loadShard(testPath("root/B.h")); + EXPECT_NE(ShardHeader, nullptr); + EXPECT_THAT(*ShardHeader->Symbols, + Contains(AllOf(Named("new_func"), Declared(), Not(Defined())))); +} + } // namespace clangd } // namespace clang