From 6cbaf8980254355660f6febd7526ba7f049f3882 Mon Sep 17 00:00:00 2001 From: eidheim Date: Sat, 17 Jun 2023 19:44:10 +0200 Subject: [PATCH] Cleanup of state handling and parsing in order to limit while loops with sleeps, and removed standard c++ version from libclang arguments due to C header crashes --- src/CMakeLists.txt | 1 + src/compile_commands.cpp | 6 - src/debug_lldb.cpp | 15 +-- src/debug_lldb.hpp | 2 +- src/directories.cpp | 4 +- src/directories.hpp | 3 +- src/source_clang.cpp | 145 ++++++++++++----------- src/source_clang.hpp | 17 +-- src/source_diff.cpp | 191 ++++++++++++++++--------------- src/source_diff.hpp | 20 ++-- src/source_language_protocol.cpp | 47 ++++---- src/source_language_protocol.hpp | 3 +- src/workers.cpp | 79 +++++++++++++ src/workers.hpp | 38 ++++++ 14 files changed, 342 insertions(+), 229 deletions(-) create mode 100644 src/workers.cpp create mode 100644 src/workers.hpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c567d1b..868462d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -27,6 +27,7 @@ set(JUCI_SHARED_FILES tooltips.cpp usages_clang.cpp utility.cpp + workers.cpp ) if(LIBLLDB_FOUND) list(APPEND JUCI_SHARED_FILES debug_lldb.cpp) diff --git a/src/compile_commands.cpp b/src/compile_commands.cpp index e2d1634..b597012 100644 --- a/src/compile_commands.cpp +++ b/src/compile_commands.cpp @@ -36,8 +36,6 @@ CompileCommands::CompileCommands(const boost::filesystem::path &build_path) { } std::vector CompileCommands::get_arguments(const boost::filesystem::path &build_path, const boost::filesystem::path &file_path) { - std::string default_std_argument = "-std=c++1y"; - auto extension = file_path.extension().string(); bool is_header = CompileCommands::is_header(file_path) || extension.empty(); // Include std C++ headers that are without extensions @@ -84,11 +82,7 @@ std::vector CompileCommands::get_arguments(const boost::filesystem: } } } - else - arguments.emplace_back(default_std_argument); } - else - arguments.emplace_back(default_std_argument); #if defined(__APPLE__) static auto resource_path = []() -> std::string { diff --git a/src/debug_lldb.cpp b/src/debug_lldb.cpp index 8110abf..83861fe 100644 --- a/src/debug_lldb.cpp +++ b/src/debug_lldb.cpp @@ -15,7 +15,7 @@ void log(const char *msg, void *) { std::cout << "debugger log: " << msg << std::endl; } -Debug::LLDB::LLDB() : state(lldb::StateType::eStateInvalid), buffer_size(131072) { +Debug::LLDB::LLDB() : listener("juCi++ lldb listener"), state(lldb::StateType::eStateInvalid), buffer_size(131072) { #ifndef __APPLE__ auto debug_server_path = filesystem::get_executable("lldb-server").string(); if(debug_server_path != "lldb-server") @@ -99,7 +99,6 @@ void Debug::LLDB::start(const std::string &command, const boost::filesystem::pat initialized = true; lldb::SBDebugger::Initialize(); debugger = std::make_unique(lldb::SBDebugger::Create(true, log, nullptr)); - listener = std::make_unique("juCi++ lldb listener"); } // Create executable string and argument array @@ -140,7 +139,7 @@ void Debug::LLDB::start(const std::string &command, const boost::filesystem::pat lldb::SBError error; if(!remote_host.empty()) { auto connect_string = "connect://" + remote_host; - process = std::make_unique(target.ConnectRemote(*listener, connect_string.c_str(), "gdb-remote", error)); + process = std::make_unique(target.ConnectRemote(listener, connect_string.c_str(), "gdb-remote", error)); if(error.Fail()) { Terminal::get().async_print(std::string("\e[31mError (debug)\e[m: ") + error.GetCString() + '\n', true); for(auto &handler : on_exit) @@ -149,7 +148,7 @@ void Debug::LLDB::start(const std::string &command, const boost::filesystem::pat } lldb::SBEvent event; while(true) { - if(listener->GetNextEvent(event)) { + if(listener.WaitForEvent(std::numeric_limits::max(), event)) { if((event.GetType() & lldb::SBProcess::eBroadcastBitStateChanged) > 0) { auto state = process->GetStateFromEvent(event); this->state = state; @@ -183,7 +182,7 @@ void Debug::LLDB::start(const std::string &command, const boost::filesystem::pat environment.emplace_back(environ[c]); environment.emplace_back(nullptr); - process = std::make_unique(target.Launch(*listener, argv.data(), environment.data(), nullptr, nullptr, nullptr, path.string().c_str(), lldb::eLaunchFlagNone, false, error)); + process = std::make_unique(target.Launch(listener, argv.data(), environment.data(), nullptr, nullptr, nullptr, path.string().c_str(), lldb::eLaunchFlagNone, false, error)); } if(error.Fail()) { @@ -206,8 +205,8 @@ void Debug::LLDB::start(const std::string &command, const boost::filesystem::pat debug_thread = std::thread([this]() { lldb::SBEvent event; while(true) { - LockGuard lock(mutex); - if(listener->GetNextEvent(event)) { + if(listener.WaitForEvent(std::numeric_limits::max(), event)) { + LockGuard lock(mutex); if((event.GetType() & lldb::SBProcess::eBroadcastBitStateChanged) > 0) { auto state = process->GetStateFromEvent(event); this->state = state; @@ -252,8 +251,6 @@ void Debug::LLDB::start(const std::string &command, const boost::filesystem::pat Terminal::get().async_print(std::string(buffer, n), true); } } - lock.unlock(); - std::this_thread::sleep_for(std::chrono::milliseconds(200)); } }); } diff --git a/src/debug_lldb.hpp b/src/debug_lldb.hpp index 905aec1..2de1225 100644 --- a/src/debug_lldb.hpp +++ b/src/debug_lldb.hpp @@ -88,8 +88,8 @@ namespace Debug { private: std::tuple, std::string, std::vector> parse_run_arguments(const std::string &command); + lldb::SBListener listener; std::unique_ptr debugger GUARDED_BY(mutex); - std::unique_ptr listener GUARDED_BY(mutex); std::unique_ptr process GUARDED_BY(mutex); std::thread debug_thread; diff --git a/src/directories.cpp b/src/directories.cpp index e7ae02b..d048a75 100644 --- a/src/directories.cpp +++ b/src/directories.cpp @@ -404,7 +404,7 @@ Directories::Directories() : Gtk::ListViewText(1) { } Directories::~Directories() { - thread_pool.shutdown(true); + worker.stop(); } void Directories::open(const boost::filesystem::path &dir_path) { @@ -794,7 +794,7 @@ void Directories::colorize_path(boost::filesystem::path dir_path, bool include_p }; if(it->second.repository) { - thread_pool.push([this, repository = it->second.repository, colorize = std::move(colorize)] { + worker.post([this, repository = it->second.repository, colorize = std::move(colorize)] { Git::Repository::Status status; try { status = repository->get_status(); diff --git a/src/directories.hpp b/src/directories.hpp index f38497c..3d428bc 100644 --- a/src/directories.hpp +++ b/src/directories.hpp @@ -2,6 +2,7 @@ #include "boost/filesystem.hpp" #include "dispatcher.hpp" #include "git.hpp" +#include "workers.hpp" #include #include #include @@ -81,7 +82,7 @@ private: std::unordered_map directories; - Glib::ThreadPool thread_pool; + Workers worker; Dispatcher dispatcher; Gtk::Menu menu; diff --git a/src/source_clang.cpp b/src/source_clang.cpp index 66d6bae..9550ae1 100644 --- a/src/source_clang.cpp +++ b/src/source_clang.cpp @@ -2,6 +2,7 @@ #include "config.hpp" #include "project_build.hpp" #include "terminal.hpp" +#include #ifdef JUCI_ENABLE_DEBUG #include "debug_lldb.hpp" #endif @@ -92,10 +93,7 @@ void Source::ClangViewParse::configure() { void Source::ClangViewParse::parse_initialize() { hide_tooltips(); parsed = false; - if(parse_thread.joinable()) - parse_thread.join(); parse_state = ParseState::processing; - parse_process_state = ParseProcessState::starting; auto buffer_ = get_buffer()->get_text(); auto &buffer_raw = const_cast(buffer_.raw()); @@ -140,60 +138,56 @@ void Source::ClangViewParse::parse_initialize() { status_state = "parsing..."; if(update_status_state) update_status_state(this); - parse_thread = std::thread([this]() { - while(true) { - while(parse_state == ParseState::processing && parse_process_state != ParseProcessState::starting && parse_process_state != ParseProcessState::processing) - std::this_thread::sleep_for(std::chrono::milliseconds(10)); - if(parse_state != ParseState::processing) - break; - auto expected = ParseProcessState::starting; - if(parse_process_state.compare_exchange_strong(expected, ParseProcessState::preprocessing)) { - dispatcher.post([this] { - auto expected = ParseProcessState::preprocessing; - if(parse_mutex.try_lock()) { - if(parse_process_state.compare_exchange_strong(expected, ParseProcessState::processing)) - parse_thread_buffer = get_buffer()->get_text(); - parse_mutex.unlock(); - } - else - parse_process_state.compare_exchange_strong(expected, ParseProcessState::starting); - }); - } - else if(parse_process_state == ParseProcessState::processing && parse_mutex.try_lock()) { + size_t count = ++parse_count; + dispatcher.post([this, count] { // Do not run in constructor to avoid data race on vptr (detected by thread sanitizer) + if(count == parse_count) + parse(count); + }); +} + +void Source::ClangViewParse::parse(size_t count) { + if(parse_state != ParseState::processing) + return; + if(parse_mutex.try_lock()) { + parse_thread_buffer = get_buffer()->get_text(); + parse_mutex.unlock(); + + worker.post([this, count] { + if(count != parse_count || parse_state != ParseState::processing) + return; + if(parse_mutex.try_lock()) { auto &parse_thread_buffer_raw = const_cast(parse_thread_buffer.raw()); if(is_language({"chdr", "cpphdr"})) clangmm::remove_include_guard(parse_thread_buffer_raw); auto status = clang_tu->reparse(parse_thread_buffer_raw); if(status == 0) { - auto expected = ParseProcessState::processing; - if(parse_process_state.compare_exchange_strong(expected, ParseProcessState::postprocessing)) { - clang_tokens = clang_tu->get_tokens(); - clang_tokens_offsets.clear(); - clang_tokens_offsets.reserve(clang_tokens->size()); - for(auto &token : *clang_tokens) - clang_tokens_offsets.emplace_back(token.get_source_range().get_offsets()); - clang_diagnostics = clang_tu->get_diagnostics(); + if(count != parse_count || parse_state != ParseState::processing) { parse_mutex.unlock(); - dispatcher.post([this] { - if(parse_mutex.try_lock()) { - auto expected = ParseProcessState::postprocessing; - if(parse_process_state.compare_exchange_strong(expected, ParseProcessState::idle)) { - update_syntax(); - update_diagnostics(); - parsed = true; - status_state = ""; - if(update_status_state) - update_status_state(this); - } - parse_mutex.unlock(); - } - }); + return; } - else - parse_mutex.unlock(); + clang_tokens = clang_tu->get_tokens(); + clang_tokens_offsets.clear(); + clang_tokens_offsets.reserve(clang_tokens->size()); + for(auto &token : *clang_tokens) + clang_tokens_offsets.emplace_back(token.get_source_range().get_offsets()); + clang_diagnostics = clang_tu->get_diagnostics(); + parse_mutex.unlock(); + dispatcher.post([this, count] { + if(count != parse_count || parse_state != ParseState::processing) + return; + if(parse_mutex.try_lock()) { + update_syntax(); + update_diagnostics(); + parsed = true; + status_state = ""; + if(update_status_state) + update_status_state(this); + parse_mutex.unlock(); + } + }); } else { - parse_state = ParseState::stop; + parse_state = ParseState::stopped; parse_mutex.unlock(); dispatcher.post([this] { Terminal::get().print("\e[31mError\e[m: failed to reparse " + filesystem::get_short_path(this->file_path).string() + "\n", true); @@ -206,8 +200,19 @@ void Source::ClangViewParse::parse_initialize() { }); } } - } - }); + }); + } + else { + delayed_reparse_connection.disconnect(); + size_t count = ++parse_count; + delayed_reparse_connection = Glib::signal_timeout().connect( + [this, count] { + if(count == parse_count) + parse(count); + return false; + }, + 100); + } } void Source::ClangViewParse::soft_reparse(bool delayed) { @@ -218,16 +223,15 @@ void Source::ClangViewParse::soft_reparse(bool delayed) { if(parse_state != ParseState::processing) return; - parse_process_state = ParseProcessState::idle; - - auto reparse = [this] { + size_t count = ++parse_count; + auto reparse = [this, count] { parsed = false; - auto expected = ParseProcessState::idle; - if(parse_process_state.compare_exchange_strong(expected, ParseProcessState::starting)) { - status_state = "parsing..."; - if(update_status_state) - update_status_state(this); - } + if(count != parse_count) + return false; + status_state = "parsing..."; + if(update_status_state) + update_status_state(this); + parse(count); return false; }; if(delayed) @@ -837,7 +841,7 @@ Source::ClangViewAutocomplete::ClangViewAutocomplete(const boost::filesystem::pa }; autocomplete.stop_parse = [this]() { - parse_process_state = ParseProcessState::idle; + ++parse_count; }; // Activate argument completions @@ -2043,7 +2047,7 @@ bool Source::ClangViewRefactor::wait_parsing() { while(true) { size_t not_parsed = 0; for(auto &clang_view : not_parsed_clang_views) { - if(clang_view->parse_state == ParseState::stop) { + if(clang_view->parse_state == ParseState::stopped) { Info::get().print("Canceled due to parsing error in " + clang_view->file_path.string()); return false; } @@ -2126,6 +2130,8 @@ void Source::ClangView::full_reparse() { if(parse_state != ParseState::processing) return; + ++parse_count; + if(full_reparse_running) { delayed_full_reparse_connection = Glib::signal_timeout().connect( [this] { @@ -2137,19 +2143,17 @@ void Source::ClangView::full_reparse() { } full_reparse_needed = false; - - parse_process_state = ParseProcessState::idle; autocomplete.state = Autocomplete::State::idle; auto expected = ParseState::processing; - if(!parse_state.compare_exchange_strong(expected, ParseState::restarting)) + if(!parse_state.compare_exchange_strong(expected, ParseState::idle)) return; full_reparse_running = true; if(full_reparse_thread.joinable()) full_reparse_thread.join(); full_reparse_thread = std::thread([this]() { - if(parse_thread.joinable()) - parse_thread.join(); + ++parse_count; + worker.restart(); if(autocomplete.thread.joinable()) autocomplete.thread.join(); dispatcher.post([this] { @@ -2175,6 +2179,7 @@ void Source::ClangView::async_delete() { Usages::Clang::cache_in_progress(); delayed_reparse_connection.disconnect(); + delayed_full_reparse_connection.disconnect(); if(full_reparse_needed) full_reparse(); else if(soft_reparse_needed || !parsed) @@ -2182,9 +2187,9 @@ void Source::ClangView::async_delete() { auto before_parse_time = std::time(nullptr); delete_thread = std::thread([this, before_parse_time, project_paths_in_use = std::move(project_paths_in_use), buffer_modified = get_buffer()->get_modified()] { - while(!parsed && parse_state != ParseState::stop) + while(!parsed && parse_state != ParseState::stopped) std::this_thread::sleep_for(std::chrono::milliseconds(10)); - parse_state = ParseState::stop; + parse_state = ParseState::stopped; if(buffer_modified) { std::ifstream stream(file_path.string(), std::ios::binary); @@ -2209,8 +2214,8 @@ void Source::ClangView::async_delete() { if(full_reparse_thread.joinable()) full_reparse_thread.join(); - if(parse_thread.joinable()) - parse_thread.join(); + ++parse_count; + worker.stop(); if(autocomplete.thread.joinable()) autocomplete.thread.join(); do_delete_object(); diff --git a/src/source_clang.hpp b/src/source_clang.hpp index 95b03be..78fd573 100644 --- a/src/source_clang.hpp +++ b/src/source_clang.hpp @@ -5,6 +5,7 @@ #include "mutex.hpp" #include "source.hpp" #include "terminal.hpp" +#include "workers.hpp" #include #include #include @@ -14,16 +15,9 @@ namespace Source { class ClangViewParse : public View { protected: enum class ParseState { - processing, - restarting, - stop - }; - enum class ParseProcessState { idle, - starting, - preprocessing, processing, - postprocessing + stopped }; public: @@ -37,7 +31,10 @@ namespace Source { protected: Dispatcher dispatcher; + Workers worker; + std::atomic parse_count = {0}; void parse_initialize(); + void parse(size_t last_count); std::unique_ptr clang_tu; std::unique_ptr clang_tokens; std::vector> clang_tokens_offsets; @@ -47,9 +44,7 @@ namespace Source { void show_type_tooltips(const Gdk::Rectangle &rectangle) override; Mutex parse_mutex; - std::thread parse_thread; - std::atomic parse_state; - std::atomic parse_process_state; + std::atomic parse_state = {ParseState::idle}; CXCompletionString selected_completion_string = nullptr; diff --git a/src/source_diff.cpp b/src/source_diff.cpp index 71b7b2e..356e056 100644 --- a/src/source_diff.cpp +++ b/src/source_diff.cpp @@ -52,10 +52,10 @@ Source::DiffView::~DiffView() { monitor_changed_connection.disconnect(); delayed_buffer_changed_connection.disconnect(); delayed_monitor_changed_connection.disconnect(); + delayed_reparse_connection.disconnect(); - parse_stop = true; - if(parse_thread.joinable()) - parse_thread.join(); + ++parse_count; + worker.stop(); } } @@ -82,6 +82,7 @@ void Source::DiffView::configure() { green.set_green(normal_color.get_green() + factor * (green.get_green() - normal_color.get_green())); green.set_blue(normal_color.get_blue() + factor * (green.get_blue() - normal_color.get_blue())); + LockGuard lock(parse_mutex); if(Config::get().source.show_git_diff) { if(repository) return; @@ -94,13 +95,10 @@ void Source::DiffView::configure() { delayed_buffer_changed_connection.disconnect(); delayed_monitor_changed_connection.disconnect(); - parse_stop = true; - if(parse_thread.joinable()) - parse_thread.join(); - repository = nullptr; - - LockGuard lock(parse_mutex); + ++parse_count; + worker.restart(); diff = nullptr; + repository = nullptr; return; } else @@ -114,8 +112,6 @@ void Source::DiffView::configure() { } get_gutter(Gtk::TextWindowType::TEXT_WINDOW_LEFT)->insert(renderer.get(), -40); - parse_state = ParseState::starting; - parse_stop = false; monitor_changed = false; buffer_insert_connection = get_buffer()->signal_insert().connect( @@ -140,11 +136,12 @@ void Source::DiffView::configure() { get_buffer()->remove_tag(renderer->tag_removed_above, start_iter, end_iter); get_buffer()->remove_tag(renderer->tag_removed_below, start_iter, end_iter); } - parse_state = ParseState::idle; delayed_buffer_changed_connection.disconnect(); + size_t count = ++parse_count; delayed_buffer_changed_connection = Glib::signal_timeout().connect( - [this]() { - parse_state = ParseState::starting; + [this, count]() { + if(count == parse_count) + parse(count); return false; }, 250); @@ -157,11 +154,12 @@ void Source::DiffView::configure() { if(start_iter.get_line() == end_iter.get_line() && start_iter.has_tag(renderer->tag_added)) return; - parse_state = ParseState::idle; delayed_buffer_changed_connection.disconnect(); + size_t count = ++parse_count; delayed_buffer_changed_connection = Glib::signal_timeout().connect( - [this]() { - parse_state = ParseState::starting; + [this, count]() { + if(count == parse_count) + parse(count); return false; }, 250); @@ -173,25 +171,27 @@ void Source::DiffView::configure() { Gio::FileMonitorEvent monitor_event) { if(monitor_event != Gio::FileMonitorEvent::FILE_MONITOR_EVENT_CHANGES_DONE_HINT) { delayed_monitor_changed_connection.disconnect(); + size_t count = ++parse_count; delayed_monitor_changed_connection = Glib::signal_timeout().connect( - [this]() { + [this, count]() { monitor_changed = true; - parse_state = ParseState::starting; - LockGuard lock(parse_mutex); - diff = nullptr; + { + LockGuard lock(parse_mutex); + diff = nullptr; + } + if(count == parse_count) + parse(count); return false; }, 500); } }); - parse_thread = std::thread([this]() { + worker.post([this] { std::string status_branch; try { - { - LockGuard lock(parse_mutex); - diff = get_diff(); - } + LockGuard lock(parse_mutex); + diff = get_diff(); status_branch = repository->get_branch(); } catch(const std::exception &) { @@ -202,81 +202,86 @@ void Source::DiffView::configure() { if(update_status_branch) update_status_branch(this); }); + }); - try { - while(true) { - while(!parse_stop && parse_state != ParseState::starting && parse_state != ParseState::processing) - std::this_thread::sleep_for(std::chrono::milliseconds(10)); - if(parse_stop) - break; - auto expected = ParseState::starting; - if(parse_state.compare_exchange_strong(expected, ParseState::preprocessing)) { - dispatcher.post([this] { - auto expected = ParseState::preprocessing; - if(parse_mutex.try_lock()) { - if(parse_state.compare_exchange_strong(expected, ParseState::processing)) - parse_buffer = get_buffer()->get_text(); - parse_mutex.unlock(); - } - else - parse_state.compare_exchange_strong(expected, ParseState::starting); - }); - } - else if(parse_state == ParseState::processing && parse_mutex.try_lock()) { - bool expected_monitor_changed = true; - if(monitor_changed.compare_exchange_strong(expected_monitor_changed, false)) { - try { - diff = get_diff(); - dispatcher.post([this, status_branch = repository->get_branch()] { - this->status_branch = status_branch; - if(update_status_branch) - update_status_branch(this); - }); - } - catch(const std::exception &) { - dispatcher.post([this] { - get_buffer()->remove_tag(renderer->tag_added, get_buffer()->begin(), get_buffer()->end()); - get_buffer()->remove_tag(renderer->tag_modified, get_buffer()->begin(), get_buffer()->end()); - get_buffer()->remove_tag(renderer->tag_removed, get_buffer()->begin(), get_buffer()->end()); - get_buffer()->remove_tag(renderer->tag_removed_below, get_buffer()->begin(), get_buffer()->end()); - get_buffer()->remove_tag(renderer->tag_removed_above, get_buffer()->begin(), get_buffer()->end()); - renderer->queue_draw(); - this->status_branch = ""; - if(update_status_branch) - update_status_branch(this); - }); - } + parse(++parse_count); +} + +void Source::DiffView::parse(size_t count) { + if(parse_mutex.try_lock()) { + parse_buffer = get_buffer()->get_text(); + parse_mutex.unlock(); + worker.post([this, count] { + if(count != parse_count) + return; + if(parse_mutex.try_lock()) { + bool expected_monitor_changed = true; + if(monitor_changed.compare_exchange_strong(expected_monitor_changed, false)) { + try { + diff = get_diff(); + dispatcher.post([this, status_branch = repository->get_branch()] { + this->status_branch = status_branch; + if(update_status_branch) + update_status_branch(this); + }); } + catch(const std::exception &) { + dispatcher.post([this] { + get_buffer()->remove_tag(renderer->tag_added, get_buffer()->begin(), get_buffer()->end()); + get_buffer()->remove_tag(renderer->tag_modified, get_buffer()->begin(), get_buffer()->end()); + get_buffer()->remove_tag(renderer->tag_removed, get_buffer()->begin(), get_buffer()->end()); + get_buffer()->remove_tag(renderer->tag_removed_below, get_buffer()->begin(), get_buffer()->end()); + get_buffer()->remove_tag(renderer->tag_removed_above, get_buffer()->begin(), get_buffer()->end()); + renderer->queue_draw(); + this->status_branch = ""; + if(update_status_branch) + update_status_branch(this); + }); + } + } - Git::Repository::Diff::Lines diff_lines; - if(diff) + Git::Repository::Diff::Lines diff_lines; + if(diff) { + try { diff_lines = diff->get_lines(parse_buffer.raw()); - auto expected = ParseState::processing; - if(parse_state.compare_exchange_strong(expected, ParseState::postprocessing)) { - parse_mutex.unlock(); - dispatcher.post([this, diff_lines = std::move(diff_lines)] { - auto expected = ParseState::postprocessing; - if(parse_state.compare_exchange_strong(expected, ParseState::idle)) - update_tags(diff_lines); - }); } - else + catch(const std::exception &) { + dispatcher.post([this] { + get_buffer()->remove_tag(renderer->tag_added, get_buffer()->begin(), get_buffer()->end()); + get_buffer()->remove_tag(renderer->tag_modified, get_buffer()->begin(), get_buffer()->end()); + get_buffer()->remove_tag(renderer->tag_removed, get_buffer()->begin(), get_buffer()->end()); + get_buffer()->remove_tag(renderer->tag_removed_below, get_buffer()->begin(), get_buffer()->end()); + get_buffer()->remove_tag(renderer->tag_removed_above, get_buffer()->begin(), get_buffer()->end()); + renderer->queue_draw(); + this->status_branch = ""; + if(update_status_branch) + update_status_branch(this); + }); parse_mutex.unlock(); + return; + } } + parse_mutex.unlock(); + + dispatcher.post([this, diff_lines = std::move(diff_lines), count] { + if(count != parse_count) + return; + update_tags(diff_lines); + }); } - } - catch(const std::exception &e) { - dispatcher.post([this, e_what = e.what()] { - get_buffer()->remove_tag(renderer->tag_added, get_buffer()->begin(), get_buffer()->end()); - get_buffer()->remove_tag(renderer->tag_modified, get_buffer()->begin(), get_buffer()->end()); - get_buffer()->remove_tag(renderer->tag_removed, get_buffer()->begin(), get_buffer()->end()); - get_buffer()->remove_tag(renderer->tag_removed_below, get_buffer()->begin(), get_buffer()->end()); - get_buffer()->remove_tag(renderer->tag_removed_above, get_buffer()->begin(), get_buffer()->end()); - renderer->queue_draw(); - Terminal::get().print(std::string("\e[31mError (git)\e[m: ") + e_what + '\n', true); - }); - } - }); + }); + } + else { + delayed_reparse_connection.disconnect(); + size_t count = ++parse_count; + delayed_reparse_connection = Glib::signal_timeout().connect( + [this, count] { + if(count == parse_count) + parse(count); + return false; + }, + 100); + } } void Source::DiffView::rename(const boost::filesystem::path &path) { diff --git a/src/source_diff.hpp b/src/source_diff.hpp index 80a39b3..5bd1106 100644 --- a/src/source_diff.hpp +++ b/src/source_diff.hpp @@ -3,6 +3,7 @@ #include "git.hpp" #include "mutex.hpp" #include "source_base.hpp" +#include "workers.hpp" #include #include #include @@ -11,14 +12,6 @@ namespace Source { class DiffView : virtual public Source::BaseView { - enum class ParseState { - idle, - starting, - preprocessing, - processing, - postprocessing - }; - class Renderer : public Gsv::GutterRenderer { public: Renderer(); @@ -55,24 +48,25 @@ namespace Source { private: std::unique_ptr renderer; Dispatcher dispatcher; + Workers worker; + std::atomic parse_count = {0}; Mutex parse_mutex; - std::shared_ptr repository; + std::shared_ptr repository GUARDED_BY(parse_mutex); std::unique_ptr diff GUARDED_BY(parse_mutex); - std::unique_ptr get_diff(); + std::unique_ptr get_diff() REQUIRES(parse_mutex); - std::thread parse_thread; - std::atomic parse_state; - std::atomic parse_stop; Glib::ustring parse_buffer GUARDED_BY(parse_mutex); sigc::connection buffer_insert_connection; sigc::connection buffer_erase_connection; sigc::connection monitor_changed_connection; sigc::connection delayed_buffer_changed_connection; sigc::connection delayed_monitor_changed_connection; + sigc::connection delayed_reparse_connection; std::atomic monitor_changed; + void parse(size_t count); void update_tags(const Git::Repository::Diff::Lines &diff_lines); }; } // namespace Source diff --git a/src/source_language_protocol.cpp b/src/source_language_protocol.cpp index 617a603..1d26e22 100644 --- a/src/source_language_protocol.cpp +++ b/src/source_language_protocol.cpp @@ -700,7 +700,9 @@ void LanguageProtocol::Client::handle_server_request(const boost::variant &language, std::string language_id_, std::string language_server_) - : Source::BaseView(file_path, language), Source::GenericView(file_path, language, false), uri(filesystem::get_uri_from_path(file_path)), uri_escaped(JSON::escape_string(uri)), language_id(std::move(language_id_)), language_server(std::move(language_server_)), client(LanguageProtocol::Client::get(file_path, language_id, language_server)) { + : Source::BaseView(file_path, language), Source::GenericView(file_path, language, false), uri(filesystem::get_uri_from_path(file_path)), + uri_escaped(JSON::escape_string(uri)), language_id(std::move(language_id_)), language_server(std::move(language_server_)), + client(LanguageProtocol::Client::get(file_path, language_id, language_server)) { initialize(); } @@ -767,7 +769,8 @@ Source::LanguageProtocolView::~LanguageProtocolView() { } write_notification("textDocument/didClose"); - thread_pool.shutdown(true); + ++update_diagnostics_async_count; + worker.stop(); client->remove(this); } @@ -849,8 +852,8 @@ void Source::LanguageProtocolView::rename(const boost::filesystem::path &path) { } write_notification("textDocument/didClose"); - while(thread_pool.unprocessed()) // TODO: does this include tasks in progress? - std::this_thread::sleep_for(std::chrono::milliseconds(10)); + ++update_diagnostics_async_count; + worker.restart(); client->remove(this); dispatcher.reset(); @@ -1940,7 +1943,7 @@ void Source::LanguageProtocolView::setup_autocomplete() { }; autocomplete->set_tooltip_buffer = [this](unsigned int index) -> std::function { - size_t last_count = ++set_tooltip_count; + size_t count = ++set_tooltip_count; auto &autocomplete_row = autocomplete_rows[index]; const static auto insert_documentation = [](Source::LanguageProtocolView *view, Tooltip &tooltip, const std::string &detail, const LanguageProtocol::Documentation &documentation) { @@ -1969,8 +1972,8 @@ void Source::LanguageProtocolView::setup_autocomplete() { // This seems to be a bug in Gtk::TreeView::signal_cursor_changed. resolve_completion_item_connection.disconnect(); resolve_completion_item_connection = Glib::signal_timeout().connect( - [this, last_count, item_object = autocomplete_row.item_object] { - if(last_count != set_tooltip_count) + [this, count, item_object = autocomplete_row.item_object] { + if(count != set_tooltip_count) return false; std::stringstream ss; bool first = true; @@ -1978,16 +1981,16 @@ void Source::LanguageProtocolView::setup_autocomplete() { ss << (!first ? ",\"" : "\"") << JSON::escape_string(child.first) << "\":" << child.second; first = false; } - write_request("completionItem/resolve", ss.str(), [this, last_count](JSON &&result, bool error) { + write_request("completionItem/resolve", ss.str(), [this, count](JSON &&result, bool error) { if(!error) { - if(last_count != set_tooltip_count) + if(count != set_tooltip_count) return; auto detail = result.string_or("detail", ""); auto documentation = LanguageProtocol::Documentation(result.child_optional("documentation")); if(detail.empty() && documentation.value.empty()) return; - dispatcher.post([this, last_count, detail = std::move(detail), documentation = std::move(documentation)] { - if(last_count != set_tooltip_count) + dispatcher.post([this, count, detail = std::move(detail), documentation = std::move(documentation)] { + if(count != set_tooltip_count) return; autocomplete->tooltips.clear(); auto iter = CompletionDialog::get()->start_mark->get_iter(); @@ -2019,10 +2022,10 @@ void Source::LanguageProtocolView::setup_autocomplete() { } void Source::LanguageProtocolView::update_diagnostics_async(std::vector> &&diagnostics) { - size_t last_count = ++update_diagnostics_async_count; + size_t count = ++update_diagnostics_async_count; if(capabilities.code_action && !diagnostics.empty()) { - dispatcher.post([this, diagnostics = std::move(diagnostics), last_count]() mutable { - if(last_count != update_diagnostics_async_count) + dispatcher.post([this, diagnostics = std::move(diagnostics), count]() mutable { + if(count != update_diagnostics_async_count) return; std::stringstream ss; bool first = true; @@ -2041,12 +2044,12 @@ void Source::LanguageProtocolView::update_diagnostics_async(std::vector> params = {range, {"context", '{' + to_string({{"diagnostics", '[' + ss.str() + ']'}, {"only", "[\"quickfix\"]"}}) + '}'}}; - thread_pool.push([this, diagnostics = std::move(diagnostics), params = std::move(params), last_count]() mutable { - if(last_count != update_diagnostics_async_count) + worker.post([this, diagnostics = std::move(diagnostics), params = std::move(params), count]() mutable { + if(count != update_diagnostics_async_count) return; std::promise result_processed; - write_request("textDocument/codeAction", to_string(params), [this, &result_processed, &diagnostics, last_count](JSON &&result, bool error) { - if(!error && last_count == update_diagnostics_async_count) { + write_request("textDocument/codeAction", to_string(params), [this, &result_processed, &diagnostics, count](JSON &&result, bool error) { + if(!error && count == update_diagnostics_async_count) { try { for(auto &code_action : result.array_or_empty()) { auto kind = code_action.string_or("kind", ""); @@ -2108,8 +2111,8 @@ void Source::LanguageProtocolView::update_diagnostics_async(std::vector #include #include @@ -259,7 +260,7 @@ namespace Source { std::thread initialize_thread; Dispatcher dispatcher; - Glib::ThreadPool thread_pool; + Workers worker; void setup_navigation_and_refactoring(); void setup_signals(); diff --git a/src/workers.cpp b/src/workers.cpp new file mode 100644 index 0000000..06f2d66 --- /dev/null +++ b/src/workers.cpp @@ -0,0 +1,79 @@ +#include "workers.hpp" + +Workers::Workers(size_t worker_count) : worker_count(worker_count) { + start(); +} + +Workers::~Workers() { + stop(); +} + +void Workers::start() { + std::unique_lock lock(start_stop_mutex); + for(size_t i = 0; i < worker_count; ++i) { + threads.emplace_back([this] { + while(true) { + std::function task; + { + std::unique_lock lock(mutex); + while(tasks.empty() && !stop_threads) + cv.wait(lock); + if(tasks.empty()) + return; + task = std::move(tasks.front()); + tasks.pop_front(); + } + task(); + + std::unique_lock lock(mutex); + if(stop_threads_on_completed_tasks && tasks.empty()) { + stop_threads = true; + lock.unlock(); + cv.notify_all(); + return; + } + } + }); + } +} + +void Workers::post(std::function &&task) { + { + std::unique_lock lock(mutex); + tasks.emplace_back(std::move(task)); + } + cv.notify_one(); +} + +void Workers::stop() { + std::unique_lock lock(start_stop_mutex); + + if(threads.empty()) + return; + + { + std::unique_lock lock(mutex); + if(tasks.empty()) { + stop_threads = true; + lock.unlock(); + cv.notify_all(); + } + else + stop_threads_on_completed_tasks = true; + } + + for(auto &thread : threads) + thread.join(); + threads.clear(); + + { + std::unique_lock lock(mutex); + stop_threads = false; + stop_threads_on_completed_tasks = false; + } +} + +void Workers::restart() { + stop(); + start(); +} diff --git a/src/workers.hpp b/src/workers.hpp new file mode 100644 index 0000000..a686900 --- /dev/null +++ b/src/workers.hpp @@ -0,0 +1,38 @@ +#pragma once +#include +#include +#include +#include +#include + +class Workers { + size_t worker_count; + std::vector threads; + + std::mutex start_stop_mutex, mutex; + + std::list> tasks; + std::condition_variable cv; + + bool stop_threads = false; + bool stop_threads_on_completed_tasks = false; + +public: + /// Calls start(). + Workers(size_t worker_count = 1); + + /// Calls stop(). + ~Workers(); + + /// Will be called by constructor. For use after stop() only. + void start(); + + /// Add task to be processed by worker(s). + void post(std::function &&task); + + /// Stop threads when tasks have been completed. Waits until all of the threads have completed. + void stop(); + + /// Calls stop() and start(). + void restart(); +};