Clean up FileConsumer and improve pipeline

src/file_consumer.cc

@@ -21,62 +21,17 @@ limitations under the License.
 #include "platform.h"
 #include "utils.h"
 
-namespace {
+bool VFS::Loaded(const std::string &path) {
-
+  std::lock_guard lock(mutex);
-std::optional<std::string>
+  return state[path].loaded;
-GetFileContents(const std::string &path,
-                std::unordered_map<std::string, FileContents> *file_contents) {
-  auto it = file_contents->find(path);
-  if (it == file_contents->end()) {
-    std::optional<std::string> content = ReadContent(path);
-    if (content)
-      (*file_contents)[path] = FileContents(path, *content);
-    return content;
-  }
-  return it->second.content;
 }
 
-} // namespace
+bool VFS::Stamp(const std::string &path, int64_t ts, int step) {
-
-FileContents::FileContents(const std::string &path, const std::string &content)
-    : path(path), content(content) {
-  line_offsets_.push_back(0);
-  for (size_t i = 0; i < content.size(); i++) {
-    if (content[i] == '\n')
-      line_offsets_.push_back(i + 1);
-  }
-}
-
-std::optional<int> FileContents::ToOffset(Position p) const {
-  if (0 <= p.line && size_t(p.line) < line_offsets_.size()) {
-    int ret = line_offsets_[p.line] + p.column;
-    if (size_t(ret) < content.size())
-      return ret;
-  }
-  return std::nullopt;
-}
-
-std::optional<std::string> FileContents::ContentsInRange(Range range) const {
-  std::optional<int> start_offset = ToOffset(range.start),
-                     end_offset = ToOffset(range.end);
-  if (start_offset && end_offset && *start_offset < *end_offset)
-    return content.substr(*start_offset, *end_offset - *start_offset);
-  return std::nullopt;
-}
-
-VFS::State VFS::Get(const std::string &file) {
   std::lock_guard<std::mutex> lock(mutex);
-  auto it = state.find(file);
+  State &st = state[path];
-  if (it != state.end())
+  if (st.timestamp < ts || (st.timestamp == ts && st.step < step)) {
-    return it->second;
+    st.timestamp = ts;
-  return {0, 0};
+    st.step = step;
-}
-
-bool VFS::Stamp(const std::string &file, int64_t ts, int64_t offset) {
-  std::lock_guard<std::mutex> lock(mutex);
-  State &st = state[file];
-  if (st.timestamp < ts) {
-    st.timestamp = ts + offset;
     return true;
   } else
     return false;
@@ -87,29 +42,26 @@ FileConsumer::FileConsumer(VFS *vfs, const std::string &parse_file)
 
 IndexFile *FileConsumer::TryConsumeFile(
     const clang::FileEntry &File,
-    std::unordered_map<std::string, FileContents> *file_contents_map) {
+    const std::unordered_map<llvm::sys::fs::UniqueID, FileConsumer::File>
+        &UID2File) {
   auto UniqueID = File.getUniqueID();
-  auto it = local_.find(UniqueID);
+  {
-  if (it != local_.end())
+    auto it = local_.find(UniqueID);
-    return it->second.get();
+    if (it != local_.end())
+      return it->second.get();
+  }
 
-  std::string file_name = FileName(File);
+  auto it = UID2File.find(UniqueID);
-  int64_t tim = File.getModificationTime();
+  assert(it != UID2File.end());
-  assert(tim);
+  assert(it->second.mtime);
-  if (!vfs_->Stamp(file_name, tim, 0)) {
+  if (!vfs_->Stamp(it->second.path, it->second.mtime, 1)) {
     local_[UniqueID] = nullptr;
     return nullptr;
   }
 
-  // Read the file contents, if we fail then we cannot index the file.
-  std::optional<std::string> contents =
-      GetFileContents(file_name, file_contents_map);
-  if (!contents)
-    return nullptr;
-
   // Build IndexFile instance.
   local_[UniqueID] =
-      std::make_unique<IndexFile>(UniqueID, file_name, *contents);
+      std::make_unique<IndexFile>(UniqueID, it->second.path, it->second.content);
   return local_[UniqueID].get();
 }
 

src/file_consumer.h

@@ -27,29 +27,17 @@ limitations under the License.
 
 struct IndexFile;
 
-struct FileContents {
-  FileContents() = default;
-  FileContents(const std::string &path, const std::string &content);
-
-  std::optional<int> ToOffset(Position p) const;
-  std::optional<std::string> ContentsInRange(Range range) const;
-
-  std::string path;
-  std::string content;
-  // {0, 1 + position of first newline, 1 + position of second newline, ...}
-  std::vector<int> line_offsets_;
-};
-
 struct VFS {
   struct State {
     int64_t timestamp;
-    bool loaded = false;
+    int step;
+    bool loaded;
   };
   mutable std::unordered_map<std::string, State> state;
   mutable std::mutex mutex;
 
-  State Get(const std::string &file);
+  bool Loaded(const std::string &path);
-  bool Stamp(const std::string &file, int64_t ts, int64_t offset);
+  bool Stamp(const std::string &path, int64_t ts, int step);
 };
 
 namespace std {
@@ -70,17 +58,18 @@ template <> struct hash<llvm::sys::fs::UniqueID> {
 // The indexer does this because header files do not have their own translation
 // units but we still want to index them.
 struct FileConsumer {
+  struct File {
+    std::string path;
+    int64_t mtime;
+    std::string content;
+  };
+
   FileConsumer(VFS *vfs, const std::string &parse_file);
 
-  // Returns IndexFile for the file or nullptr. |is_first_ownership| is set
+  // Returns IndexFile or nullptr for the file or nullptr.
-  // to true iff the function just took ownership over the file. Otherwise it
+  IndexFile *TryConsumeFile(
-  // is set to false.
+      const clang::FileEntry &file,
-  //
+      const std::unordered_map<llvm::sys::fs::UniqueID, File> &);
-  // note: file_contents is passed as a parameter instead of as a member
-  // variable since it is large and we do not want to copy it.
-  IndexFile *
-  TryConsumeFile(const clang::FileEntry &file,
-                 std::unordered_map<std::string, FileContents> *file_contents);
 
   // Returns and passes ownership of all local state.
   std::vector<std::unique_ptr<IndexFile>> TakeLocalState();

src/indexer.cc

@@ -49,10 +49,8 @@ constexpr int kInitializerMaxLines = 3;
 GroupMatch *multiVersionMatcher;
 
 struct IndexParam {
-  std::unordered_map<llvm::sys::fs::UniqueID, std::string> SeenUniqueID;
+  std::unordered_map<llvm::sys::fs::UniqueID, FileConsumer::File> UID2File;
   std::unordered_map<llvm::sys::fs::UniqueID, bool> UID2multi;
-  std::unordered_map<std::string, FileContents> file_contents;
-  std::unordered_map<std::string, int64_t> file2mtime;
   struct DeclInfo {
     Usr usr;
     std::string short_name;
@@ -68,17 +66,22 @@ struct IndexParam {
   void SeenFile(const FileEntry &File) {
     // If this is the first time we have seen the file (ignoring if we are
     // generating an index for it):
-    auto [it, inserted] = SeenUniqueID.try_emplace(File.getUniqueID());
+    auto [it, inserted] = UID2File.try_emplace(File.getUniqueID());
     if (inserted) {
-      std::string file_name = FileName(File);
+      std::string path = FileName(File);
-      it->second = file_name;
+      it->second.path = path;
-      file2mtime[file_name] = File.getModificationTime();
+      it->second.mtime = File.getModificationTime();
+      if (!it->second.mtime)
+        if (auto tim = LastWriteTime(path))
+          it->second.mtime = *tim;
+      if (std::optional<std::string> content = ReadContent(path))
+        it->second.content = *content;
     }
   }
 
   IndexFile *ConsumeFile(const FileEntry &FE) {
     SeenFile(FE);
-    return file_consumer->TryConsumeFile(FE, &file_contents);
+    return file_consumer->TryConsumeFile(FE, UID2File);
   }
 
   bool UseMultiVersion(const FileEntry &FE) {
@@ -1344,15 +1347,15 @@ Index(CompletionManager *completion, WorkingFiles *wfiles, VFS *vfs,
     for (auto &it : entry->usr2var)
       Uniquify(it.second.uses);
 
-    // Update file contents and modification time.
+    // Update dependencies for the file.
-    entry->mtime = param.file2mtime[entry->path];
+    for (auto &[_, file] : param.UID2File) {
-
+      const std::string &path = file.path;
-    // Update dependencies for the file. Do not include the file in its own
+      if (path == entry->path)
-    // dependency set.
+        entry->mtime = file.mtime;
-    for (auto &[_, path] : param.SeenUniqueID)
+      else if (path != entry->import_file)
-      if (path != entry->path && path != entry->import_file)
         entry->dependencies[llvm::CachedHashStringRef(Intern(path))] =
-            param.file2mtime[path];
+            file.mtime;
+    }
   }
 
   return result;

src/pipeline.cc

@@ -206,16 +206,21 @@ bool Indexer_Parse(CompletionManager *completion, WorkingFiles *wfiles,
   std::optional<int64_t> write_time = LastWriteTime(path_to_index);
   if (!write_time)
     return true;
-  int reparse = vfs->Stamp(path_to_index, *write_time, -1);
+  int reparse = vfs->Stamp(path_to_index, *write_time, 0);
   if (request.path != path_to_index) {
     std::optional<int64_t> mtime1 = LastWriteTime(request.path);
     if (!mtime1)
       return true;
-    if (vfs->Stamp(request.path, *mtime1, -1))
+    if (vfs->Stamp(request.path, *mtime1, 0))
       reparse = 1;
   }
-  if (g_config->index.onChange)
+  if (g_config->index.onChange) {
     reparse = 2;
+    std::lock_guard lock(vfs->mutex);
+    vfs->state[path_to_index].step = 0;
+    if (request.path != path_to_index)
+      vfs->state[request.path].step = 0;
+  }
   if (!reparse)
     return true;
 
@@ -242,6 +247,8 @@ bool Indexer_Parse(CompletionManager *completion, WorkingFiles *wfiles,
       LOG_S(INFO) << "load cache for " << path_to_index;
       auto dependencies = prev->dependencies;
       if (reparse) {
+        if (vfs->Loaded(path_to_index))
+          return true;
         IndexUpdate update = IndexUpdate::CreateDelta(nullptr, prev.get());
         on_indexed->PushBack(std::move(update),
                              request.mode != IndexMode::NonInteractive);
@@ -309,12 +316,7 @@ bool Indexer_Parse(CompletionManager *completion, WorkingFiles *wfiles,
                          << ")";
     {
       std::lock_guard lock(mutexes[std::hash<std::string>()(path) % N_MUTEXES]);
-      bool loaded;
+      if (vfs->Loaded(path))
-      {
-        std::lock_guard lock1(vfs->mutex);
-        loaded = vfs->state[path].loaded;
-      }
-      if (loaded)
         prev = RawCacheLoad(path);
       else
         prev.reset();
@@ -543,13 +545,6 @@ void Index(const std::string &path, const std::vector<const char *> &args,
   index_request->PushBack({path, args, mode, id}, mode != IndexMode::NonInteractive);
 }
 
-std::optional<int64_t> LastWriteTime(const std::string &path) {
-  sys::fs::file_status Status;
-  if (sys::fs::status(path, Status))
-    return {};
-  return sys::toTimeT(Status.getLastModificationTime());
-}
-
 std::optional<std::string> LoadIndexedContent(const std::string &path) {
   if (g_config->cacheDirectory.empty()) {
     std::shared_lock lock(g_index_mutex);

src/pipeline.hh

@@ -46,7 +46,6 @@ void MainLoop();
 void Index(const std::string &path, const std::vector<const char *> &args,
            IndexMode mode, lsRequestId id = {});
 
-std::optional<int64_t> LastWriteTime(const std::string &path);
 std::optional<std::string> LoadIndexedContent(const std::string& path);
 void WriteStdout(MethodType method, lsBaseOutMessage &response);
 } // namespace pipeline

src/utils.cc

@@ -20,7 +20,10 @@ limitations under the License.
 
 #include <siphash.h>
 
-#include "llvm/ADT/StringRef.h"
+#include <llvm/ADT/StringRef.h>
+#include <llvm/Support/FileSystem.h>
+#include <llvm/Support/Path.h>
+using namespace llvm;
 
 #include <algorithm>
 #include <assert.h>
@@ -124,6 +127,13 @@ std::string EscapeFileName(std::string path) {
   return path;
 }
 
+std::optional<int64_t> LastWriteTime(const std::string &path) {
+  sys::fs::file_status Status;
+  if (sys::fs::status(path, Status))
+    return {};
+  return sys::toTimeT(Status.getLastModificationTime());
+}
+
 std::optional<std::string> ReadContent(const std::string &filename) {
   char buf[4096];
   std::string ret;

src/utils.h

@@ -73,6 +73,7 @@ void EnsureEndsInSlash(std::string &path);
 // e.g. foo/bar.c => foo_bar.c
 std::string EscapeFileName(std::string path);
 
+std::optional<int64_t> LastWriteTime(const std::string &path);
 std::optional<std::string> ReadContent(const std::string &filename);
 void WriteToFile(const std::string &filename, const std::string &content);