Enable simple sanity test for import pipeline.

- Add FakeCacheManager
- Add IIndexer so we don't call out to clang

src/cache_manager.cc

@@ -12,97 +12,108 @@
 
 namespace {
 
-std::string GetCachedBaseFileName(Config* config,
-                                  const std::string& source_file,
-                                  bool create_dir = false) {
-  assert(!config->cacheDirectory.empty());
-  std::string cache_file;
-  size_t len = config->projectRoot.size();
-  if (StartsWith(source_file, config->projectRoot)) {
-    cache_file = EscapeFileName(config->projectRoot) + '/' +
-                 EscapeFileName(source_file.substr(len));
-  } else
-    cache_file = EscapeFileName(source_file);
-
-  return config->cacheDirectory + cache_file;
-}
-
-std::string GetCacheFileName(Config* config, const std::string& base) {
-  switch (config->cacheFormat) {
-    case SerializeFormat::Json:
-      return base + "json";
-    case SerializeFormat::MessagePack:
-      return base + "mpack";
-  }
-}
-
-std::unique_ptr<IndexFile> LoadCachedIndex(Config* config,
-                                           const std::string& filename) {
-  if (!config->enableCacheRead)
-    return nullptr;
-
-  optional<std::string> file_content = ReadContent(
-      GetCacheFileName(config, GetCachedBaseFileName(config, filename)));
-  if (!file_content)
-    return nullptr;
-
-  return Deserialize(config->cacheFormat, filename, *file_content,
-                     IndexFile::kCurrentVersion);
-}
-
 // Manages loading caches from file paths for the indexer process.
 struct RealCacheManager : ICacheManager {
   explicit RealCacheManager(Config* config) : config_(config) {}
   ~RealCacheManager() override = default;
 
-  IndexFile* TryLoad(const std::string& path) override {
+  void WriteToCache(IndexFile& file) override {
-    auto it = caches.find(path);
+    if (!config_->enableCacheWrite)
-    if (it != caches.end())
+      return;
-      return it->second.get();
 
-    std::unique_ptr<IndexFile> cache = LoadCachedIndex(config_, path);
+    std::string cache_path = GetCachePath(file.path);
-    if (!cache)
-      return nullptr;
 
-    caches[path] = std::move(cache);
+    if (file.file_contents_.empty()) {
-    return caches[path].get();
+      LOG_S(ERROR) << "No cached file contents; performing potentially stale "
-  }
+                   << "file-copy for " << file.path;
-
+      CopyFileTo(cache_path, file.path);
-  std::unique_ptr<IndexFile> TryTakeOrLoad(const std::string& path) override {
+    } else {
-    auto it = caches.find(path);
+      WriteToFile(cache_path, file.file_contents_);
-    if (it != caches.end()) {
-      auto result = std::move(it->second);
-      caches.erase(it);
-      return result;
     }
 
-    return LoadCachedIndex(config_, path);
+    std::string indexed_content = Serialize(file);
+    WriteToFile(AppendSerializationFormat(cache_path), indexed_content);
   }
 
   optional<std::string> LoadCachedFileContents(
-      const std::string& filename) override {
+      const std::string& path) override {
     if (!config_->enableCacheRead)
       return nullopt;
 
-    return ReadContent(GetCachedBaseFileName(config_, filename));
+    return ReadContent(GetCachePath(path));
   }
 
-  void IterateLoadedCaches(std::function<void(IndexFile*)> fn) override {
+  std::unique_ptr<IndexFile> RawCacheLoad(const std::string& path) {
-    for (const auto& it : caches) {
+    if (!config_->enableCacheRead)
-      assert(it.second);
+      return nullptr;
-      fn(it.second.get());
+
+    std::string cache_path = GetCachePath(path);
+    optional<std::string> file_content =
+        ReadContent(AppendSerializationFormat(cache_path));
+    if (!file_content)
+      return nullptr;
+
+    return Deserialize(config_->cacheFormat, path, *file_content,
+                       IndexFile::kCurrentVersion);
+  }
+
+  std::string GetCachePath(const std::string& source_file) {
+    assert(!config_->cacheDirectory.empty());
+    std::string cache_file;
+    size_t len = config_->projectRoot.size();
+    if (StartsWith(source_file, config_->projectRoot)) {
+      cache_file = EscapeFileName(config_->projectRoot) + '/' +
+                   EscapeFileName(source_file.substr(len));
+    } else {
+      cache_file = EscapeFileName(source_file);
     }
+
+    return config_->cacheDirectory + cache_file;
+  }
+
+  std::string AppendSerializationFormat(const std::string& base) {
+    switch (config_->cacheFormat) {
+      case SerializeFormat::Json:
+        return base + ".json";
+      case SerializeFormat::MessagePack:
+        return base + ".mpack";
+    }
+    assert(false);
+    return ".json";
   }
 
-  std::unordered_map<std::string, std::unique_ptr<IndexFile>> caches;
   Config* config_;
 };
 
-// struct FakeCacheManager : ICacheManager {
+struct FakeCacheManager : ICacheManager {
-//   explicit FakeCacheManager(const std::vector<FakeCacheEntry>& entries) {
+  explicit FakeCacheManager(const std::vector<FakeCacheEntry>& entries)
-//     assert(false && "TODO");
+      : entries_(entries) {}
-//   }
+
-// };
+  void WriteToCache(IndexFile& file) override { assert(false); }
+
+  optional<std::string> LoadCachedFileContents(
+      const std::string& path) override {
+    for (const FakeCacheEntry& entry : entries_) {
+      if (entry.path == path) {
+        return entry.content;
+      }
+    }
+
+    return nullopt;
+  }
+
+  std::unique_ptr<IndexFile> RawCacheLoad(const std::string& path) {
+    for (const FakeCacheEntry& entry : entries_) {
+      if (entry.path == path) {
+        return Deserialize(SerializeFormat::Json, path, entry.json, nullopt);
+      }
+    }
+
+    return nullptr;
+  }
+
+  std::vector<FakeCacheEntry> entries_;
+};
 
 }  // namespace
 
@@ -114,33 +125,45 @@ std::unique_ptr<ICacheManager> ICacheManager::Make(Config* config) {
 // static
 std::unique_ptr<ICacheManager> ICacheManager::MakeFake(
     const std::vector<FakeCacheEntry>& entries) {
-  // return MakeUnique<FakeCacheManager>(entries);
+  return MakeUnique<FakeCacheManager>(entries);
-  assert(false && "TODO");
-  return nullptr;
 }
 
 ICacheManager::~ICacheManager() = default;
 
+IndexFile* ICacheManager::TryLoad(const std::string& path) {
+  auto it = caches_.find(path);
+  if (it != caches_.end())
+    return it->second.get();
+
+  std::unique_ptr<IndexFile> cache = RawCacheLoad(path);
+  if (!cache)
+    return nullptr;
+
+  caches_[path] = std::move(cache);
+  return caches_[path].get();
+}
+
+std::unique_ptr<IndexFile> ICacheManager::TryTakeOrLoad(
+    const std::string& path) {
+  auto it = caches_.find(path);
+  if (it != caches_.end()) {
+    auto result = std::move(it->second);
+    caches_.erase(it);
+    return result;
+  }
+
+  return RawCacheLoad(path);
+}
+
 std::unique_ptr<IndexFile> ICacheManager::TakeOrLoad(const std::string& path) {
   auto result = TryTakeOrLoad(path);
   assert(result);
   return result;
 }
 
-void WriteToCache(Config* config, IndexFile& file) {
+void ICacheManager::IterateLoadedCaches(std::function<void(IndexFile*)> fn) {
-  if (!config->enableCacheWrite)
+  for (const auto& cache : caches_) {
-    return;
+    assert(cache.second);
-
+    fn(cache.second.get());
-  std::string cache_basename = GetCachedBaseFileName(config, file.path);
-
-  if (file.file_contents_.empty()) {
-    LOG_S(ERROR) << "No cached file contents; performing potentially stale "
-                 << "file-copy for " << file.path;
-    CopyFileTo(cache_basename, file.path);
-  } else {
-    WriteToFile(cache_basename, file.file_contents_);
   }
-
-  std::string indexed_content = Serialize(file);
-  WriteToFile(GetCacheFileName(config, cache_basename), indexed_content);
 }

src/cache_manager.h

@@ -5,6 +5,7 @@
 #include <functional>
 #include <memory>
 #include <string>
+#include <unordered_map>
 #include <vector>
 
 struct Config;
@@ -25,21 +26,25 @@ struct ICacheManager {
 
   // Tries to load a cache for |path|, returning null if there is none. The
   // cache loader still owns the cache.
-  virtual IndexFile* TryLoad(const std::string& path) = 0;
+  IndexFile* TryLoad(const std::string& path);
 
   // Takes the existing cache or loads the cache at |path|. May return null if
   // the cache does not exist.
-  virtual std::unique_ptr<IndexFile> TryTakeOrLoad(const std::string& path) = 0;
+  std::unique_ptr<IndexFile> TryTakeOrLoad(const std::string& path);
 
   // Takes the existing cache or loads the cache at |path|. Asserts the cache
   // exists.
   std::unique_ptr<IndexFile> TakeOrLoad(const std::string& path);
 
+  virtual void WriteToCache(IndexFile& file) = 0;
+
   virtual optional<std::string> LoadCachedFileContents(
-      const std::string& filename) = 0;
+      const std::string& path) = 0;
 
   // Iterate over all loaded caches.
-  virtual void IterateLoadedCaches(std::function<void(IndexFile*)> fn) = 0;
+  void IterateLoadedCaches(std::function<void(IndexFile*)> fn);
-};
 
-void WriteToCache(Config* config, IndexFile& file);
+ protected:
+  virtual std::unique_ptr<IndexFile> RawCacheLoad(const std::string& path) = 0;
+  std::unordered_map<std::string, std::unique_ptr<IndexFile>> caches_;
+};

src/iindexer.cc

@@ -0,0 +1,79 @@
+#include "iindexer.h"
+
+#include "indexer.h"
+
+namespace {
+struct ClangIndexer : IIndexer {
+  ~ClangIndexer() override = default;
+
+  std::vector<std::unique_ptr<IndexFile>> Index(
+      Config* config,
+      FileConsumerSharedState* file_consumer_shared,
+      std::string file,
+      const std::vector<std::string>& args,
+      const std::vector<FileContents>& file_contents,
+      PerformanceImportFile* perf) {
+    return Parse(config, file_consumer_shared, file, args, file_contents, perf,
+                 &index);
+  }
+
+  // Note: constructing this acquires a global lock
+  ClangIndex index;
+};
+
+struct TestIndexer : IIndexer {
+  static std::unique_ptr<TestIndexer> FromEntries(
+      const std::vector<TestEntry>& entries) {
+    auto result = MakeUnique<TestIndexer>();
+
+    for (const TestEntry& entry : entries) {
+      std::vector<std::unique_ptr<IndexFile>> indexes;
+
+      if (entry.num_indexes > 0)
+        indexes.push_back(MakeUnique<IndexFile>(entry.path));
+      for (int i = 1; i < entry.num_indexes; ++i) {
+        indexes.push_back(MakeUnique<IndexFile>(entry.path + "_extra_" +
+                                                std::to_string(i) + ".h"));
+      }
+
+      result->indexes.insert(std::make_pair(entry.path, std::move(indexes)));
+    }
+
+    return result;
+  }
+
+  ~TestIndexer() override = default;
+
+  std::vector<std::unique_ptr<IndexFile>> Index(
+      Config* config,
+      FileConsumerSharedState* file_consumer_shared,
+      std::string file,
+      const std::vector<std::string>& args,
+      const std::vector<FileContents>& file_contents,
+      PerformanceImportFile* perf) {
+    auto it = indexes.find(file);
+    if (it == indexes.end())
+      return {};
+    // FIXME: allow user to control how many times we return the index for a
+    // specific file (atm it is always 1)
+    auto result = std::move(it->second);
+    indexes.erase(it);
+    return result;
+  }
+
+  std::unordered_map<std::string, std::vector<std::unique_ptr<IndexFile>>>
+      indexes;
+};
+
+}  // namespace
+
+// static
+std::unique_ptr<IIndexer> IIndexer::MakeClangIndexer() {
+  return MakeUnique<ClangIndexer>();
+}
+
+// static
+std::unique_ptr<IIndexer> IIndexer::MakeTestIndexer(
+    const std::vector<TestEntry>& entries) {
+  return TestIndexer::FromEntries(entries);
+}

src/iindexer.h

@@ -0,0 +1,38 @@
+#pragma once
+
+#include <memory>
+#include <vector>
+
+// TODO:
+//  - rename indexer.h to clang_indexer.h and pull out non-clang specific code
+//    like IndexFile
+//  - rename this file to indexer.h
+
+struct Config;
+struct IndexFile;
+struct FileContents;
+struct FileConsumerSharedState;
+struct PerformanceImportFile;
+
+// Abstracts away the actual indexing process. Each IIndexer instance is
+// per-thread and constructing an instance may be extremely expensive (ie,
+// acquire a lock) and should be done as rarely as possible.
+struct IIndexer {
+  struct TestEntry {
+    std::string path;
+    int num_indexes = 0;
+  };
+
+  static std::unique_ptr<IIndexer> MakeClangIndexer();
+  static std::unique_ptr<IIndexer> MakeTestIndexer(
+      const std::vector<TestEntry>& entries);
+
+  virtual ~IIndexer() = default;
+  virtual std::vector<std::unique_ptr<IndexFile>> Index(
+      Config* config,
+      FileConsumerSharedState* file_consumer_shared,
+      std::string file,
+      const std::vector<std::string>& args,
+      const std::vector<FileContents>& file_contents,
+      PerformanceImportFile* perf) = 0;
+};

src/import_pipeline.cc

@@ -2,6 +2,7 @@
 
 #include "cache_manager.h"
 #include "config.h"
+#include "iindexer.h"
 #include "import_manager.h"
 #include "language_server_api.h"
 #include "message_handler.h"
@@ -41,11 +42,11 @@ enum class FileParseQuery { NeedsParse, DoesNotNeedParse, NoSuchFile };
 std::vector<Index_DoIdMap> DoParseFile(
     Config* config,
     WorkingFiles* working_files,
-    ClangIndex* index,
     FileConsumerSharedState* file_consumer_shared,
     TimestampManager* timestamp_manager,
     ImportManager* import_manager,
     ICacheManager* cache_manager,
+    IIndexer* indexer,
     bool is_interactive,
     const std::string& path,
     const std::vector<std::string>& args,
@@ -192,8 +193,8 @@ std::vector<Index_DoIdMap> DoParseFile(
   }
 
   PerformanceImportFile perf;
-  std::vector<std::unique_ptr<IndexFile>> indexes = Parse(
+  std::vector<std::unique_ptr<IndexFile>> indexes = indexer->Index(
-      config, file_consumer_shared, path, args, file_contents, &perf, index);
+      config, file_consumer_shared, path, args, file_contents, &perf);
   for (std::unique_ptr<IndexFile>& new_index : indexes) {
     Timer time;
 
@@ -216,24 +217,23 @@ std::vector<Index_DoIdMap> DoParseFile(
 std::vector<Index_DoIdMap> ParseFile(
     Config* config,
     WorkingFiles* working_files,
-    ClangIndex* index,
     FileConsumerSharedState* file_consumer_shared,
     TimestampManager* timestamp_manager,
     ImportManager* import_manager,
+    ICacheManager* cache_manager,
+    IIndexer* indexer,
     bool is_interactive,
     const Project::Entry& entry,
     const std::string& contents) {
   FileContents file_contents(entry.filename, contents);
 
-  std::unique_ptr<ICacheManager> cache_manager = ICacheManager::Make(config);
-
   // Try to determine the original import file by loading the file from cache.
   // This lets the user request an index on a header file, which clang will
   // complain about if indexed by itself.
   IndexFile* entry_cache = cache_manager->TryLoad(entry.filename);
   std::string tu_path = entry_cache ? entry_cache->import_file : entry.filename;
-  return DoParseFile(config, working_files, index, file_consumer_shared,
+  return DoParseFile(config, working_files, file_consumer_shared,
-                     timestamp_manager, import_manager, cache_manager.get(),
+                     timestamp_manager, import_manager, cache_manager, indexer,
                      is_interactive, tu_path, entry.args, file_contents);
 }
 
@@ -242,7 +242,8 @@ bool IndexMain_DoParse(Config* config,
                        FileConsumerSharedState* file_consumer_shared,
                        TimestampManager* timestamp_manager,
                        ImportManager* import_manager,
-                       ClangIndex* index) {
+                       ICacheManager* cache_manager,
+                       IIndexer* indexer) {
   auto* queue = QueueManager::instance();
   optional<Index_Request> request = queue->index_request.TryDequeue();
   if (!request)
@@ -251,9 +252,10 @@ bool IndexMain_DoParse(Config* config,
   Project::Entry entry;
   entry.filename = request->path;
   entry.args = request->args;
-  std::vector<Index_DoIdMap> responses = ParseFile(
+  std::vector<Index_DoIdMap> responses =
-      config, working_files, index, file_consumer_shared, timestamp_manager,
+      ParseFile(config, working_files, file_consumer_shared, timestamp_manager,
-      import_manager, request->is_interactive, entry, request->contents);
+                import_manager, cache_manager, indexer, request->is_interactive,
+                entry, request->contents);
 
   // Don't bother sending an IdMap request if there are no responses.
   if (responses.empty())
@@ -264,8 +266,8 @@ bool IndexMain_DoParse(Config* config,
   return true;
 }
 
-bool IndexMain_DoCreateIndexUpdate(Config* config,
+bool IndexMain_DoCreateIndexUpdate(TimestampManager* timestamp_manager,
-                                   TimestampManager* timestamp_manager) {
+                                   ICacheManager* cache_manager) {
   auto* queue = QueueManager::instance();
   optional<Index_OnIdMapped> response = queue->on_id_mapped.TryDequeue();
   if (!response)
@@ -293,7 +295,7 @@ bool IndexMain_DoCreateIndexUpdate(Config* config,
     LOG_S(INFO) << "Writing cached index to disk for "
                 << response->current->file->path;
     time.Reset();
-    WriteToCache(config, *response->current->file);
+    cache_manager->WriteToCache(*response->current->file);
     response->perf.index_save_to_disk = time.ElapsedMicrosecondsAndReset();
     timestamp_manager->UpdateCachedModificationTime(
         response->current->file->path,
@@ -420,7 +422,7 @@ void Indexer_Main(Config* config,
   std::unique_ptr<ICacheManager> cache_manager = ICacheManager::Make(config);
   auto* queue = QueueManager::instance();
   // Build one index per-indexer, as building the index acquires a global lock.
-  ClangIndex index;
+  auto indexer = IIndexer::MakeClangIndexer();
 
   while (true) {
     status->num_active_threads++;
@@ -435,12 +437,13 @@ void Indexer_Main(Config* config,
     // IndexMain_DoCreateIndexUpdate so we don't starve querydb from doing any
     // work. Running both also lets the user query the partially constructed
     // index.
-    bool did_parse =
+    std::unique_ptr<ICacheManager> cache_manager = ICacheManager::Make(config);
-        IndexMain_DoParse(config, working_files, file_consumer_shared,
+    bool did_parse = IndexMain_DoParse(
-                          timestamp_manager, import_manager, &index);
+        config, working_files, file_consumer_shared, timestamp_manager,
+        import_manager, cache_manager.get(), indexer.get());
 
     bool did_create_update =
-        IndexMain_DoCreateIndexUpdate(config, timestamp_manager);
+        IndexMain_DoCreateIndexUpdate(timestamp_manager, cache_manager.get());
 
     bool did_load_previous = IndexMain_LoadPreviousIndex(cache_manager.get());
 
@@ -583,17 +586,19 @@ bool QueryDb_ImportMain(Config* config,
   return did_work;
 }
 
-#if false
 TEST_SUITE("ImportPipeline") {
   TEST_CASE("hello") {
-    MultiQueueWaiter waiter;
+    MultiQueueWaiter querydb_waiter;
-    QueueManager::CreateInstance(&waiter);
+    MultiQueueWaiter indexer_waiter;
+    MultiQueueWaiter stdout_waiter;
+    QueueManager::CreateInstance(&querydb_waiter, &indexer_waiter,
+                                 &stdout_waiter);
     auto* queue = QueueManager::instance();
 
     std::string path = "foo.cc";
     std::vector<std::string> args = {};
     bool is_interactive = false;
-    optional<std::string> contents = std::string("void foo();");
+    std::string contents = std::string("void foo();");
     queue->index_request.Enqueue(
         Index_Request(path, args, is_interactive, contents));
 
@@ -602,12 +607,15 @@ TEST_SUITE("ImportPipeline") {
     FileConsumerSharedState file_consumer_shared;
     TimestampManager timestamp_manager;
     ImportManager import_manager;
-    ClangIndex index;
+
+    std::unique_ptr<ICacheManager> cache_manager = ICacheManager::MakeFake({});
+    auto indexer = IIndexer::MakeTestIndexer({{"foo.cc", 1}});
+
     IndexMain_DoParse(&config, &working_files, &file_consumer_shared,
-                      &timestamp_manager, &import_manager, &index);
+                      &timestamp_manager, &import_manager, cache_manager.get(),
+                      indexer.get());
 
     REQUIRE(queue->index_request.Size() == 0);
     REQUIRE(queue->do_id_map.Size() == 1);
   }
 }
-#endif