Author: sammccall Date: Fri Jul 12 03:18:42 2019 New Revision: 365888 URL: http://llvm.org/viewvc/llvm-project?rev=365888&view=rev Log: [clangd] Prioritize indexing of files that share a basename with the open file.
Summary: In practice, opening Foo.h will still often result in Foo.cpp making the second index build instead of the first, as the rebuild policy doesn't know to wait. Reviewers: kadircet Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D64575 Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/TUScheduler.cpp clang-tools-extra/trunk/clangd/TUScheduler.h clang-tools-extra/trunk/clangd/index/Background.cpp clang-tools-extra/trunk/clangd/index/Background.h clang-tools-extra/trunk/clangd/index/BackgroundQueue.cpp clang-tools-extra/trunk/clangd/unittests/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=365888&r1=365887&r2=365888&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Jul 12 03:18:42 2019 @@ -151,7 +151,10 @@ void ClangdServer::addDocument(PathRef F Inputs.Contents = Contents; Inputs.Opts = std::move(Opts); Inputs.Index = Index; - WorkScheduler.update(File, Inputs, WantDiags); + bool NewFile = WorkScheduler.update(File, Inputs, WantDiags); + // If we loaded Foo.h, we want to make sure Foo.cpp is indexed. + if (NewFile && BackgroundIdx) + BackgroundIdx->boostRelated(File); } void ClangdServer::removeDocument(PathRef File) { WorkScheduler.remove(File); } Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=365888&r1=365887&r2=365888&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original) +++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Fri Jul 12 03:18:42 2019 @@ -860,9 +860,10 @@ bool TUScheduler::blockUntilIdle(Deadlin return true; } -void TUScheduler::update(PathRef File, ParseInputs Inputs, +bool TUScheduler::update(PathRef File, ParseInputs Inputs, WantDiagnostics WantDiags) { std::unique_ptr<FileData> &FD = Files[File]; + bool NewFile = FD == nullptr; if (!FD) { // Create a new worker to process the AST-related tasks. ASTWorkerHandle Worker = ASTWorker::create( @@ -875,6 +876,7 @@ void TUScheduler::update(PathRef File, P FD->Contents = Inputs.Contents; } FD->Worker->update(std::move(Inputs), WantDiags); + return NewFile; } void TUScheduler::remove(PathRef File) { Modified: clang-tools-extra/trunk/clangd/TUScheduler.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.h?rev=365888&r1=365887&r2=365888&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/TUScheduler.h (original) +++ clang-tools-extra/trunk/clangd/TUScheduler.h Fri Jul 12 03:18:42 2019 @@ -142,13 +142,14 @@ public: /// contain files that currently run something over their AST. std::vector<Path> getFilesWithCachedAST() const; - /// Schedule an update for \p File. Adds \p File to a list of tracked files if - /// \p File was not part of it before. The compile command in \p Inputs is - /// ignored; worker queries CDB to get the actual compile command. + /// Schedule an update for \p File. + /// The compile command in \p Inputs is ignored; worker queries CDB to get + /// the actual compile command. /// If diagnostics are requested (Yes), and the context is cancelled /// before they are prepared, they may be skipped if eventual-consistency /// permits it (i.e. WantDiagnostics is downgraded to Auto). - void update(PathRef File, ParseInputs Inputs, WantDiagnostics WD); + /// Returns true if the file was not previously tracked. + bool update(PathRef File, ParseInputs Inputs, WantDiagnostics WD); /// Remove \p File from the list of tracked files and schedule removal of its /// resources. Pending diagnostics for closed files may not be delivered, even 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=365888&r1=365887&r2=365888&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Background.cpp (original) +++ clang-tools-extra/trunk/clangd/index/Background.cpp Fri Jul 12 03:18:42 2019 @@ -27,6 +27,7 @@ #include "index/SymbolCollector.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" +#include "clang/Driver/Types.h" #include "llvm/ADT/Hashing.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" @@ -171,6 +172,11 @@ BackgroundQueue::Task BackgroundIndex::c return T; } +static llvm::StringRef filenameWithoutExtension(llvm::StringRef Path) { + Path = llvm::sys::path::filename(Path); + return Path.drop_back(llvm::sys::path::extension(Path).size()); +} + BackgroundQueue::Task BackgroundIndex::indexFileTask(tooling::CompileCommand Cmd, BackgroundIndexStorage *Storage) { @@ -182,9 +188,19 @@ BackgroundIndex::indexFileTask(tooling:: elog("Indexing {0} failed: {1}", FileName, std::move(Error)); }); T.QueuePri = IndexFile; + T.Tag = filenameWithoutExtension(Cmd.Filename); return T; } +void BackgroundIndex::boostRelated(llvm::StringRef Path) { + namespace types = clang::driver::types; + auto Type = + types::lookupTypeForExtension(llvm::sys::path::extension(Path).substr(1)); + // is this a header? + if (Type != types::TY_INVALID && types::onlyPrecompileType(Type)) + Queue.boost(filenameWithoutExtension(Path), IndexBoostedFile); +} + /// Given index results from a TU, only update symbols coming from files that /// are different or missing from than \p ShardVersionsSnapshot. Also stores new /// index information on IndexStorage. 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=365888&r1=365887&r2=365888&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Background.h (original) +++ clang-tools-extra/trunk/clangd/index/Background.h Fri Jul 12 03:18:42 2019 @@ -69,8 +69,8 @@ public: std::function<void()> Run; llvm::ThreadPriority ThreadPri = llvm::ThreadPriority::Background; - // Higher-priority tasks will run first. - unsigned QueuePri = 0; + unsigned QueuePri = 0; // Higher-priority tasks will run first. + std::string Tag; // Allows priority to be boosted later. bool operator<(const Task &O) const { return QueuePri < O.QueuePri; } }; @@ -78,6 +78,10 @@ public: // Add tasks to the queue. void push(Task); void append(std::vector<Task>); + // Boost priority of current and new tasks with matching Tag, if they are + // lower priority. + // Reducing the boost of a tag affects future tasks but not current ones. + void boost(llvm::StringRef Tag, unsigned NewPriority); // Process items on the queue until the queue is stopped. // If the queue becomes empty, OnIdle will be called (on one worker). @@ -98,6 +102,7 @@ private: std::condition_variable CV; bool ShouldStop = false; std::vector<Task> Queue; // max-heap + llvm::StringMap<unsigned> Boosts; }; // Builds an in-memory index by by running the static indexer action over @@ -123,6 +128,10 @@ public: Queue.push(changedFilesTask(ChangedFiles)); } + /// Boosts priority of indexing related to Path. + /// Typically used to index TUs when headers are opened. + void boostRelated(llvm::StringRef Path); + // Cause background threads to stop after ther current task, any remaining // tasks will be discarded. void stop() { @@ -187,6 +196,7 @@ private: // from lowest to highest priority enum QueuePriority { IndexFile, + IndexBoostedFile, LoadShards, }; BackgroundQueue Queue; Modified: clang-tools-extra/trunk/clangd/index/BackgroundQueue.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundQueue.cpp?rev=365888&r1=365887&r2=365888&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/BackgroundQueue.cpp (original) +++ clang-tools-extra/trunk/clangd/index/BackgroundQueue.cpp Fri Jul 12 03:18:42 2019 @@ -67,6 +67,7 @@ void BackgroundQueue::stop() { void BackgroundQueue::push(Task T) { { std::lock_guard<std::mutex> Lock(Mu); + T.QueuePri = std::max(T.QueuePri, Boosts.lookup(T.Tag)); Queue.push_back(std::move(T)); std::push_heap(Queue.begin(), Queue.end()); } @@ -76,12 +77,33 @@ void BackgroundQueue::push(Task T) { void BackgroundQueue::append(std::vector<Task> Tasks) { { std::lock_guard<std::mutex> Lock(Mu); + for (Task &T : Tasks) + T.QueuePri = std::max(T.QueuePri, Boosts.lookup(T.Tag)); std::move(Tasks.begin(), Tasks.end(), std::back_inserter(Queue)); std::make_heap(Queue.begin(), Queue.end()); } CV.notify_all(); } +void BackgroundQueue::boost(llvm::StringRef Tag, unsigned NewPriority) { + std::lock_guard<std::mutex> Lock(Mu); + unsigned &Boost = Boosts[Tag]; + bool Increase = NewPriority > Boost; + Boost = NewPriority; + if (!Increase) + return; // existing tasks unaffected + + unsigned Changes = 0; + for (Task &T : Queue) + if (Tag == T.Tag && NewPriority > T.QueuePri) { + T.QueuePri = NewPriority; + ++Changes; + } + if (Changes) + std::make_heap(Queue.begin(), Queue.end()); + // No need to signal, only rearranged items in the queue. +} + bool BackgroundQueue::blockUntilIdleForTest( llvm::Optional<double> TimeoutSeconds) { std::unique_lock<std::mutex> Lock(Mu); 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=365888&r1=365887&r2=365888&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp Fri Jul 12 03:18:42 2019 @@ -678,5 +678,40 @@ TEST(BackgroundQueueTest, Priority) { EXPECT_EQ(LoRan, 0u); } +TEST(BackgroundQueueTest, Boost) { + std::string Sequence; + + BackgroundQueue::Task A([&] { Sequence.push_back('A'); }); + A.Tag = "A"; + A.QueuePri = 1; + + BackgroundQueue::Task B([&] { Sequence.push_back('B'); }); + B.QueuePri = 2; + B.Tag = "B"; + + { + BackgroundQueue Q; + Q.append({A, B}); + Q.work([&] { Q.stop(); }); + EXPECT_EQ("BA", Sequence) << "priority order"; + } + Sequence.clear(); + { + BackgroundQueue Q; + Q.boost("A", 3); + Q.append({A, B}); + Q.work([&] { Q.stop(); }); + EXPECT_EQ("AB", Sequence) << "A was boosted before enqueueing"; + } + Sequence.clear(); + { + BackgroundQueue Q; + Q.append({A, B}); + Q.boost("A", 3); + Q.work([&] { Q.stop(); }); + EXPECT_EQ("AB", Sequence) << "A was boosted after enqueueing"; + } +} + } // namespace clangd } // namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
