ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: jkorous, MaskRay, ioeric, javed.absar, klimek.

After this commit, clangd will only keep the last 3 accessed ASTs in
memory. Preambles for each of the opened files are still kept in
memory to make completion and AST rebuilds fast.

AST rebuilds are usually fast enough, but having the last ASTs in
memory still considerably improves latency of operations like
findDefinition and documeneHighlight, which are often sent multiple
times a second when moving around the code. So keeping some of the last
accessed ASTs in memory seems like a reasonable tradeoff.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  test/clangd/trace.test

Index: test/clangd/trace.test
===================================================================
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -8,14 +8,14 @@
 # CHECK:   "args": {
 # CHECK:     "File": "{{.*(/|\\)}}foo.c"
 # CHECK:   },
-# CHECK:   "name": "Preamble",
+# CHECK:   "name": "BuildPreamble",
 # CHECK:   "ph": "X",
 # CHECK: }
 # CHECK: {
 # CHECK:   "args": {
 # CHECK:     "File": "{{.*(/|\\)}}foo.c"
 # CHECK:   },
-# CHECK:   "name": "Build",
+# CHECK:   "name": "BuildAST",
 # CHECK:   "ph": "X",
 # CHECK: }
 # CHECK: },
Index: clangd/TUScheduler.h
===================================================================
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -42,6 +42,15 @@
         /// within a bounded amount of time.
 };
 
+/// Configuration of the AST retention policy. This only covers retention of
+/// *idle* ASTs. If queue has operations requiring the AST, they might be
+/// kept in memory.
+struct ASTRetentionParams {
+  /// Maximum number of ASTs to be retained in memory when there are no pending
+  /// requests for them.
+  unsigned MaxRetainedASTs = 3;
+};
+
 /// Handles running tasks for ClangdServer and managing the resources (e.g.,
 /// preambles and ASTs) for opened files.
 /// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -53,7 +62,8 @@
 public:
   TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
               ASTParsedCallback ASTCallback,
-              std::chrono::steady_clock::duration UpdateDebounce);
+              std::chrono::steady_clock::duration UpdateDebounce,
+              ASTRetentionParams RetentionConfig = {});
   ~TUScheduler();
 
   /// Returns estimated memory usage for each of the currently open files.
@@ -98,12 +108,17 @@
 private:
   /// This class stores per-file data in the Files map.
   struct FileData;
-
+public:
+  /// Responsible for retaining and rebuilding idle ASTs. An implementation is
+  /// an LRU cache.
+  class IdleASTs;
+private:
   const bool StorePreamblesInMemory;
   const std::shared_ptr<PCHContainerOperations> PCHOps;
   const ASTParsedCallback ASTCallback;
   Semaphore Barrier;
   llvm::StringMap<std::unique_ptr<FileData>> Files;
+  std::unique_ptr<IdleASTs> ASTCache;
   // None when running tasks synchronously and non-None when running tasks
   // asynchronously.
   llvm::Optional<AsyncTaskRunner> PreambleTasks;
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -45,7 +45,9 @@
 #include "TUScheduler.h"
 #include "Logger.h"
 #include "Trace.h"
+#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
 #include <memory>
@@ -55,6 +57,172 @@
 namespace clang {
 namespace clangd {
 using std::chrono::steady_clock;
+namespace {
+class ASTWorker;
+}
+
+/// Provides an LRU cache of ASTs.
+class TUScheduler::IdleASTs {
+  struct CachedAST;
+
+public:
+  using Key = const ASTWorker *;
+
+  IdleASTs(unsigned MaxRetainedASTs) : MaxRetainedASTs(MaxRetainedASTs) {}
+
+  /// Update a function used to compute an AST for \p K.
+  void update(Key K, std::function<llvm::Optional<ParsedAST>()> BuildAST) {
+    std::unique_lock<std::mutex> Lock(Mut);
+    auto &Item = Items[K];
+    assert(!Item.Active);
+    llvm::Optional<ParsedAST> ForCleanup = Item.removeCached();
+    Item.BuildAST = std::move(BuildAST);
+    // Run AST destructor outside the lock.
+    Lock.unlock();
+    ForCleanup.reset();
+  }
+
+  /// Get the size of the AST.
+  std::size_t getASTBytes(Key K) {
+    std::unique_lock<std::mutex> Lock(Mut);
+    auto It = Items.find(K);
+    assert(It != Items.end());
+    return It->second.ASTUsedBytes;
+  }
+
+  /// Runs user-provided function over the cached AST for \p K.
+  /// - If AST is currently available in the cache, it is reused.
+  /// - If AST was evicted, it is recomputed.
+  /// Also marks the AST for \p K as most recently used.
+  void runOnAST(Key K, llvm::function_ref<void(ParsedAST *)> F) {
+    std::unique_lock<std::mutex> Lock(Mut);
+    auto &Item = findNonActive(K)->second;
+    // Mark item as active for the duration of this operation.
+    // ASTs for active items will not be removed.
+    Item.Active = true;
+    // Compute AST if it's not cached.
+    if (!Item.AST) {
+      Lock.unlock();
+      assert(Item.BuildAST);
+      llvm::Optional<ParsedAST> NewAST = Item.BuildAST();
+      Lock.lock();
+      Item.AST = std::move(NewAST);
+      Item.updateASTSize();
+    }
+    // Put item at the top of the LRU queue.
+    llvm::Optional<ParsedAST> RemovedAST = markAccessed(Item);
+    // Run AST destructor and the user-provided operation without holding the
+    // lock.
+    Lock.unlock();
+    RemovedAST.reset();
+    F(*Item.AST ? Item.AST->getPointer() : nullptr);
+    Lock.lock();
+    // ASTSize might change after reads, too. Update it.
+    Item.updateASTSize();
+    // We're done, mark as non-active.
+    Item.Active = false;
+    // Cleanup the AST if Item was evicted from the LRU cache.
+    llvm::Optional<ParsedAST> ForCleanup;
+    if (Item.ScheduledForCleanup) {
+      ForCleanup = Item.removeCached();
+      Item.ScheduledForCleanup = false;
+    }
+    // Run AST destructor outside the lock.
+    Lock.unlock();
+    ForCleanup.reset();
+  }
+
+  /// Remove the entry for \p K from the LRU cache.
+  void remove(Key K) {
+    std::unique_lock<std::mutex> Lock(Mut);
+    auto It = findNonActive(K);
+    // Find and erase an entry from the LRU list.
+    auto LRUIt = std::find(LRU.begin(), LRU.end(), &It->second);
+    if (LRUIt != LRU.end())
+      LRU.erase(LRUIt);
+    assert(!It->second.Active);
+    llvm::Optional<ParsedAST> ForCleanup = It->second.removeCached();
+    // Erase an entry from the map.
+    Items.erase(It);
+    // Run AST destructor outside the lock.
+    Lock.unlock();
+    ForCleanup.reset();
+  }
+
+private:
+  /// Marks the item as most recently used. If more than the allowed number of
+  /// ASTs are stored at the moment, the last one is removed
+  LLVM_NODISCARD
+  llvm::Optional<ParsedAST> markAccessed(CachedAST &Item) /* REQUIRES(Mu) */ {
+    auto Existing = std::find(LRU.begin(), LRU.end(), &Item);
+    if (Existing != LRU.end())
+      LRU.erase(Existing);
+    LRU.insert(LRU.begin(), &Item);
+    if (LRU.size() <= MaxRetainedASTs)
+      return llvm::None;
+
+    llvm::Optional<ParsedAST> ForCleanup;
+    if (LRU.back()->Active) {
+      // An active action is running, so we can't remove the AST.
+      // We'll signal a running action to do a cleanup instead.
+      LRU.back()->ScheduledForCleanup = true;
+    } else {
+      ForCleanup = LRU.back()->removeCached();
+    }
+    LRU.pop_back();
+    return ForCleanup;
+  }
+
+  std::map<Key, CachedAST>::iterator findNonActive(Key K) /* REQUIRES(Mu) */ {
+    auto It = Items.find(K);
+    assert(It != Items.end());
+    assert(!It->second.Active);
+    return It;
+  }
+
+private:
+  struct CachedAST {
+    /// Top-level optional is set iff AST is cached.
+    /// The inner optional indicates whether an error occured while building the
+    /// AST.
+    llvm::Optional<llvm::Optional<ParsedAST>> AST;
+    /// getUsedBytes, called on the last AST.
+    size_t ASTUsedBytes = 0;
+    /// Function to build AST in case it's missing.
+    std::function<llvm::Optional<ParsedAST>()> BuildAST;
+    /// Indicates an active operation runs over an AST and it can't be removed.
+    bool Active = false;
+    /// Indicates that any running operation must remove the cached value after
+    /// it finishes.
+    bool ScheduledForCleanup = false;
+
+    /// Remove the cached AST and return it to the caller.
+    /// Can only be run when Active=false.
+    LLVM_NODISCARD
+    llvm::Optional<ParsedAST> removeCached() {
+      assert(!Active);
+      if (!AST)
+        return llvm::None;
+      llvm::Optional<ParsedAST> Result = std::move(*AST);
+      AST.reset();
+      ASTUsedBytes = 0;
+      return Result;
+    }
+
+    void updateASTSize() {
+      ASTUsedBytes = !AST || !*AST ? 0 : (*AST)->getUsedBytes();
+    }
+  };
+
+  std::mutex Mut;
+  unsigned MaxRetainedASTs;
+  // FIXME(ibiryukov): we could get rid of this map and lookups in it by
+  // changing our API slightly. Might be worth exploring.
+  std::map<Key, CachedAST> Items; /* GUARDED_BY(Mut) */
+  /// Items sorted in LRU order, i.e. first item is the most recently used one.
+  std::vector<CachedAST *> LRU; /* GUARDED_BY(Mut) */
+};
+
 namespace {
 class ASTWorkerHandle;
 
@@ -70,17 +238,19 @@
 /// worker.
 class ASTWorker {
   friend class ASTWorkerHandle;
-  ASTWorker(llvm::StringRef File, Semaphore &Barrier, CppFile AST, bool RunSync,
+  ASTWorker(ASTBuilder Builder, TUScheduler::IdleASTs &ASTCache,
+            Semaphore &Barrier, bool RunSync,
             steady_clock::duration UpdateDebounce);
 
 public:
   /// Create a new ASTWorker and return a handle to it.
   /// The processing thread is spawned using \p Tasks. However, when \p Tasks
   /// is null, all requests will be processed on the calling thread
   /// synchronously instead. \p Barrier is acquired when processing each
   /// request, it is be used to limit the number of actively running threads.
-  static ASTWorkerHandle Create(llvm::StringRef File, AsyncTaskRunner *Tasks,
-                                Semaphore &Barrier, CppFile AST,
+  static ASTWorkerHandle Create(ASTBuilder Builder,
+                                TUScheduler::IdleASTs &ASTCache,
+                                AsyncTaskRunner *Tasks, Semaphore &Barrier,
                                 steady_clock::duration UpdateDebounce);
   ~ASTWorker();
 
@@ -119,21 +289,19 @@
     llvm::Optional<WantDiagnostics> UpdateType;
   };
 
-  const std::string File;
+  // Handles building and retention of ASTs.
+  TUScheduler::IdleASTs &ASTCache;
   const bool RunSync;
   // Time to wait after an update to see whether another update obsoletes it.
   const steady_clock::duration UpdateDebounce;
 
   Semaphore &Barrier;
-  // AST and FileInputs are only accessed on the processing thread from run().
-  CppFile AST;
   // Inputs, corresponding to the current state of AST.
   ParseInputs FileInputs;
+  ASTBuilder Builder;
   // Guards members used by both TUScheduler and the worker thread.
   mutable std::mutex Mutex;
   std::shared_ptr<const PreambleData> LastBuiltPreamble; /* GUARDED_BY(Mutex) */
-  // Result of getUsedBytes() after the last rebuild or read of AST.
-  std::size_t LastASTSize; /* GUARDED_BY(Mutex) */
   // Set to true to signal run() to finish processing.
   bool Done;                    /* GUARDED_BY(Mutex) */
   std::deque<Request> Requests; /* GUARDED_BY(Mutex) */
@@ -182,27 +350,33 @@
   std::shared_ptr<ASTWorker> Worker;
 };
 
-ASTWorkerHandle ASTWorker::Create(llvm::StringRef File, AsyncTaskRunner *Tasks,
-                                  Semaphore &Barrier, CppFile AST,
+ASTWorkerHandle ASTWorker::Create(ASTBuilder Builder,
+                                  TUScheduler::IdleASTs &ASTCache,
+                                  AsyncTaskRunner *Tasks, Semaphore &Barrier,
                                   steady_clock::duration UpdateDebounce) {
-  std::shared_ptr<ASTWorker> Worker(new ASTWorker(
-      File, Barrier, std::move(AST), /*RunSync=*/!Tasks, UpdateDebounce));
+  std::shared_ptr<ASTWorker> Worker(new ASTWorker(std::move(Builder), ASTCache,
+                                                  Barrier, /*RunSync=*/!Tasks,
+                                                  UpdateDebounce));
   if (Tasks)
-    Tasks->runAsync("worker:" + llvm::sys::path::filename(File),
-                    [Worker]() { Worker->run(); });
+    Tasks->runAsync(
+        "worker:" + llvm::sys::path::filename(Worker->Builder.getFileName()),
+        [Worker]() { Worker->run(); });
 
   return ASTWorkerHandle(std::move(Worker));
 }
 
-ASTWorker::ASTWorker(llvm::StringRef File, Semaphore &Barrier, CppFile AST,
-                     bool RunSync, steady_clock::duration UpdateDebounce)
-    : File(File), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
-      Barrier(Barrier), AST(std::move(AST)), Done(false) {
-  if (RunSync)
-    return;
+ASTWorker::ASTWorker(ASTBuilder Builder, TUScheduler::IdleASTs &ASTCache,
+                     Semaphore &Barrier, bool RunSync,
+                     steady_clock::duration UpdateDebounce)
+    : ASTCache(ASTCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
+      Barrier(Barrier), Builder(std::move(Builder)), Done(false) {
+  // Register us in the ASTCache.
+  ASTCache.update(this, []() { return llvm::None; });
 }
 
 ASTWorker::~ASTWorker() {
+  // Unregister from the ASTCache.
+  ASTCache.remove(this);
 #ifndef NDEBUG
   std::lock_guard<std::mutex> Lock(Mutex);
   assert(Done && "handle was not destroyed");
@@ -213,20 +387,44 @@
 void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags,
                        UniqueFunction<void(std::vector<Diag>)> OnUpdated) {
   auto Task = [=](decltype(OnUpdated) OnUpdated) mutable {
+    tooling::CompileCommand OldCommand = std::move(FileInputs.CompileCommand);
     FileInputs = Inputs;
-    auto Diags = AST.rebuild(std::move(Inputs));
 
+    log("Updating file " + Builder.getFileName() + " with command [" +
+        Inputs.CompileCommand.Directory + "] " +
+        llvm::join(Inputs.CompileCommand.CommandLine, " "));
+    // Rebuild the preamble and the AST.
+    std::unique_ptr<CompilerInvocation> CI =
+        Builder.buildCompilerInvocation(Inputs);
+    if (!CI) {
+      ASTCache.update(this, []() { return llvm::None; });
+      return;
+    }
+    std::shared_ptr<const PreambleData> NewPreamble = Builder.buildPreamble(
+        *CI, getPossiblyStalePreamble(), OldCommand, Inputs);
     {
       std::lock_guard<std::mutex> Lock(Mutex);
-      if (AST.getPreamble())
-        LastBuiltPreamble = AST.getPreamble();
-      LastASTSize = AST.getUsedBytes();
+      if (NewPreamble)
+        LastBuiltPreamble = NewPreamble;
     }
+    // Make our ASTCache aware of the AST change, it will build the AST when
+    // it is needed.
+    CompilerInvocation ASTCI = std::move(*CI);
+    auto BuildAST = [this, NewPreamble, Inputs, ASTCI]() {
+      auto CI = llvm::make_unique<CompilerInvocation>(ASTCI);
+      return Builder.buildAST(std::move(CI), NewPreamble, Inputs);
+    };
+    ASTCache.update(this, BuildAST);
     // We want to report the diagnostics even if this update was cancelled.
     // It seems more useful than making the clients wait indefinitely if they
     // spam us with updates.
-    if (Diags && WantDiags != WantDiagnostics::No)
-      OnUpdated(std::move(*Diags));
+    if (WantDiags != WantDiagnostics::No) {
+      ASTCache.runOnAST(this, [&](ParsedAST *AST) -> void {
+        if (!AST)
+          return;
+        OnUpdated(AST->getDiagnostics());
+      });
+    }
   };
 
   startTask("Update", Bind(Task, std::move(OnUpdated)), WantDiags);
@@ -236,20 +434,13 @@
     llvm::StringRef Name,
     UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action) {
   auto Task = [=](decltype(Action) Action) {
-    ParsedAST *ActualAST = AST.getAST();
-    if (!ActualAST) {
-      Action(llvm::make_error<llvm::StringError>("invalid AST",
-                                                 llvm::errc::invalid_argument));
-      return;
-    }
-    Action(InputsAndAST{FileInputs, *ActualAST});
-
-    // Size of the AST might have changed after reads too, e.g. if some decls
-    // were deserialized from preamble.
-    std::lock_guard<std::mutex> Lock(Mutex);
-    LastASTSize = ActualAST->getUsedBytes();
+    ASTCache.runOnAST(this, [&](ParsedAST *AST) {
+      if (!AST)
+        return Action(llvm::make_error<llvm::StringError>(
+            "invalid AST", llvm::errc::invalid_argument));
+      Action(InputsAndAST{FileInputs, *AST});
+    });
   };
-
   startTask(Name, Bind(Task, std::move(Action)),
             /*UpdateType=*/llvm::None);
 }
@@ -261,8 +452,10 @@
 }
 
 std::size_t ASTWorker::getUsedBytes() const {
-  std::lock_guard<std::mutex> Lock(Mutex);
-  return LastASTSize;
+  std::size_t Result = ASTCache.getASTBytes(this);
+  if (auto Preamble = getPossiblyStalePreamble()) 
+    Result += Preamble->Preamble.getSize();
+  return Result;
 }
 
 void ASTWorker::stop() {
@@ -278,7 +471,8 @@
                           llvm::Optional<WantDiagnostics> UpdateType) {
   if (RunSync) {
     assert(!Done && "running a task after stop()");
-    trace::Span Tracer(Name + ":" + llvm::sys::path::filename(File));
+    trace::Span Tracer(Name + ":" +
+                       llvm::sys::path::filename(Builder.getFileName()));
     Task();
     return;
   }
@@ -415,10 +609,12 @@
 TUScheduler::TUScheduler(unsigned AsyncThreadsCount,
                          bool StorePreamblesInMemory,
                          ASTParsedCallback ASTCallback,
-                         steady_clock::duration UpdateDebounce)
+                         std::chrono::steady_clock::duration UpdateDebounce,
+                         ASTRetentionParams RetentionConfig)
     : StorePreamblesInMemory(StorePreamblesInMemory),
       PCHOps(std::make_shared<PCHContainerOperations>()),
       ASTCallback(std::move(ASTCallback)), Barrier(AsyncThreadsCount),
+      ASTCache(llvm::make_unique<IdleASTs>(RetentionConfig.MaxRetainedASTs)),
       UpdateDebounce(UpdateDebounce) {
   if (0 < AsyncThreadsCount) {
     PreambleTasks.emplace();
@@ -454,9 +650,9 @@
   if (!FD) {
     // Create a new worker to process the AST-related tasks.
     ASTWorkerHandle Worker = ASTWorker::Create(
-        File, WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier,
-        CppFile(File, StorePreamblesInMemory, PCHOps, ASTCallback),
-        UpdateDebounce);
+        ASTBuilder(File, StorePreamblesInMemory, PCHOps, ASTCallback),
+        *ASTCache, WorkerThreads ? WorkerThreads.getPointer() : nullptr,
+        Barrier, UpdateDebounce);
     FD = std::unique_ptr<FileData>(new FileData{
         Inputs.Contents, Inputs.CompileCommand, std::move(Worker)});
   } else {
Index: clangd/ClangdUnit.h
===================================================================
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -47,6 +47,7 @@
                std::vector<serialization::DeclID> TopLevelDeclIDs,
                std::vector<Diag> Diags, std::vector<Inclusion> Inclusions);
 
+  tooling::CompileCommand CompileCommand;
   PrecompiledPreamble Preamble;
   std::vector<serialization::DeclID> TopLevelDeclIDs;
   std::vector<Diag> Diags;
@@ -128,48 +129,41 @@
 
 using ASTParsedCallback = std::function<void(PathRef Path, ParsedAST *)>;
 
-/// Manages resources, required by clangd. Allows to rebuild file with new
-/// contents, and provides AST and Preamble for it.
-class CppFile {
+/// A helper class that handles building preambles and ASTs for a file. Also
+/// adds some logging.
+class ASTBuilder {
 public:
-  CppFile(PathRef FileName, bool StorePreamblesInMemory,
-          std::shared_ptr<PCHContainerOperations> PCHs,
-          ASTParsedCallback ASTCallback);
-
-  /// Rebuild the AST and the preamble.
-  /// Returns a list of diagnostics or llvm::None, if an error occured.
-  llvm::Optional<std::vector<Diag>> rebuild(ParseInputs &&Inputs);
-  /// Returns the last built preamble.
-  const std::shared_ptr<const PreambleData> &getPreamble() const;
-  /// Returns the last built AST.
-  ParsedAST *getAST() const;
-  /// Returns an estimated size, in bytes, currently occupied by the AST and the
-  /// Preamble.
-  std::size_t getUsedBytes() const;
+  ASTBuilder(PathRef FileName, bool StorePreambleInMemory,
+             std::shared_ptr<PCHContainerOperations> PCHs,
+             ASTParsedCallback ASTCallback);
 
-private:
-  /// Build a new preamble for \p Inputs. If the current preamble can be reused,
-  /// it is returned instead.
-  /// This method is const to ensure we don't incidentally modify any fields.
+  PathRef getFileName() const;
+
+  /// Builds compiler invocation that could be used to build AST or preamble.
+  std::unique_ptr<CompilerInvocation>
+  buildCompilerInvocation(const ParseInputs &Inputs) const;
+
+  /// Rebuild the preamble for the new inputs unless the old one can be reused.
+  /// If \p OldPreamble can be reused, it is returned unchanged.
+  /// If \p OldPreamble is null, always builds the preamble.
   std::shared_ptr<const PreambleData>
-  rebuildPreamble(CompilerInvocation &CI,
-                  const tooling::CompileCommand &Command,
-                  IntrusiveRefCntPtr<vfs::FileSystem> FS,
-                  llvm::MemoryBuffer &ContentsBuffer) const;
-
-  const Path FileName;
-  const bool StorePreamblesInMemory;
-
-  /// The last CompileCommand used to build AST and Preamble.
-  tooling::CompileCommand Command;
-  /// The last parsed AST.
-  llvm::Optional<ParsedAST> AST;
-  /// The last built Preamble.
-  std::shared_ptr<const PreambleData> Preamble;
-  /// Utility class required by clang
+  buildPreamble(CompilerInvocation &CI,
+                std::shared_ptr<const PreambleData> OldPreamble,
+                const tooling::CompileCommand &OldCompileCommand,
+                const ParseInputs &Inputs) const;
+
+  /// Builds the AST using an existing \p Preamble. Note that \p Preamble is
+  /// always used, without the checks that it is not outdated.
+  /// To get the correct preamble, use buildPreamble.
+  llvm::Optional<ParsedAST>
+  buildAST(std::unique_ptr<CompilerInvocation> CI,
+           std::shared_ptr<const PreambleData> Preamble,
+           const ParseInputs &Inputs) const;
+
+private:
+  Path FileName;
+  bool StorePreambleInMemory;
   std::shared_ptr<PCHContainerOperations> PCHs;
-  /// This is called after the file is parsed. This can be nullptr if there is
-  /// no callback.
   ASTParsedCallback ASTCallback;
 };
 
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -13,6 +13,7 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "Trace.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -25,7 +26,6 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Basic/LangOptions.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CrashRecoveryContext.h"
@@ -182,8 +182,12 @@
   ASTDiags.EndSourceFile();
 
   std::vector<const Decl *> ParsedDecls = Action->takeTopLevelDecls();
+  std::vector<Diag> Diags = ASTDiags.take();
+  // Add diagnostics from the preamble, if any.
+  if (Preamble)
+    Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
-                   std::move(ParsedDecls), ASTDiags.take(),
+                   std::move(ParsedDecls), std::move(Diags),
                    std::move(Inclusions));
 }
 
@@ -275,19 +279,16 @@
   assert(this->Action);
 }
 
-CppFile::CppFile(PathRef FileName, bool StorePreamblesInMemory,
-                 std::shared_ptr<PCHContainerOperations> PCHs,
-                 ASTParsedCallback ASTCallback)
-    : FileName(FileName), StorePreamblesInMemory(StorePreamblesInMemory),
-      PCHs(std::move(PCHs)), ASTCallback(std::move(ASTCallback)) {
-  log("Created CppFile for " + FileName);
-}
+ASTBuilder::ASTBuilder(PathRef FileName, bool StorePreambleInMemory,
+                       std::shared_ptr<PCHContainerOperations> PCHs,
+                       ASTParsedCallback ASTCallback)
+    : FileName(FileName), StorePreambleInMemory(StorePreambleInMemory),
+      PCHs(std::move(PCHs)), ASTCallback(std::move(ASTCallback)) {}
 
-llvm::Optional<std::vector<Diag>> CppFile::rebuild(ParseInputs &&Inputs) {
-  log("Rebuilding file " + FileName + " with command [" +
-      Inputs.CompileCommand.Directory + "] " +
-      llvm::join(Inputs.CompileCommand.CommandLine, " "));
+PathRef ASTBuilder::getFileName() const { return FileName; }
 
+std::unique_ptr<CompilerInvocation>
+ASTBuilder::buildCompilerInvocation(const ParseInputs &Inputs) const {
   std::vector<const char *> ArgStrs;
   for (const auto &S : Inputs.CompileCommand.CommandLine)
     ArgStrs.push_back(S.c_str());
@@ -298,101 +299,46 @@
     // working dirs.
   }
 
-  // Prepare CompilerInvocation.
-  std::unique_ptr<CompilerInvocation> CI;
-  {
-    // FIXME(ibiryukov): store diagnostics from CommandLine when we start
-    // reporting them.
-    IgnoreDiagnostics IgnoreDiagnostics;
-    IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
-        CompilerInstance::createDiagnostics(new DiagnosticOptions,
-                                            &IgnoreDiagnostics, false);
-    CI = createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine,
-                                         Inputs.FS);
-    if (!CI) {
-      log("Could not build CompilerInvocation for file " + FileName);
-      AST = llvm::None;
-      Preamble = nullptr;
-      return llvm::None;
-    }
-    // createInvocationFromCommandLine sets DisableFree.
-    CI->getFrontendOpts().DisableFree = false;
-    CI->getLangOpts()->CommentOpts.ParseAllComments = true;
-  }
-
-  std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
-      llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, FileName);
-
-  // Compute updated Preamble.
-  std::shared_ptr<const PreambleData> NewPreamble =
-      rebuildPreamble(*CI, Inputs.CompileCommand, Inputs.FS, *ContentsBuffer);
-
-  // Remove current AST to avoid wasting memory.
-  AST = llvm::None;
-  // Compute updated AST.
-  llvm::Optional<ParsedAST> NewAST;
-  {
-    trace::Span Tracer("Build");
-    SPAN_ATTACH(Tracer, "File", FileName);
-    NewAST = ParsedAST::Build(std::move(CI), NewPreamble,
-                              std::move(ContentsBuffer), PCHs, Inputs.FS);
-  }
-
-  std::vector<Diag> Diagnostics;
-  if (NewAST) {
-    // Collect diagnostics from both the preamble and the AST.
-    if (NewPreamble)
-      Diagnostics = NewPreamble->Diags;
-    Diagnostics.insert(Diagnostics.end(), NewAST->getDiagnostics().begin(),
-                       NewAST->getDiagnostics().end());
-  }
-  if (ASTCallback && NewAST) {
-    trace::Span Tracer("Running ASTCallback");
-    ASTCallback(FileName, NewAST.getPointer());
+  // FIXME(ibiryukov): store diagnostics from CommandLine when we start
+  // reporting them.
+  IgnoreDiagnostics IgnoreDiagnostics;
+  IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
+      CompilerInstance::createDiagnostics(new DiagnosticOptions,
+                                          &IgnoreDiagnostics, false);
+  std::unique_ptr<CompilerInvocation> CI = createInvocationFromCommandLine(
+      ArgStrs, CommandLineDiagsEngine, Inputs.FS);
+  if (!CI) {
+    log("Could not build CompilerInvocation for file " + FileName);
+    return nullptr;
   }
-
-  // Write the results of rebuild into class fields.
-  Command = std::move(Inputs.CompileCommand);
-  Preamble = std::move(NewPreamble);
-  AST = std::move(NewAST);
-  return Diagnostics;
-}
-
-const std::shared_ptr<const PreambleData> &CppFile::getPreamble() const {
-  return Preamble;
-}
-
-ParsedAST *CppFile::getAST() const {
-  // We could add mutable to AST instead of const_cast here, but that would also
-  // allow writing to AST from const methods.
-  return AST ? const_cast<ParsedAST *>(AST.getPointer()) : nullptr;
-}
-
-std::size_t CppFile::getUsedBytes() const {
-  std::size_t Total = 0;
-  if (AST)
-    Total += AST->getUsedBytes();
-  if (StorePreamblesInMemory && Preamble)
-    Total += Preamble->Preamble.getSize();
-  return Total;
+  // createInvocationFromCommandLine sets DisableFree.
+  CI->getFrontendOpts().DisableFree = false;
+  CI->getLangOpts()->CommentOpts.ParseAllComments = true;
+  return CI;
 }
 
 std::shared_ptr<const PreambleData>
-CppFile::rebuildPreamble(CompilerInvocation &CI,
-                         const tooling::CompileCommand &Command,
-                         IntrusiveRefCntPtr<vfs::FileSystem> FS,
-                         llvm::MemoryBuffer &ContentsBuffer) const {
-  const auto &OldPreamble = this->Preamble;
-  auto Bounds = ComputePreambleBounds(*CI.getLangOpts(), &ContentsBuffer, 0);
-  if (OldPreamble && compileCommandsAreEqual(this->Command, Command) &&
-      OldPreamble->Preamble.CanReuse(CI, &ContentsBuffer, Bounds, FS.get())) {
+ASTBuilder::buildPreamble(CompilerInvocation &CI,
+                          std::shared_ptr<const PreambleData> OldPreamble,
+                          const tooling::CompileCommand &OldCompileCommand,
+                          const ParseInputs &Inputs) const {
+  // Note that we don't need to copy the input contents, preamble can live
+  // without those.
+  auto ContentsBuffer = llvm::MemoryBuffer::getMemBuffer(Inputs.Contents);
+  auto Bounds =
+      ComputePreambleBounds(*CI.getLangOpts(), ContentsBuffer.get(), 0);
+
+  if (OldPreamble &&
+      compileCommandsAreEqual(Inputs.CompileCommand, OldCompileCommand) &&
+      OldPreamble->Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds,
+                                     Inputs.FS.get())) {
     log("Reusing preamble for file " + Twine(FileName));
     return OldPreamble;
   }
   log("Preamble for file " + Twine(FileName) +
       " cannot be reused. Attempting to rebuild it.");
 
-  trace::Span Tracer("Preamble");
+  trace::Span Tracer("BuildPreamble");
   SPAN_ATTACH(Tracer, "File", FileName);
   StoreDiags PreambleDiagnostics;
   IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
@@ -406,17 +352,16 @@
 
   CppFilePreambleCallbacks SerializedDeclsCollector;
   auto BuiltPreamble = PrecompiledPreamble::Build(
-      CI, &ContentsBuffer, Bounds, *PreambleDiagsEngine, FS, PCHs,
-      /*StoreInMemory=*/StorePreamblesInMemory, SerializedDeclsCollector);
+      CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, Inputs.FS, PCHs,
+      /*StoreInMemory=*/StorePreambleInMemory, SerializedDeclsCollector);
 
   // When building the AST for the main file, we do want the function
   // bodies.
   CI.getFrontendOpts().SkipFunctionBodies = false;
 
   if (BuiltPreamble) {
     log("Built preamble of size " + Twine(BuiltPreamble->getSize()) +
         " for file " + Twine(FileName));
-
     return std::make_shared<PreambleData>(
         std::move(*BuiltPreamble),
         SerializedDeclsCollector.takeTopLevelDeclIDs(),
@@ -427,6 +372,26 @@
   }
 }
 
+llvm::Optional<ParsedAST>
+ASTBuilder::buildAST(std::unique_ptr<CompilerInvocation> CI,
+                     std::shared_ptr<const PreambleData> Preamble,
+                     const ParseInputs &Inputs) const {
+  trace::Span Tracer("BuildAST");
+  SPAN_ATTACH(Tracer, "File", FileName);
+  llvm::Optional<ParsedAST> AST;
+  {
+    trace::Span ParseTracer("ParseAST");
+    AST = ParsedAST::Build(
+        std::move(CI), std::move(Preamble),
+        llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), PCHs, Inputs.FS);
+  }
+  if (ASTCallback) {
+    trace::Span Tracer("Running ASTCallback");
+    ASTCallback(FileName, AST ? AST.getPointer() : nullptr);
+  }
+  return AST;
+}
+
 SourceLocation clangd::getBeginningOfIdentifier(ParsedAST &Unit,
                                                 const Position &Pos,
                                                 const FileID FID) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to