From 11d662393822bf0fc40426971f6fb765a4e919d0 Mon Sep 17 00:00:00 2001 From: Jacob Dufault Date: Thu, 20 Apr 2017 23:32:18 -0700 Subject: [PATCH] Update WorkingFile indexed content correctly. We copy the file contents we indexed over to the index cache folder. Then we load those file contents into the WorkingFile instance as needed. This means code lens should never get out of sync, as the index buffer cache will always be correct. --- src/cache.cc | 24 ++++++++++++---- src/cache.h | 11 +++++-- src/command_line.cc | 67 +++++++++++++++++++++++++++++++------------ src/platform.h | 2 ++ src/platform_linux.cc | 49 +++++++++++++++++++++++++++++++ src/platform_win.cc | 7 +++++ src/working_files.cc | 11 ++++--- src/working_files.h | 8 +++++- 8 files changed, 144 insertions(+), 35 deletions(-) diff --git a/src/cache.cc b/src/cache.cc index ebdda7a8..b6046cb6 100644 --- a/src/cache.cc +++ b/src/cache.cc @@ -1,27 +1,29 @@ #include "cache.h" #include "indexer.h" +#include "platform.h" +#include "language_server_api.h" #include namespace { -std::string GetCachedFileName(const std::string& cache_directory, std::string source_file) { +std::string GetCachedBaseFileName(const std::string& cache_directory, std::string source_file) { assert(!cache_directory.empty()); std::replace(source_file.begin(), source_file.end(), '\\', '_'); std::replace(source_file.begin(), source_file.end(), '/', '_'); std::replace(source_file.begin(), source_file.end(), ':', '_'); std::replace(source_file.begin(), source_file.end(), '.', '_'); - return cache_directory + source_file + ".json"; + return cache_directory + source_file; } } // namespace -std::unique_ptr LoadCachedFile(IndexerConfig* config, const std::string& filename) { +std::unique_ptr LoadCachedIndex(IndexerConfig* config, const std::string& filename) { if (!config->enableCacheRead) return nullptr; - optional file_content = ReadContent(GetCachedFileName(config->cacheDirectory, filename)); + optional file_content = ReadContent(GetCachedBaseFileName(config->cacheDirectory, filename) + ".json"); if (!file_content) return nullptr; @@ -32,14 +34,24 @@ std::unique_ptr LoadCachedFile(IndexerConfig* config, const std::st return nullptr; } +optional LoadCachedFileContents(IndexerConfig* config, const std::string& filename) { + if (!config->enableCacheRead) + return nullptr; + + return ReadContent(GetCachedBaseFileName(config->cacheDirectory, filename) + ".txt"); +} + void WriteToCache(IndexerConfig* config, const std::string& filename, IndexedFile& file) { if (!config->enableCacheWrite) return; - std::string indexed_content = Serialize(file); + std::string cache_basename = GetCachedBaseFileName(config->cacheDirectory, filename); + CopyFileTo(cache_basename + ".txt", filename); + + std::string indexed_content = Serialize(file); std::ofstream cache; - cache.open(GetCachedFileName(config->cacheDirectory, filename)); + cache.open(cache_basename + ".json"); assert(cache.good()); cache << indexed_content; cache.close(); diff --git a/src/cache.h b/src/cache.h index f01215cc..4cdb0e60 100644 --- a/src/cache.h +++ b/src/cache.h @@ -1,12 +1,17 @@ #pragma once -#include "language_server_api.h" - +#include #include #include +using std::experimental::optional; +using std::experimental::nullopt; + +struct IndexerConfig; struct IndexedFile; -std::unique_ptr LoadCachedFile(IndexerConfig* config, const std::string& filename); +std::unique_ptr LoadCachedIndex(IndexerConfig* config, const std::string& filename); + +optional LoadCachedFileContents(IndexerConfig* config, const std::string& filename); void WriteToCache(IndexerConfig* config, const std::string& filename, IndexedFile& file); \ No newline at end of file diff --git a/src/command_line.cc b/src/command_line.cc index a9d245f0..f607d8ba 100644 --- a/src/command_line.cc +++ b/src/command_line.cc @@ -893,7 +893,7 @@ void ImportCachedIndex(IndexerConfig* config, Timer time; - std::unique_ptr cache = LoadCachedFile(config, path); + std::unique_ptr cache = LoadCachedIndex(config, path); time.ResetAndPrint("Reading cached index from disk " + path); if (!cache) return; @@ -914,7 +914,7 @@ void ParseFile(IndexerConfig* config, Timer time; // Parse request and send a response. - std::unique_ptr cached_path_index = LoadCachedFile(config, path); + std::unique_ptr cached_path_index = LoadCachedIndex(config, path); if (cached_path_index) { // Give the user dependencies if requested. @@ -946,7 +946,7 @@ void ParseFile(IndexerConfig* config, if (new_index->path == path) cached_index = std::move(cached_path_index); else - cached_index = LoadCachedFile(config, new_index->path); + cached_index = LoadCachedIndex(config, new_index->path); time.ResetAndPrint("Loading cached index"); // Update dependencies on |new_index|, since they won't get reparsed if we @@ -1026,10 +1026,10 @@ bool IndexMain_DoCreateIndexUpdate( Timer time; IndexUpdate update = IndexUpdate::CreateDelta(response->previous_id_map.get(), response->current_id_map.get(), response->previous_index.get(), response->current_index.get()); - time.ResetAndPrint("Creating delta IndexUpdate"); + time.ResetAndPrint("[indexer] Creating delta IndexUpdate"); Index_OnIndexed reply(update); queue_on_indexed->Enqueue(std::move(reply)); - time.ResetAndPrint("Sending update to server"); + time.ResetAndPrint("[indexer] Sending update to server"); return true; } @@ -1048,7 +1048,7 @@ void IndexJoinIndexUpdates(Index_OnIndexedQueue* queue_on_indexed) { Timer time; root->update.Merge(to_join->update); - time.ResetAndPrint("Indexer joining two querydb updates"); + time.ResetAndPrint("[indexer] Joining two querydb updates"); } } @@ -1199,7 +1199,13 @@ void QueryDbMainLoop( case IpcId::TextDocumentDidOpen: { auto msg = static_cast(message.get()); //std::cerr << "Opening " << msg->params.textDocument.uri.GetPath() << std::endl; - working_files->OnOpen(msg->params); + WorkingFile* working_file = working_files->OnOpen(msg->params); + optional cached_file_contents = LoadCachedFileContents(config, msg->params.textDocument.uri.GetPath()); + if (cached_file_contents) + working_file->SetIndexContent(*cached_file_contents); + else + working_file->SetIndexContent(working_file->buffer_content); + break; } case IpcId::TextDocumentDidChange: { @@ -1219,15 +1225,14 @@ void QueryDbMainLoop( auto msg = static_cast(message.get()); std::string path = msg->params.textDocument.uri.GetPath(); - - // TODO: Update working file indexed content when we actually apply the - // index update. - WorkingFile* working_file = working_files->GetFileByFilename(path); - if (working_file) - working_file->SetIndexContent(working_file->buffer_content); - // Send an index update request. queue_do_index->Enqueue(Index_DoIndex(Index_DoIndex::Type::Parse, path, project->FindArgsForFile(path))); + + // Copy current buffer content so it can be applied when index request is done. + WorkingFile* working_file = working_files->GetFileByFilename(path); + if (working_file) + working_file->pending_new_index_content = working_file->buffer_content; + break; } @@ -1579,13 +1584,13 @@ void QueryDbMainLoop( response.id = msg->id; - std::cerr << "- Considering " << db->detailed_names.size() + std::cerr << "[querydb] Considering " << db->detailed_names.size() << " candidates for query " << msg->params.query << std::endl; std::string query = msg->params.query; for (int i = 0; i < db->detailed_names.size(); ++i) { if (response.result.size() >= config->maxWorkspaceSearchResults) { - std::cerr << "Query exceeded maximum number of responses (" << config->maxWorkspaceSearchResults << "), output may not contain all results" << std::endl; + std::cerr << "[querydb] - Query exceeded maximum number of responses (" << config->maxWorkspaceSearchResults << "), output may not contain all results" << std::endl; break; } @@ -1610,7 +1615,7 @@ void QueryDbMainLoop( } } - std::cerr << "- Found " << response.result.size() << " results for query " << query << std::endl; + std::cerr << "[querydb] - Found " << response.result.size() << " results for query " << query << std::endl; ipc->SendOutMessageToClient(response); break; } @@ -1634,6 +1639,7 @@ void QueryDbMainLoop( Index_OnIdMapped response; Timer time; + if (request->previous) { response.previous_id_map = MakeUnique(db, request->previous->id_cache); response.previous_index = std::move(request->previous); @@ -1641,7 +1647,7 @@ void QueryDbMainLoop( assert(request->current); response.current_id_map = MakeUnique(db, request->current->id_cache); - time.ResetAndPrint("Create IdMap " + request->current->path); + time.ResetAndPrint("[querydb] Create IdMap " + request->current->path); response.current_index = std::move(request->current); queue_on_id_mapped->Enqueue(std::move(response)); @@ -1653,8 +1659,31 @@ void QueryDbMainLoop( break; Timer time; + + for (auto& updated_file : response->update.files_def_update) { + // TODO: We're reading a file on querydb thread. This is slow!! If it is a + // problem in practice we need to create a file reader queue, dispatch the + // read to it, get a response, and apply the new index then. + WorkingFile* working_file = working_files->GetFileByFilename(updated_file.path); + if (working_file) { + if (working_file->pending_new_index_content) { + working_file->SetIndexContent(*working_file->pending_new_index_content); + working_file->pending_new_index_content = nullopt; + time.ResetAndPrint("[querydb] Update WorkingFile index contents (via in-memory buffer) for " + updated_file.path); + } + else { + optional cached_file_contents = LoadCachedFileContents(config, updated_file.path); + if (cached_file_contents) + working_file->SetIndexContent(*cached_file_contents); + else + working_file->SetIndexContent(working_file->buffer_content); + time.ResetAndPrint("[querydb] Update WorkingFile index contents (via disk load) for " + updated_file.path); + } + } + } + db->ApplyIndexUpdate(&response->update); - time.ResetAndPrint("Applying index update"); + time.ResetAndPrint("[querydb] Applying index update"); } } diff --git a/src/platform.h b/src/platform.h index 47eab34a..c4abcf53 100644 --- a/src/platform.h +++ b/src/platform.h @@ -39,5 +39,7 @@ void SetCurrentThreadName(const std::string& thread_name); int64_t GetLastModificationTime(const std::string& absolute_path); +void CopyFileTo(const std::string& destination, const std::string& source); + // Returns any clang arguments that are specific to the current platform. std::vector GetPlatformClangArguments(); \ No newline at end of file diff --git a/src/platform_linux.cc b/src/platform_linux.cc index 04289b53..3cf0720f 100644 --- a/src/platform_linux.cc +++ b/src/platform_linux.cc @@ -23,6 +23,9 @@ #include // required for stat.h #include +#include +#include +#include #include /* For O_* constants */ #include /* For mode constants */ @@ -182,6 +185,52 @@ int64_t GetLastModificationTime(const std::string& absolute_path) { return buf.st_mtime; } +// See http://stackoverflow.com/q/13198627 +void CopyFileTo(const std::string& dest, const std::string& source) { + char buf[4096]; + ssize_t nread; + int saved_errno; + + int fd_from = open(source.c_str(), O_RDONLY); + if (fd_from < 0) + return; + + int fd_to = open(dest.c_str(), O_WRONLY | O_CREAT | O_EXCL, 0666); + if (fd_to < 0) + goto out_error; + + while (nread = read(fd_from, buf, sizeof buf), nread > 0) { + char *out_ptr = buf; + ssize_t nwritten; + + do { + nwritten = write(fd_to, out_ptr, nread); + + if (nwritten >= 0) { + nread -= nwritten; + out_ptr += nwritten; + } + else if (errno != EINTR) + goto out_error; + } while (nread > 0); + } + + if (nread == 0) { + if (close(fd_to) < 0) { + fd_to = -1; + goto out_error; + } + close(fd_from); + + return; + } + +out_error: + close(fd_from); + if (fd_to >= 0) + close(fd_to); +} + std::vector GetPlatformClangArguments() { // TODO: use install config variable for path? return { diff --git a/src/platform_win.cc b/src/platform_win.cc index e550ff45..b2669076 100644 --- a/src/platform_win.cc +++ b/src/platform_win.cc @@ -202,6 +202,13 @@ int64_t GetLastModificationTime(const std::string& absolute_path) { return buf.st_mtime; } +void CopyFileTo(const std::string& destination, const std::string& source) { + CopyFile( + source.c_str(), + destination.c_str(), + false /*failIfExists*/); +} + std::vector GetPlatformClangArguments() { return { "-fms-compatibility", diff --git a/src/working_files.cc b/src/working_files.cc index cf789f3a..8f19b626 100644 --- a/src/working_files.cc +++ b/src/working_files.cc @@ -24,8 +24,8 @@ int GetOffsetForPosition(lsPosition position, const std::string& content) { WorkingFile::WorkingFile(const std::string& filename, const std::string& buffer_content) : filename(filename), buffer_content(buffer_content) { OnBufferContentUpdated(); - // TODO: use cached index file contents - SetIndexContent(buffer_content); + + // SetIndexContent gets called when the file is opened. } void WorkingFile::SetIndexContent(const std::string& index_content) { @@ -155,21 +155,20 @@ WorkingFile* WorkingFiles::GetFileByFilename(const std::string& filename) { return nullptr; } -void WorkingFiles::OnOpen(const Ipc_TextDocumentDidOpen::Params& open) { +WorkingFile* WorkingFiles::OnOpen(const Ipc_TextDocumentDidOpen::Params& open) { std::string filename = open.textDocument.uri.GetPath(); std::string content = open.textDocument.text; // The file may already be open. if (WorkingFile* file = GetFileByFilename(filename)) { file->version = open.textDocument.version; - // TODO: Load saved indexed content and not the initial buffer content. - file->SetIndexContent(content); file->buffer_content = content; file->OnBufferContentUpdated(); - return; + return file; } files.push_back(MakeUnique(filename, content)); + return files[files.size() - 1].get(); } void WorkingFiles::OnChange(const Ipc_TextDocumentDidChange::Params& change) { diff --git a/src/working_files.h b/src/working_files.h index bd4c27ed..497bc771 100644 --- a/src/working_files.h +++ b/src/working_files.h @@ -27,6 +27,12 @@ struct WorkingFile { // Note: The items in the value entry are 1-based liness. std::unordered_map> all_buffer_lines_lookup; + // The buffer content when the file was saved. This will be applied with + // SetIndexContent when the index has been updated and applied. + // + // This is an optimization that lets us avoid a disk read on the querydb + // thread when actively editing and saving a file. + optional pending_new_index_content; WorkingFile(const std::string& filename, const std::string& buffer_content); @@ -48,7 +54,7 @@ struct WorkingFile { struct WorkingFiles { // Find the file with the given filename. WorkingFile* GetFileByFilename(const std::string& filename); - void OnOpen(const Ipc_TextDocumentDidOpen::Params& open); + WorkingFile* OnOpen(const Ipc_TextDocumentDidOpen::Params& open); void OnChange(const Ipc_TextDocumentDidChange::Params& change); void OnClose(const Ipc_TextDocumentDidClose::Params& close);