<div dir="ltr">Hi Azhar, D64980 should fix the problem. I am reverting your revert while adding the fix in r366559.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 19, 2019 at 11:29 AM Azhar Mohammed <<a href="mailto:azhar@apple.com">azhar@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;">Reverted in r366551. <span><div><br></div><div><br></div><div><div> Revert r366458, r366467 and r366468</div><div><br></div><div> r366458 is causing test failures. r366467 and r366468 had to be reverted as</div><div> they were casuing conflict while reverting r366458.</div><div><br></div><div> r366468 [clangd] Remove dead code from BackgroundIndex</div><div> r366467 [clangd] BackgroundIndex stores shards to the closest project</div><div> r366458 [clangd] Refactor background-index shard loading</div></div><div><br></div></span><span><div><div><div><blockquote type="cite"><div>On Jul 18, 2019, at 6:21 PM, Azhar Mohammed <<a href="mailto:azhar@apple.com" target="_blank">azhar@apple.com</a>> wrote:</div><br class="gmail-m_1817645778349330322Apple-interchange-newline"><div><div style="overflow-wrap: break-word;">Hi Kadir<div><br></div><div>This change is causing test failures, can you please look into it. Refer to <a href="http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/58104/testReport/" target="_blank">http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/58104/testReport/</a>. </div><div><br></div><div><pre style="box-sizing:border-box;white-space:pre-wrap;margin-top:0px;margin-bottom:0px;color:rgb(51,51,51);font-size:15px">Assertion failed: (TUsIt != FileToTU.end() && "No TU registered for the shard"), function takeResult, file /Users/buildslave/jenkins/workspace/clang-stage1-configure-RA/llvm/tools/clang/tools/extra/clangd/index/BackgroundIndexLoader.cpp, line 131.</pre><div><br></div><div><br></div><div><pre style="box-sizing:border-box;white-space:pre-wrap;margin-top:0px;margin-bottom:0px;color:rgb(51,51,51);font-size:15px" class="gmail-m_1817645778349330322console-output">Failing Tests (10):
Clangd :: did-change-configuration-params.test
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.CmdLineHash
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.DirectIncludesTest
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.IndexTwoFiles
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.NoCrashOnErrorFile
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.NoDotsInAbsPath
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.ShardStorageEmptyFile
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.ShardStorageLoad
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.ShardStorageTest
Clangd Unit Tests :: ./ClangdTests/BackgroundIndexTest.UncompilableFiles</pre><div><br></div></div><div><br><blockquote type="cite"><div>On Jul 18, 2019, at 9:25 AM, Kadir Cetinkaya via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:</div><br class="gmail-m_1817645778349330322Apple-interchange-newline"><div><div>Author: kadircet<br>Date: Thu Jul 18 09:25:36 2019<br>New Revision: 366458<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=366458&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=366458&view=rev</a><br>Log:<br>[clangd] Refactor background-index shard loading<br><br>Reviewers: sammccall<br><br>Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits<br><br>Tags: #clang<br><br>Differential Revision: <a href="https://reviews.llvm.org/D64712" target="_blank">https://reviews.llvm.org/D64712</a><br><br>Added:<br> clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp<br> clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.h<br>Modified:<br> clang-tools-extra/trunk/clangd/CMakeLists.txt<br> clang-tools-extra/trunk/clangd/index/Background.cpp<br> clang-tools-extra/trunk/clangd/index/Background.h<br> clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp<br> clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h<br> clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp<br><br>Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt<br>URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=366458&r1=366457&r2=366458&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=366458&r1=366457&r2=366458&view=diff</a><br>==============================================================================<br>--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)<br>+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Jul 18 09:25:36 2019<br>@@ -73,6 +73,7 @@ add_clang_library(clangDaemon<br> XRefs.cpp<br><br> index/Background.cpp<br>+ index/BackgroundIndexLoader.cpp<br> index/BackgroundIndexStorage.cpp<br> index/BackgroundQueue.cpp<br> index/BackgroundRebuild.cpp<br><br>Modified: clang-tools-extra/trunk/clangd/index/Background.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=366458&r1=366457&r2=366458&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=366458&r1=366457&r2=366458&view=diff</a><br>==============================================================================<br>--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)<br>+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Jul 18 09:25:36 2019<br>@@ -10,6 +10,7 @@<br> #include "ClangdUnit.h"<br> #include "Compiler.h"<br> #include "Context.h"<br>+#include "FSProvider.h"<br> #include "Headers.h"<br> #include "Logger.h"<br> #include "Path.h"<br>@@ -18,6 +19,7 @@<br> #include "Threading.h"<br> #include "Trace.h"<br> #include "URI.h"<br>+#include "index/BackgroundIndexLoader.h"<br> #include "index/FileIndex.h"<br> #include "index/IndexAction.h"<br> #include "index/MemIndex.h"<br>@@ -28,6 +30,8 @@<br> #include "clang/Basic/SourceLocation.h"<br> #include "clang/Basic/SourceManager.h"<br> #include "clang/Driver/Types.h"<br>+#include "llvm/ADT/ArrayRef.h"<br>+#include "llvm/ADT/DenseSet.h"<br> #include "llvm/ADT/Hashing.h"<br> #include "llvm/ADT/STLExtras.h"<br> #include "llvm/ADT/ScopeExit.h"<br>@@ -42,6 +46,7 @@<br> #include <atomic><br> #include <chrono><br> #include <condition_variable><br>+#include <cstddef><br> #include <memory><br> #include <mutex><br> #include <numeric><br>@@ -49,6 +54,8 @@<br> #include <random><br> #include <string><br> #include <thread><br>+#include <utility><br>+#include <vector><br><br> namespace clang {<br> namespace clangd {<br>@@ -119,6 +126,18 @@ llvm::SmallString<128> getAbsolutePath(c<br> }<br> return AbsolutePath;<br> }<br>+<br>+bool shardIsStale(const LoadedShard &LS, llvm::vfs::FileSystem *FS) {<br>+ auto Buf = FS->getBufferForFile(LS.AbsolutePath);<br>+ if (!Buf) {<br>+ elog("Background-index: Couldn't read {0} to validate stored index: {1}",<br>+ LS.AbsolutePath, Buf.getError().message());<br>+ // There is no point in indexing an unreadable file.<br>+ return false;<br>+ }<br>+ return digest(Buf->get()->getBuffer()) != LS.Digest;<br>+}<br>+<br> } // namespace<br><br> BackgroundIndex::BackgroundIndex(<br>@@ -156,7 +175,7 @@ BackgroundQueue::Task BackgroundIndex::c<br> log("Enqueueing {0} commands for indexing", ChangedFiles.size());<br> SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));<br><br>- auto NeedsReIndexing = loadShards(std::move(ChangedFiles));<br>+ auto NeedsReIndexing = loadProject(std::move(ChangedFiles));<br> // Run indexing for files that need to be updated.<br> std::shuffle(NeedsReIndexing.begin(), NeedsReIndexing.end(),<br> std::mt19937(std::random_device{}()));<br>@@ -431,169 +450,77 @@ llvm::Error BackgroundIndex::index(tooli<br> return llvm::Error::success();<br> }<br><br>-std::vector<BackgroundIndex::Source><br>-BackgroundIndex::loadShard(const tooling::CompileCommand &Cmd,<br>- BackgroundIndexStorage *IndexStorage,<br>- llvm::StringSet<> &LoadedShards) {<br>- struct ShardInfo {<br>- std::string AbsolutePath;<br>- std::unique_ptr<IndexFileIn> Shard;<br>- FileDigest Digest = {};<br>- bool CountReferences = false;<br>- bool HadErrors = false;<br>- };<br>- std::vector<ShardInfo> IntermediateSymbols;<br>- // Make sure we don't have duplicate elements in the queue. Keys are absolute<br>- // paths.<br>- llvm::StringSet<> InQueue;<br>- auto FS = FSProvider.getFileSystem();<br>- // Dependencies of this TU, paired with the information about whether they<br>- // need to be re-indexed or not.<br>- std::vector<Source> Dependencies;<br>- std::queue<Source> ToVisit;<br>- std::string AbsolutePath = getAbsolutePath(Cmd).str();<br>- // Up until we load the shard related to a dependency it needs to be<br>- // re-indexed.<br>- ToVisit.emplace(AbsolutePath, true);<br>- InQueue.insert(AbsolutePath);<br>- // Goes over each dependency.<br>- while (!ToVisit.empty()) {<br>- Dependencies.push_back(std::move(ToVisit.front()));<br>- // Dependencies is not modified during the rest of the loop, so it is safe<br>- // to keep the reference.<br>- auto &CurDependency = Dependencies.back();<br>- ToVisit.pop();<br>- // If we have already seen this shard before(either loaded or failed) don't<br>- // re-try again. Since the information in the shard won't change from one TU<br>- // to another.<br>- if (!LoadedShards.try_emplace(CurDependency.Path).second) {<br>- // If the dependency needs to be re-indexed, first occurence would already<br>- // have detected that, so we don't need to issue it again.<br>- CurDependency.NeedsReIndexing = false;<br>- continue;<br>- }<br>-<br>- auto Shard = IndexStorage->loadShard(CurDependency.Path);<br>- if (!Shard || !Shard->Sources) {<br>- // File will be returned as requiring re-indexing to caller.<br>- vlog("Failed to load shard: {0}", CurDependency.Path);<br>- continue;<br>- }<br>- // These are the edges in the include graph for current dependency.<br>- for (const auto &I : *Shard->Sources) {<br>- auto U = URI::parse(I.getKey());<br>- if (!U)<br>- continue;<br>- auto AbsolutePath = URI::resolve(*U, CurDependency.Path);<br>- if (!AbsolutePath)<br>- continue;<br>- // Add file as dependency if haven't seen before.<br>- if (InQueue.try_emplace(*AbsolutePath).second)<br>- ToVisit.emplace(*AbsolutePath, true);<br>- // The node contains symbol information only for current file, the rest is<br>- // just edges.<br>- if (*AbsolutePath != CurDependency.Path)<br>- continue;<br>+// Restores shards for \p MainFiles from index storage. Then checks staleness of<br>+// those shards and returns a list of TUs that needs to be indexed to update<br>+// staleness.<br>+std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>><br>+BackgroundIndex::loadProject(std::vector<std::string> MainFiles) {<br>+ std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>><br>+ NeedsReIndexing;<br><br>- // We found source file info for current dependency.<br>- assert(I.getValue().Digest != FileDigest{{0}} && "Digest is empty?");<br>- ShardInfo SI;<br>- SI.AbsolutePath = CurDependency.Path;<br>- SI.Shard = std::move(Shard);<br>- SI.Digest = I.getValue().Digest;<br>- SI.CountReferences =<br>- I.getValue().Flags & IncludeGraphNode::SourceFlag::IsTU;<br>- SI.HadErrors =<br>- I.getValue().Flags & IncludeGraphNode::SourceFlag::HadErrors;<br>- IntermediateSymbols.push_back(std::move(SI));<br>- // Check if the source needs re-indexing.<br>- // Get the digest, skip it if file doesn't exist.<br>- auto Buf = FS->getBufferForFile(CurDependency.Path);<br>- if (!Buf) {<br>- elog("Couldn't get buffer for file: {0}: {1}", CurDependency.Path,<br>- Buf.getError().message());<br>- continue;<br>- }<br>- // If digests match then dependency doesn't need re-indexing.<br>- // FIXME: Also check for dependencies(sources) of this shard and compile<br>- // commands for cache invalidation.<br>- CurDependency.NeedsReIndexing =<br>- digest(Buf->get()->getBuffer()) != I.getValue().Digest;<br>- }<br>- }<br>- // Load shard information into background-index.<br>+ Rebuilder.startLoading();<br>+ // Load shards for all of the mainfiles.<br>+ const std::vector<LoadedShard> Result =<br>+ loadIndexShards(MainFiles, IndexStorageFactory, CDB);<br>+ size_t LoadedShards = 0;<br> {<br>+ // Update in-memory state.<br> std::lock_guard<std::mutex> Lock(ShardVersionsMu);<br>- // This can override a newer version that is added in another thread,<br>- // if this thread sees the older version but finishes later. This<br>- // should be rare in practice.<br>- for (const ShardInfo &SI : IntermediateSymbols) {<br>+ for (auto &LS : Result) {<br>+ if (!LS.Shard)<br>+ continue;<br> auto SS =<br>- SI.Shard->Symbols<br>- ? llvm::make_unique<SymbolSlab>(std::move(*SI.Shard->Symbols))<br>+ LS.Shard->Symbols<br>+ ? llvm::make_unique<SymbolSlab>(std::move(*LS.Shard->Symbols))<br> : nullptr;<br>- auto RS = SI.Shard->Refs<br>- ? llvm::make_unique<RefSlab>(std::move(*SI.Shard->Refs))<br>+ auto RS = LS.Shard->Refs<br>+ ? llvm::make_unique<RefSlab>(std::move(*LS.Shard->Refs))<br> : nullptr;<br> auto RelS =<br>- SI.Shard->Relations<br>- ? llvm::make_unique<RelationSlab>(std::move(*SI.Shard->Relations))<br>+ LS.Shard->Relations<br>+ ? llvm::make_unique<RelationSlab>(std::move(*LS.Shard->Relations))<br> : nullptr;<br>- ShardVersion &SV = ShardVersions[SI.AbsolutePath];<br>- SV.Digest = SI.Digest;<br>- SV.HadErrors = SI.HadErrors;<br>+ ShardVersion &SV = ShardVersions[LS.AbsolutePath];<br>+ SV.Digest = LS.Digest;<br>+ SV.HadErrors = LS.HadErrors;<br>+ ++LoadedShards;<br><br>- IndexedSymbols.update(SI.AbsolutePath, std::move(SS), std::move(RS),<br>- std::move(RelS), SI.CountReferences);<br>+ IndexedSymbols.update(LS.AbsolutePath, std::move(SS), std::move(RS),<br>+ std::move(RelS), LS.CountReferences);<br> }<br> }<br>- if (!IntermediateSymbols.empty())<br>- Rebuilder.loadedTU();<br>+ Rebuilder.loadedShard(LoadedShards);<br>+ Rebuilder.doneLoading();<br><br>- return Dependencies;<br>-}<br>+ auto FS = FSProvider.getFileSystem();<br>+ llvm::DenseSet<PathRef> TUsToIndex;<br>+ // We'll accept data from stale shards, but ensure the files get reindexed<br>+ // soon.<br>+ for (auto &LS : Result) {<br>+ if (!shardIsStale(LS, FS.get()))<br>+ continue;<br>+ PathRef TUForFile = LS.DependentTU;<br>+ assert(!TUForFile.empty() && "File without a TU!");<br><br>-// Goes over each changed file and loads them from index. Returns the list of<br>-// TUs that had out-of-date/no shards.<br>-std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>><br>-BackgroundIndex::loadShards(std::vector<std::string> ChangedFiles) {<br>- std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>><br>- NeedsReIndexing;<br>- // Keeps track of the files that will be reindexed, to make sure we won't<br>- // re-index same dependencies more than once. Keys are AbsolutePaths.<br>- llvm::StringSet<> FilesToIndex;<br>- // Keeps track of the loaded shards to make sure we don't perform redundant<br>- // disk IO. Keys are absolute paths.<br>- llvm::StringSet<> LoadedShards;<br>- Rebuilder.startLoading();<br>- for (const auto &File : ChangedFiles) {<br>- auto Cmd = CDB.getCompileCommand(File);<br>+ // FIXME: Currently, we simply schedule indexing on a TU whenever any of<br>+ // its dependencies needs re-indexing. We might do it smarter by figuring<br>+ // out a minimal set of TUs that will cover all the stale dependencies.<br>+ // FIXME: Try looking at other TUs if no compile commands are available<br>+ // for this TU, i.e TU was deleted after we performed indexing.<br>+ TUsToIndex.insert(TUForFile);<br>+ }<br>+<br>+ for (PathRef TU : TUsToIndex) {<br>+ auto Cmd = CDB.getCompileCommand(TU);<br> if (!Cmd)<br> continue;<br>-<br> std::string ProjectRoot;<br>- if (auto PI = CDB.getProjectInfo(File))<br>+ if (auto PI = CDB.getProjectInfo(TU))<br> ProjectRoot = std::move(PI->SourceRoot);<br>-<br>- BackgroundIndexStorage *IndexStorage = IndexStorageFactory(ProjectRoot);<br>- auto Dependencies = loadShard(*Cmd, IndexStorage, LoadedShards);<br>- for (const auto &Dependency : Dependencies) {<br>- if (!Dependency.NeedsReIndexing || FilesToIndex.count(Dependency.Path))<br>- continue;<br>- // FIXME: Currently, we simply schedule indexing on a TU whenever any of<br>- // its dependencies needs re-indexing. We might do it smarter by figuring<br>- // out a minimal set of TUs that will cover all the stale dependencies.<br>- vlog("Enqueueing TU {0} because its dependency {1} needs re-indexing.",<br>- Cmd->Filename, Dependency.Path);<br>- NeedsReIndexing.push_back({std::move(*Cmd), IndexStorage});<br>- // Mark all of this TU's dependencies as to-be-indexed so that we won't<br>- // try to re-index those.<br>- for (const auto &Dependency : Dependencies)<br>- FilesToIndex.insert(Dependency.Path);<br>- break;<br>- }<br>+ BackgroundIndexStorage *Storage = IndexStorageFactory(ProjectRoot);<br>+ NeedsReIndexing.emplace_back(std::move(*Cmd), Storage);<br> }<br>- Rebuilder.doneLoading();<br>+<br> return NeedsReIndexing;<br> }<br><br><br>Modified: clang-tools-extra/trunk/clangd/index/Background.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.h?rev=366458&r1=366457&r2=366458&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.h?rev=366458&r1=366457&r2=366458&view=diff</a><br>==============================================================================<br>--- clang-tools-extra/trunk/clangd/index/Background.h (original)<br>+++ clang-tools-extra/trunk/clangd/index/Background.h Thu Jul 18 09:25:36 2019<br>@@ -12,6 +12,7 @@<br> #include "Context.h"<br> #include "FSProvider.h"<br> #include "GlobalCompilationDatabase.h"<br>+#include "Path.h"<br> #include "SourceCode.h"<br> #include "Threading.h"<br> #include "index/BackgroundRebuild.h"<br>@@ -173,20 +174,9 @@ private:<br> std::mutex ShardVersionsMu;<br><br> BackgroundIndexStorage::Factory IndexStorageFactory;<br>- struct Source {<br>- std::string Path;<br>- bool NeedsReIndexing;<br>- Source(llvm::StringRef Path, bool NeedsReIndexing)<br>- : Path(Path), NeedsReIndexing(NeedsReIndexing) {}<br>- };<br>- // Loads the shards for a single TU and all of its dependencies. Returns the<br>- // list of sources and whether they need to be re-indexed.<br>- std::vector<Source> loadShard(const tooling::CompileCommand &Cmd,<br>- BackgroundIndexStorage *IndexStorage,<br>- llvm::StringSet<> &LoadedShards);<br>- // Tries to load shards for the ChangedFiles.<br>+ // Tries to load shards for the MainFiles and their dependencies.<br> std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>><br>- loadShards(std::vector<std::string> ChangedFiles);<br>+ loadProject(std::vector<std::string> MainFiles);<br><br> BackgroundQueue::Task<br> changedFilesTask(const std::vector<std::string> &ChangedFiles);<br><br>Added: clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp?rev=366458&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp?rev=366458&view=auto</a><br>==============================================================================<br>--- clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp (added)<br>+++ clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp Thu Jul 18 09:25:36 2019<br>@@ -0,0 +1,153 @@<br>+//===-- BackgroundIndexLoader.cpp - ---------------------------------------===//<br>+//<br>+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.<br>+// See <a href="https://llvm.org/LICENSE.txt" target="_blank">https://llvm.org/LICENSE.txt</a> for license information.<br>+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception<br>+//<br>+//===----------------------------------------------------------------------===//<br>+<br>+#include "index/BackgroundIndexLoader.h"<br>+#include "GlobalCompilationDatabase.h"<br>+#include "Logger.h"<br>+#include "Path.h"<br>+#include "index/Background.h"<br>+#include "llvm/ADT/DenseMap.h"<br>+#include "llvm/ADT/DenseSet.h"<br>+#include "llvm/ADT/SmallString.h"<br>+#include "llvm/ADT/StringMap.h"<br>+#include "llvm/Support/Path.h"<br>+#include <string><br>+#include <utility><br>+#include <vector><br>+<br>+namespace clang {<br>+namespace clangd {<br>+namespace {<br>+<br>+llvm::Optional<Path> uriToAbsolutePath(llvm::StringRef URI, PathRef HintPath) {<br>+ auto U = URI::parse(URI);<br>+ if (!U)<br>+ return llvm::None;<br>+ auto AbsolutePath = URI::resolve(*U, HintPath);<br>+ if (!AbsolutePath)<br>+ return llvm::None;<br>+ return *AbsolutePath;<br>+}<br>+<br>+/// A helper class to cache BackgroundIndexStorage operations and keep the<br>+/// inverse dependency mapping.<br>+class BackgroundIndexLoader {<br>+public:<br>+ /// Load the shards for \p MainFile and all of its dependencies.<br>+ void load(PathRef MainFile, BackgroundIndexStorage *Storage);<br>+<br>+ /// Consumes the loader and returns all shards.<br>+ std::vector<LoadedShard> takeResult() &&;<br>+<br>+private:<br>+ /// Returns the Shard for \p StartSourceFile from cache or loads it from \p<br>+ /// Storage. Also returns paths for dependencies of \p StartSourceFile if it<br>+ /// wasn't cached yet.<br>+ std::pair<const LoadedShard &, std::vector<Path>><br>+ loadShard(PathRef StartSourceFile, BackgroundIndexStorage *Storage);<br>+<br>+ /// Cache for Storage lookups.<br>+ llvm::StringMap<LoadedShard> LoadedShards;<br>+<br>+ /// References are into the AbsolutePaths in LoadedShards.<br>+ llvm::DenseMap<PathRef, PathRef> FileToTU;<br>+};<br>+<br>+std::pair<const LoadedShard &, std::vector<Path>><br>+BackgroundIndexLoader::loadShard(PathRef StartSourceFile,<br>+ BackgroundIndexStorage *Storage) {<br>+ auto It = LoadedShards.try_emplace(StartSourceFile);<br>+ LoadedShard &LS = It.first->getValue();<br>+ std::vector<Path> Edges = {};<br>+ // Return the cached shard.<br>+ if (!It.second)<br>+ return {LS, Edges};<br>+<br>+ LS.AbsolutePath = StartSourceFile.str();<br>+ auto Shard = Storage->loadShard(StartSourceFile);<br>+ if (!Shard || !Shard->Sources) {<br>+ vlog("Failed to load shard: {0}", StartSourceFile);<br>+ return {LS, Edges};<br>+ }<br>+<br>+ LS.Shard = std::move(Shard);<br>+ for (const auto &It : *LS.Shard->Sources) {<br>+ auto AbsPath = uriToAbsolutePath(It.getKey(), StartSourceFile);<br>+ if (!AbsPath)<br>+ continue;<br>+ // A shard contains only edges for non main-file sources.<br>+ if (*AbsPath != StartSourceFile) {<br>+ Edges.push_back(*AbsPath);<br>+ continue;<br>+ }<br>+<br>+ // Fill in shard metadata.<br>+ const IncludeGraphNode &IGN = It.getValue();<br>+ LS.Digest = IGN.Digest;<br>+ LS.CountReferences = IGN.Flags & IncludeGraphNode::SourceFlag::IsTU;<br>+ LS.HadErrors = IGN.Flags & IncludeGraphNode::SourceFlag::HadErrors;<br>+ }<br>+ assert(LS.Digest != FileDigest{{0}} && "Digest is empty?");<br>+ return {LS, Edges};<br>+}<br>+<br>+void BackgroundIndexLoader::load(PathRef MainFile,<br>+ BackgroundIndexStorage *Storage) {<br>+ llvm::StringSet<> InQueue;<br>+ // Following containers points to strings inside InQueue.<br>+ std::queue<PathRef> ToVisit;<br>+ InQueue.insert(MainFile);<br>+ ToVisit.push(MainFile);<br>+<br>+ while (!ToVisit.empty()) {<br>+ PathRef SourceFile = ToVisit.front();<br>+ ToVisit.pop();<br>+<br>+ auto ShardAndEdges = loadShard(SourceFile, Storage);<br>+ FileToTU[ShardAndEdges.first.AbsolutePath] = MainFile;<br>+ for (PathRef Edge : ShardAndEdges.second) {<br>+ auto It = InQueue.insert(Edge);<br>+ if (It.second)<br>+ ToVisit.push(It.first->getKey());<br>+ }<br>+ }<br>+}<br>+<br>+std::vector<LoadedShard> BackgroundIndexLoader::takeResult() && {<br>+ std::vector<LoadedShard> Result;<br>+ Result.reserve(LoadedShards.size());<br>+ for (auto &It : LoadedShards) {<br>+ Result.push_back(std::move(It.getValue()));<br>+ LoadedShard &LS = Result.back();<br>+ auto TUsIt = FileToTU.find(LS.AbsolutePath);<br>+ assert(TUsIt != FileToTU.end() && "No TU registered for the shard");<br>+ Result.back().DependentTU = TUsIt->second;<br>+ }<br>+ return Result;<br>+}<br>+} // namespace<br>+<br>+std::vector<LoadedShard><br>+loadIndexShards(llvm::ArrayRef<Path> MainFiles,<br>+ BackgroundIndexStorage::Factory &IndexStorageFactory,<br>+ const GlobalCompilationDatabase &CDB) {<br>+ BackgroundIndexLoader Loader;<br>+ for (llvm::StringRef MainFile : MainFiles) {<br>+ assert(llvm::sys::path::is_absolute(MainFile));<br>+<br>+ std::string ProjectRoot;<br>+ if (auto PI = CDB.getProjectInfo(MainFile))<br>+ ProjectRoot = std::move(PI->SourceRoot);<br>+ BackgroundIndexStorage *Storage = IndexStorageFactory(ProjectRoot);<br>+ Loader.load(MainFile, Storage);<br>+ }<br>+ return std::move(Loader).takeResult();<br>+}<br>+<br>+} // namespace clangd<br>+} // namespace clang<br><br>Added: clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.h?rev=366458&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.h?rev=366458&view=auto</a><br>==============================================================================<br>--- clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.h (added)<br>+++ clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.h Thu Jul 18 09:25:36 2019<br>@@ -0,0 +1,54 @@<br>+//===--- BackgroundIndexLoader.h - Load shards from index storage-*- C++-*-===//<br>+//<br>+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.<br>+// See <a href="https://llvm.org/LICENSE.txt" target="_blank">https://llvm.org/LICENSE.txt</a> for license information.<br>+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception<br>+//<br>+//===----------------------------------------------------------------------===//<br>+<br>+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_BACKGROUND_INDEX_LOADER_H<br>+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_BACKGROUND_INDEX_LOADER_H<br>+<br>+#include "Path.h"<br>+#include "index/Background.h"<br>+#include "llvm/ADT/ArrayRef.h"<br>+#include "llvm/ADT/DenseMap.h"<br>+#include "llvm/ADT/Optional.h"<br>+#include "llvm/ADT/StringMap.h"<br>+#include "llvm/ADT/StringRef.h"<br>+#include "llvm/Support/VirtualFileSystem.h"<br>+#include <memory><br>+#include <vector><br>+<br>+namespace clang {<br>+namespace clangd {<br>+<br>+/// Represents a shard loaded from storage, stores contents in \p Shard and<br>+/// metadata about the source file that generated this shard.<br>+struct LoadedShard {<br>+ /// Path of the source file that produced this shard.<br>+ Path AbsolutePath;<br>+ /// Digest of the source file contents that produced this shard.<br>+ FileDigest Digest = {};<br>+ /// Whether the RefSlab in Shard should be used for updating symbol reference<br>+ /// counts when building an index.<br>+ bool CountReferences = false;<br>+ /// Whether the indexing action producing that shard had errors.<br>+ bool HadErrors = false;<br>+ /// Path to a TU that is depending on this shard.<br>+ Path DependentTU;<br>+ /// Will be nullptr when index storage couldn't provide a valid shard for<br>+ /// AbsolutePath.<br>+ std::unique_ptr<IndexFileIn> Shard;<br>+};<br>+<br>+/// Loads all shards for the TU \p MainFile from \p Storage.<br>+std::vector<LoadedShard><br>+loadIndexShards(llvm::ArrayRef<Path> MainFiles,<br>+ BackgroundIndexStorage::Factory &IndexStorageFactory,<br>+ const GlobalCompilationDatabase &CDB);<br>+<br>+} // namespace clangd<br>+} // namespace clang<br>+<br>+#endif<br><br>Modified: clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp?rev=366458&r1=366457&r2=366458&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp?rev=366458&r1=366457&r2=366458&view=diff</a><br>==============================================================================<br>--- clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp (original)<br>+++ clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp Thu Jul 18 09:25:36 2019<br>@@ -78,13 +78,13 @@ void BackgroundIndexRebuilder::idle() {<br> void BackgroundIndexRebuilder::startLoading() {<br> std::lock_guard<std::mutex> Lock(Mu);<br> if (!Loading)<br>- LoadedTUs = 0;<br>+ LoadedShards = 0;<br> ++Loading;<br> }<br>-void BackgroundIndexRebuilder::loadedTU() {<br>+void BackgroundIndexRebuilder::loadedShard(size_t ShardCount) {<br> std::lock_guard<std::mutex> Lock(Mu);<br> assert(Loading);<br>- ++LoadedTUs;<br>+ LoadedShards += ShardCount;<br> }<br> void BackgroundIndexRebuilder::doneLoading() {<br> maybeRebuild("after loading index from disk", [this] {<br>@@ -93,7 +93,7 @@ void BackgroundIndexRebuilder::doneLoadi<br> if (Loading) // was loading multiple batches concurrently<br> return false; // rebuild once the last batch is done.<br> // Rebuild if we loaded any shards, or if we stopped an indexedTU rebuild.<br>- return LoadedTUs > 0 || enoughTUsToRebuild();<br>+ return LoadedShards > 0 || enoughTUsToRebuild();<br> });<br> }<br><br><br>Modified: clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h?rev=366458&r1=366457&r2=366458&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h?rev=366458&r1=366457&r2=366458&view=diff</a><br>==============================================================================<br>--- clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h (original)<br>+++ clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h Thu Jul 18 09:25:36 2019<br>@@ -17,6 +17,7 @@<br> #include "index/FileIndex.h"<br> #include "index/Index.h"<br> #include "llvm/Support/Threading.h"<br>+#include <cstddef><br><br> namespace clang {<br> namespace clangd {<br>@@ -61,7 +62,7 @@ public:<br> // sessions may happen concurrently.<br> void startLoading();<br> // Called to indicate some shards were actually loaded from disk.<br>- void loadedTU();<br>+ void loadedShard(size_t ShardCount);<br> // Called to indicate we're finished loading shards from disk.<br> // May rebuild (if any were loaded).<br> void doneLoading();<br>@@ -89,7 +90,7 @@ private:<br> unsigned IndexedTUsAtLastRebuild = 0;<br> // Are we loading shards? May be multiple concurrent sessions.<br> unsigned Loading = 0;<br>- unsigned LoadedTUs; // In the current loading session.<br>+ unsigned LoadedShards; // In the current loading session.<br><br> SwapIndex *Target;<br> FileSymbols *Source;<br><br>Modified: clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp?rev=366458&r1=366457&r2=366458&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp?rev=366458&r1=366457&r2=366458&view=diff</a><br>==============================================================================<br>--- clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp (original)<br>+++ clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp Thu Jul 18 09:25:36 2019<br>@@ -211,7 +211,7 @@ TEST_F(BackgroundIndexTest, ShardStorage<br> OverlayCDB CDB(/*Base=*/nullptr);<br> BackgroundIndex Idx(Context::empty(), FS, CDB,<br> [&](llvm::StringRef) { return &MSS; });<br>- CDB.setCompileCommand(testPath("root"), Cmd);<br>+ CDB.setCompileCommand(testPath("root/<a href="http://a.cc/" target="_blank">A.cc</a>"), Cmd);<br> ASSERT_TRUE(Idx.blockUntilIdleForTest());<br> }<br> EXPECT_EQ(CacheHits, 2U); // Check both <a href="http://a.cc/" target="_blank">A.cc</a> and A.h loaded from cache.<br>@@ -335,7 +335,7 @@ TEST_F(BackgroundIndexTest, ShardStorage<br> OverlayCDB CDB(/*Base=*/nullptr);<br> BackgroundIndex Idx(Context::empty(), FS, CDB,<br> [&](llvm::StringRef) { return &MSS; });<br>- CDB.setCompileCommand(testPath("root"), Cmd);<br>+ CDB.setCompileCommand(testPath("root/<a href="http://a.cc/" target="_blank">A.cc</a>"), Cmd);<br> ASSERT_TRUE(Idx.blockUntilIdleForTest());<br> }<br> EXPECT_EQ(CacheHits, 2U); // Check both <a href="http://a.cc/" target="_blank">A.cc</a> and A.h loaded from cache.<br>@@ -353,7 +353,7 @@ TEST_F(BackgroundIndexTest, ShardStorage<br> OverlayCDB CDB(/*Base=*/nullptr);<br> BackgroundIndex Idx(Context::empty(), FS, CDB,<br> [&](llvm::StringRef) { return &MSS; });<br>- CDB.setCompileCommand(testPath("root"), Cmd);<br>+ CDB.setCompileCommand(testPath("root/<a href="http://a.cc/" target="_blank">A.cc</a>"), Cmd);<br> ASSERT_TRUE(Idx.blockUntilIdleForTest());<br> }<br> EXPECT_EQ(CacheHits, 2U); // Check both <a href="http://a.cc/" target="_blank">A.cc</a> and A.h loaded from cache.<br>@@ -621,8 +621,8 @@ TEST_F(BackgroundIndexRebuilderTest, Ind<br><br> TEST_F(BackgroundIndexRebuilderTest, LoadingShards) {<br> Rebuilder.startLoading();<br>- Rebuilder.loadedTU();<br>- Rebuilder.loadedTU();<br>+ Rebuilder.loadedShard(10);<br>+ Rebuilder.loadedShard(20);<br> EXPECT_TRUE(checkRebuild([&] { Rebuilder.doneLoading(); }));<br><br> // No rebuild for no shards.<br>@@ -631,11 +631,11 @@ TEST_F(BackgroundIndexRebuilderTest, Loa<br><br> // Loads can overlap.<br> Rebuilder.startLoading();<br>- Rebuilder.loadedTU();<br>+ Rebuilder.loadedShard(1);<br> Rebuilder.startLoading();<br>- Rebuilder.loadedTU();<br>+ Rebuilder.loadedShard(1);<br> EXPECT_FALSE(checkRebuild([&] { Rebuilder.doneLoading(); }));<br>- Rebuilder.loadedTU();<br>+ Rebuilder.loadedShard(1);<br> EXPECT_TRUE(checkRebuild([&] { Rebuilder.doneLoading(); }));<br><br> // No rebuilding for indexed files while loading.<br><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br></div></div></blockquote></div><br></div></div></div></blockquote></div><br></div></div></span></div></blockquote></div>