Skip to content

Commit

Permalink
perf: Enhance language server response times
Browse files Browse the repository at this point in the history
  • Loading branch information
slavek-kucera authored Nov 29, 2022
1 parent 1fb6427 commit 3452dfd
Show file tree
Hide file tree
Showing 29 changed files with 270 additions and 104 deletions.
1 change: 1 addition & 0 deletions clients/vscode-hlasmplugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#### Fixed
- LSP requests on virtual files are evaluated in the correct workspace context
- Enhance language server response times

#### Changed
- Macro label is the preferred go to definition target unless the request is made from the label itself
Expand Down
61 changes: 30 additions & 31 deletions language_server/src/request_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

#include "request_manager.h"

#include <string_view>

using namespace hlasm_plugin::language_server;

request::request(json message, server* executing_server)
Expand All @@ -39,17 +41,18 @@ void request_manager::add_request(server* server, json message)
// add request to q
{
std::unique_lock<std::mutex> lock(q_mtx_);
bool is_parsing_required = false;
// get new file
auto file = get_request_file_(message, &is_parsing_required);
auto [file, is_parsing_required] = get_request_file_(message);
// if the new file is the same as the currently running one, cancel the old one
if (currently_running_file_ == file && currently_running_file_ != "" && is_parsing_required)
if (currently_running_file_ == file && currently_running_file_ != ""
&& (is_parsing_required == request_parsing_implication::parsing_required
|| is_parsing_required == request_parsing_implication::stop_parsing))
{
*cancel_ = true;
// mark redundant requests as non valid
for (auto& req : requests_)
{
if (req.valid && get_request_file_(req.message) == file)
if (req.valid && get_request_file_(req.message).first == file)
req.valid = false;
}
}
Expand Down Expand Up @@ -93,7 +96,7 @@ void request_manager::handle_request_(const std::atomic<bool>* end_loop)
auto to_run = std::move(requests_.front());
requests_.pop_front();
// remember file name that is about to be parsed
currently_running_file_ = get_request_file_(to_run.message);
currently_running_file_ = get_request_file_(to_run.message).first;
// if the request is valid, do not cancel
// if not, cancel the parsing right away, only the file manager should update the data
if (cancel_)
Expand Down Expand Up @@ -139,35 +142,31 @@ void request_manager::finish_server_requests(server* to_finish)
requests_.end());
}


std::string request_manager::get_request_file_(const json& r, bool* is_parsing_required) const
std::pair<std::string, request_manager::request_parsing_implication> request_manager::get_request_file_(
const json& r) const
{
constexpr const char* didOpen = "textDocument/didOpen";
constexpr const char* didChange = "textDocument/didChange";
constexpr auto parsing_implication = [](std::string_view method) {
if (method == "textDocument/didOpen" || method == "textDocument/didChange")
return request_parsing_implication::parsing_required;
if (method == "textDocument/didClose")
return request_parsing_implication::stop_parsing;
return request_parsing_implication::parsing_not_required;
};

auto found = r.find("method");
if (found == r.end())
return "";
return {};
auto method = found->get<std::string>();
if (method.substr(0, 12) == "textDocument")
{
if (is_parsing_required)
{
if (method == didOpen || method == didChange)
*is_parsing_required = true;
else
*is_parsing_required = false;
}
const auto params = r.find("params");
if (params == r.end())
return "";
const auto textDocument = params->find("textDocument");
if (textDocument == params->end())
return "";
const auto uri = textDocument->find("uri");
if (uri == textDocument->end())
return "";
return uri->get<std::string>();
}
return "";
if (!method.starts_with("textDocument"))
return {};
const auto params = r.find("params");
if (params == r.end())
return {};
const auto textDocument = params->find("textDocument");
if (textDocument == params->end())
return {};
const auto uri = textDocument->find("uri");
if (uri == textDocument->end())
return {};
return { uri->get<std::string>(), parsing_implication(method) };
}
11 changes: 10 additions & 1 deletion language_server/src/request_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <deque>
#include <mutex>
#include <thread>
#include <utility>

#include "server.h"

Expand Down Expand Up @@ -66,7 +67,15 @@ class request_manager
std::atomic<server*> currently_running_server_ = nullptr;

void handle_request_(const std::atomic<bool>* end_loop);
std::string get_request_file_(const json& r, bool* is_parsing_required = nullptr) const;

enum class request_parsing_implication
{
parsing_not_required,
parsing_required,
stop_parsing,
};

std::pair<std::string, request_parsing_implication> get_request_file_(const json& r) const;

std::deque<request> requests_;

Expand Down
4 changes: 2 additions & 2 deletions parser_library/src/workspace_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ class workspace_manager::impl final : public diagnosable_impl

void did_open_file(const utils::resource::resource_location& document_loc, version_t version, std::string text)
{
file_manager_.did_open_file(document_loc, version, std::move(text));
auto file_changed = file_manager_.did_open_file(document_loc, version, std::move(text));
if (cancel_ && *cancel_)
return;

workspaces::workspace& ws = ws_path_match(document_loc.get_uri());
auto metadata = ws.did_open_file(document_loc);
auto metadata = ws.did_open_file(document_loc, file_changed);
if (cancel_ && *cancel_)
return;

Expand Down
17 changes: 11 additions & 6 deletions parser_library/src/workspaces/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,23 @@
#ifndef HLASMPLUGIN_PARSERLIBRARY_FILE_H
#define HLASMPLUGIN_PARSERLIBRARY_FILE_H

#include <filesystem>
#include <istream>
#include <string>

#include "diagnosable.h"
#include "protocol.h"
#include "utils/resource_location.h"

namespace hlasm_plugin::parser_library::workspaces {

enum class open_file_result;
using file_location = utils::resource::resource_location;

enum class update_file_result
{
identical,
changed,
bad,
};

// Interface that represents both file opened in LSP
// as well as a file opened by parser library from the disk.
class file : public virtual diagnosable
Expand All @@ -37,15 +42,15 @@ class file : public virtual diagnosable
virtual const std::string& get_text() = 0;
// Returns whether file is bad - bad file cannot be loaded from disk.
// LSP files are never bad.
virtual bool update_and_get_bad() = 0;
virtual update_file_result update_and_get_bad() = 0;
// Returns whether file is open by LSP.
virtual bool get_lsp_editing() = 0;
virtual bool get_lsp_editing() const = 0;

// Gets LSP version of file.
virtual version_t get_version() = 0;

// LSP notifications
virtual void did_open(std::string new_text, version_t version) = 0;
virtual open_file_result did_open(std::string new_text, version_t version) = 0;
virtual void did_change(std::string new_text) = 0;
virtual void did_change(range range, std::string new_text) = 0;
virtual void did_close() = 0;
Expand Down
30 changes: 20 additions & 10 deletions parser_library/src/workspaces/file_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <locale>
#include <string>

#include "file_manager.h"
#include "utils/content_loader.h"
#include "utils/unicode_text.h"

Expand All @@ -44,15 +45,20 @@ const std::string& file_impl::get_text()
return text_;
}

void file_impl::load_text()
update_file_result file_impl::load_text()
{
if (auto loaded_text = utils::resource::load_text(file_location_); loaded_text.has_value())
{
text_ = std::move(loaded_text.value());
text_ = utils::replace_non_utf8_chars(text_);
bool was_up_to_date = up_to_date_;
bool identical = text_ == loaded_text.value();
if (!identical)
{
text_ = std::move(loaded_text.value());
text_ = utils::replace_non_utf8_chars(text_);
}
up_to_date_ = true;
bad_ = false;
return;
return identical && was_up_to_date ? update_file_result::identical : update_file_result::changed;
}

text_ = "";
Expand All @@ -62,6 +68,8 @@ void file_impl::load_text()
// TODO Figure out how to present this error in VSCode.
// Also think about the lifetime of the error as it seems that it will stay in Problem panel forever
// add_diagnostic(diagnostic_s::error_W0001(file_location_.to_presentable()));

return update_file_result::bad;
}

// adds positions of newlines into vector 'lines'
Expand Down Expand Up @@ -102,8 +110,9 @@ size_t find_newlines(const std::string& text, std::vector<size_t>& lines)
return lines.size() - before;
}

void file_impl::did_open(std::string new_text, version_t version)
open_file_result file_impl::did_open(std::string new_text, version_t version)
{
bool identical = text_ == new_text;
text_ = std::move(new_text);
version_ = version;

Expand All @@ -114,9 +123,11 @@ void file_impl::did_open(std::string new_text, version_t version)
up_to_date_ = true;
bad_ = false;
editing_ = true;

return identical ? open_file_result::changed_lsp : open_file_result::changed_content;
}

bool file_impl::get_lsp_editing() { return editing_; }
bool file_impl::get_lsp_editing() const { return editing_; }


// applies a change to the text and updates line beginnings
Expand Down Expand Up @@ -189,14 +200,13 @@ const std::string& file_impl::get_text_ref() { return text_; }

version_t file_impl::get_version() { return version_; }

bool file_impl::update_and_get_bad()
update_file_result file_impl::update_and_get_bad()
{
// If user is editing file through LSP, do not load from disk.
if (editing_)
return false;
return update_file_result::identical;

load_text();
return bad_;
return load_text();
}

size_t file_impl::index_from_position(const std::string& text, const std::vector<size_t>& line_indices, position loc)
Expand Down
8 changes: 4 additions & 4 deletions parser_library/src/workspaces/file_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ class file_impl : public virtual file, public virtual diagnosable_impl
const file_location& get_location() override;
const std::string& get_text() override;
version_t get_version() override;
bool update_and_get_bad() override;
bool get_lsp_editing() override;
update_file_result update_and_get_bad() override;
bool get_lsp_editing() const override;

void did_open(std::string new_text, version_t version) override;
open_file_result did_open(std::string new_text, version_t version) override;
void did_change(std::string new_text) override;
void did_change(range range, std::string new_text) override;
void did_close() override;
Expand Down Expand Up @@ -73,7 +73,7 @@ class file_impl : public virtual file, public virtual diagnosable_impl

version_t version_ = 0;

void load_text();
update_file_result load_text();
};

#pragma warning(pop)
Expand Down
10 changes: 9 additions & 1 deletion parser_library/src/workspaces/file_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ using file_ptr = std::shared_ptr<file>;
using processor_file_ptr = std::shared_ptr<processor_file>;
using list_directory_result =
std::pair<std::vector<std::pair<std::string, utils::resource::resource_location>>, utils::path::list_directory_rc>;
enum class open_file_result
{
identical,
changed_lsp,
changed_content,
};

// Wraps an associative array of file names and files.
// Implements LSP text synchronization methods.
Expand Down Expand Up @@ -73,7 +79,7 @@ class file_manager : public virtual diagnosable

virtual bool dir_exists(const utils::resource::resource_location& dir_loc) const = 0;

virtual void did_open_file(const file_location& document_loc, version_t version, std::string text) = 0;
virtual open_file_result did_open_file(const file_location& document_loc, version_t version, std::string text) = 0;
virtual void did_change_file(
const file_location& document_loc, version_t version, const document_change* changes, size_t ch_size) = 0;
virtual void did_close_file(const file_location& document_loc) = 0;
Expand All @@ -84,6 +90,8 @@ class file_manager : public virtual diagnosable
virtual std::string get_virtual_file(unsigned long long id) const = 0;
virtual utils::resource::resource_location get_virtual_file_workspace(unsigned long long id) const = 0;

virtual open_file_result update_file(const file_location& document_loc) = 0;

protected:
~file_manager() = default;
};
Expand Down
21 changes: 18 additions & 3 deletions parser_library/src/workspaces/file_manager_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,14 @@ void file_manager_impl::prepare_file_for_change_(std::shared_ptr<file_impl>& fil
file = std::make_shared<file_impl>(*file);
}

void file_manager_impl::did_open_file(const file_location& document_loc, version_t version, std::string text)
open_file_result file_manager_impl::did_open_file(
const file_location& document_loc, version_t version, std::string text)
{
std::lock_guard guard(files_mutex);
auto [ret, _] = files_.try_emplace(document_loc, std::make_shared<file_impl>(document_loc));
auto [ret, inserted] = files_.try_emplace(document_loc, std::make_shared<file_impl>(document_loc));
prepare_file_for_change_(ret->second);
ret->second->did_open(std::move(text), version);
auto changed = ret->second->did_open(std::move(text), version);
return inserted ? open_file_result::changed_content : changed;
}

void file_manager_impl::did_change_file(
Expand Down Expand Up @@ -218,4 +220,17 @@ utils::resource::resource_location file_manager_impl::get_virtual_file_workspace
return utils::resource::resource_location();
}

open_file_result file_manager_impl::update_file(const file_location& document_loc)
{
std::lock_guard guard(files_mutex);
auto f = files_.find(document_loc);
if (f == files_.end())
return open_file_result::identical;

if (f->second->update_and_get_bad() == update_file_result::identical)
return open_file_result::identical;
else
return open_file_result::changed_content;
}

} // namespace hlasm_plugin::parser_library::workspaces
4 changes: 3 additions & 1 deletion parser_library/src/workspaces/file_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class file_manager_impl : public file_manager, public diagnosable_impl

std::string canonical(const utils::resource::resource_location& res_loc, std::error_code& ec) const override;

void did_open_file(const file_location& document_loc, version_t version, std::string text) override;
open_file_result did_open_file(const file_location& document_loc, version_t version, std::string text) override;
void did_change_file(
const file_location& document_loc, version_t version, const document_change* changes, size_t ch_size) override;
void did_close_file(const file_location& document_loc) override;
Expand All @@ -74,6 +74,8 @@ class file_manager_impl : public file_manager, public diagnosable_impl
std::string get_virtual_file(unsigned long long id) const override;
utils::resource::resource_location get_virtual_file_workspace(unsigned long long id) const override;

open_file_result update_file(const file_location& document_loc) override;

private:
struct virtual_file_entry
{
Expand Down
1 change: 1 addition & 0 deletions parser_library/src/workspaces/library.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class library : public virtual diagnosable
virtual std::shared_ptr<processor> find_file(const std::string& file) = 0;
virtual void refresh() = 0;
virtual std::vector<std::string> list_files() = 0;
virtual std::string refresh_url_prefix() const = 0;
};

} // namespace hlasm_plugin::parser_library::workspaces
Expand Down
Loading

0 comments on commit 3452dfd

Please sign in to comment.