Skip to content

Commit

Permalink
fix: Closing a dependency without saving does not trigger reparsing
Browse files Browse the repository at this point in the history
  • Loading branch information
slavek-kucera authored Mar 23, 2023
1 parent 166ecd0 commit 8b576fa
Show file tree
Hide file tree
Showing 32 changed files with 461 additions and 290 deletions.
7 changes: 4 additions & 3 deletions benchmark/benchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ json parse_one_file(const std::string& source_file,
+ metrics.lookahead_statements + metrics.reparsed_statements;
s.average_stmt_ms += (exec_statements / (double)time);
s.average_line_ms += metrics.lines / (double)time;
s.all_files += metrics.files;
s.all_files += collector.data.ws_info.files_processed;
s.whole_time += time;

auto top_messages = benchmark::get_top_messages(diag_counter.message_counts);
Expand All @@ -165,9 +165,10 @@ json parse_one_file(const std::string& source_file,
{ "Continued Statements", metrics.continued_statements },
{ "Non-continued Statements", metrics.non_continued_statements },
{ "Lines", metrics.lines },
{ "Files", metrics.files },
{ "Files", collector.data.ws_info.files_processed },
});

auto first_ws_info = collector.data.ws_info;
auto first_parse_metrics = metrics;
auto first_diag_counter = diag_counter;
long long reparse_time = 0;
Expand Down Expand Up @@ -226,7 +227,7 @@ json parse_one_file(const std::string& source_file,
<< "Lines: " << first_parse_metrics.lines << '\n'
<< "Executed Statement/ms: " << exec_statements / (double)time << '\n'
<< "Line/ms: " << first_parse_metrics.lines / (double)time << '\n'
<< "Files: " << first_parse_metrics.files << '\n'
<< "Files: " << first_ws_info.files_processed << '\n'
<< "Top messages: " << top_messages.dump() << '\n'
<< '\n'
<< std::endl;
Expand Down
1 change: 1 addition & 0 deletions clients/vscode-hlasmplugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- New or changed files are parsed only when they are opened in the editor
- High CPU usage while going to the symbol definition
- Inconsistent identification of inactive statements
- Closing a dependency without saving does not trigger reparsing

## [1.7.0](https://github.com/eclipse/che-che4z-lsp-for-hlasm/compare/1.6.0...1.7.0) (2023-03-08)

Expand Down
12 changes: 8 additions & 4 deletions language_server/src/parsing_metadata_serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@ namespace hlasm_plugin::parser_library {

void to_json(nlohmann::json& j, const parser_library::workspace_file_info& info)
{
j = nlohmann::json { { "config_parsing", info.config_parsing },
j = nlohmann::json {
{ "config_parsing", info.config_parsing },
{ "diagnostics_suppressed", info.diagnostics_suppressed },
{ "processor_group_found", info.processor_group_found } };
{ "processor_group_found", info.processor_group_found },
{ "files_processed", info.files_processed },
};
}

void to_json(nlohmann::json& j, const parser_library::performance_metrics& metrics)
{
j = nlohmann::json { { "Open Code Statements", metrics.open_code_statements },
j = nlohmann::json {
{ "Open Code Statements", metrics.open_code_statements },
{ "Copy Statements", metrics.copy_statements },
{ "Macro Statements", metrics.macro_statements },
{ "Copy Def Statements", metrics.copy_def_statements },
Expand All @@ -37,7 +41,7 @@ void to_json(nlohmann::json& j, const parser_library::performance_metrics& metri
{ "Continued Statements", metrics.continued_statements },
{ "Non-continued Statements", metrics.non_continued_statements },
{ "Lines", metrics.lines },
{ "Files", metrics.files } };
};
}

void to_json(nlohmann::json& j, const parser_library::parsing_metadata& metadata)
Expand Down
2 changes: 1 addition & 1 deletion language_server/test/telemetry_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,12 @@ TEST(telemetry, lsp_server_did_open)

nlohmann::json& metrics = telemetry_reply["params"]["measurements"];

EXPECT_EQ(metrics["Files"], 1);
EXPECT_GT(metrics["duration"], 0U);
EXPECT_EQ(metrics["error_count"], 1);

nlohmann::json& ws_info = telemetry_reply["params"]["properties"];

EXPECT_EQ(ws_info["files_processed"], 1);
EXPECT_EQ(ws_info["diagnostics_suppressed"], false);
}

Expand Down
4 changes: 2 additions & 2 deletions parser_library/fuzzer/fuzzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class fuzzer_lib_provider : public parse_lib_provider
callback(true);
}

bool has_library(std::string_view library, resource_location* url) const override
bool has_library(std::string_view library, resource_location* url) override
{
auto lib = read_library_name(library);
if (!lib.has_value())
Expand All @@ -81,7 +81,7 @@ class fuzzer_lib_provider : public parse_lib_provider
}

void get_library(std::string_view library,
std::function<void(std::optional<std::pair<std::string, resource_location>>)> callback) const override
std::function<void(std::optional<std::pair<std::string, resource_location>>)> callback) override
{
assert(callback);
auto lib = read_library_name(library);
Expand Down
14 changes: 3 additions & 11 deletions parser_library/include/protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,13 @@ struct PARSER_LIBRARY_EXPORT performance_metrics
size_t lookahead_statements = 0;
size_t continued_statements = 0;
size_t non_continued_statements = 0;
size_t files = 0;

bool operator==(const performance_metrics&) const noexcept = default;
};

struct PARSER_LIBRARY_EXPORT workspace_file_info
{
size_t files_processed = 0;
bool config_parsing = false;
bool diagnostics_suppressed = false;
bool processor_group_found = false;
Expand All @@ -294,16 +296,6 @@ struct PARSER_LIBRARY_EXPORT parsing_metadata
workspace_file_info ws_info;
};

bool inline operator==(const performance_metrics& lhs, const performance_metrics& rhs)
{
return lhs.lines == rhs.lines && lhs.macro_def_statements == rhs.macro_def_statements
&& lhs.macro_statements == rhs.macro_statements && lhs.open_code_statements == rhs.open_code_statements
&& lhs.copy_def_statements == rhs.copy_def_statements && lhs.copy_statements == rhs.copy_statements
&& lhs.reparsed_statements == rhs.reparsed_statements && lhs.lookahead_statements == rhs.lookahead_statements
&& lhs.continued_statements == rhs.continued_statements
&& lhs.non_continued_statements == rhs.non_continued_statements && lhs.files == rhs.files;
}

struct PARSER_LIBRARY_EXPORT diagnostic_list
{
diagnostic_list();
Expand Down
6 changes: 1 addition & 5 deletions parser_library/src/analyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,7 @@ void analyzer::collect_diags() const
collect_diags_from_child(field_parser_);
}

const performance_metrics& analyzer::get_metrics() const
{
ctx_.hlasm_ctx->fill_metrics_files();
return ctx_.hlasm_ctx->metrics;
}
const performance_metrics& analyzer::get_metrics() const { return ctx_.hlasm_ctx->metrics; }

void analyzer::register_stmt_analyzer(statement_analyzer* stmt_analyzer)
{
Expand Down
4 changes: 2 additions & 2 deletions parser_library/src/analyzer.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ class analyzer : public diagnosable_ctx

processing::statement_fields_parser field_parser_;

std::vector<virtual_file_handle> vf_handles_;
std::vector<std::pair<virtual_file_handle, utils::resource::resource_location>> vf_handles_;

processing::processing_manager mngr_;

public:
analyzer(std::string_view text, analyzer_options opts = {});
std::vector<virtual_file_handle> take_vf_handles() { return std::move(vf_handles_); }
auto take_vf_handles() { return std::move(vf_handles_); }
analyzing_context context() const;

context::hlasm_context& hlasm_ctx();
Expand Down
19 changes: 1 addition & 18 deletions parser_library/src/context/hlasm_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ hlasm_context::hlasm_context(
init_instruction_map(opcode_mnemo_, *ids_, asm_options_.instr_set);

add_global_system_vars(scope_stack_.emplace_back());
visited_files_.insert(file_loc);
push_statement_processing(processing::processing_kind::ORDINARY, std::move(file_loc));
}

Expand Down Expand Up @@ -642,8 +641,6 @@ std::vector<id_index> hlasm_context::whole_copy_stack() const
return ret;
}

void hlasm_context::fill_metrics_files() { metrics.files = visited_files_.size(); }

const hlasm_context::global_variable_storage& hlasm_context::globals() const { return globals_; }

var_sym_ptr hlasm_context::get_var_sym(id_index name) const
Expand Down Expand Up @@ -894,8 +891,6 @@ macro_invo_ptr hlasm_context::enter_macro(id_index name, macro_data_ptr label_pa
add_system_vars_to_scope(new_scope);
add_global_system_vars(new_scope);

visited_files_.insert(macro_def->definition_location.resource_loc);

++SYSNDX_;
mnote_last_max = 0;

Expand Down Expand Up @@ -927,24 +922,17 @@ const location* hlasm_context::current_macro_definition_location() const

const utils::resource::resource_location& hlasm_context::opencode_location() const { return opencode_file_location_; }

const std::set<utils::resource::resource_location>& hlasm_context::get_visited_files() const { return visited_files_; }

copy_member_ptr hlasm_context::add_copy_member(
id_index member, statement_block definition, location definition_location)
{
auto& copydef = copy_members_[member];
if (!copydef)
copydef = std::make_shared<copy_member>(member, std::move(definition), definition_location);
visited_files_.insert(std::move(definition_location.resource_loc));

return copydef;
}

void hlasm_context::add_copy_member(copy_member_ptr member)
{
visited_files_.insert(member->definition_location.resource_loc);
copy_members_[member->name] = std::move(member);
}
void hlasm_context::add_copy_member(copy_member_ptr member) { copy_members_[member->name] = std::move(member); }


copy_member_ptr hlasm_context::get_copy_member(id_index member) const
Expand All @@ -968,11 +956,6 @@ const hlasm_context::copy_member_storage& hlasm_context::copy_members() { return

void hlasm_context::leave_copy_member() { source_stack_.back().copy_stack.pop_back(); }

void hlasm_context::add_preprocessor_dependency(const utils::resource::resource_location& file_loc)
{
visited_files_.emplace(file_loc);
}

void hlasm_context::apply_source_snapshot(source_snapshot snapshot)
{
assert(std::transform_reduce(source_stack_.begin(),
Expand Down
8 changes: 0 additions & 8 deletions parser_library/src/context/hlasm_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ class hlasm_context

// opencode file location
utils::resource::resource_location opencode_file_location_;
// all files processes via macro or copy member invocation
std::set<utils::resource::resource_location> visited_files_;

// Compiler options
asm_option asm_options_;
Expand Down Expand Up @@ -131,8 +129,6 @@ class hlasm_context

// gets opencode file location
const utils::resource::resource_location& opencode_location() const;
// accesses visited files
const std::set<utils::resource::resource_location>& get_visited_files() const;

// gets current source
const source_context& current_source() const;
Expand Down Expand Up @@ -187,7 +183,6 @@ class hlasm_context
// performance metrics
performance_metrics metrics;

void fill_metrics_files();
// return map of global set vars
const global_variable_storage& globals() const;

Expand Down Expand Up @@ -259,9 +254,6 @@ class hlasm_context
// leaves current copy member
void leave_copy_member();

// register preprocessor dependency
void add_preprocessor_dependency(const utils::resource::resource_location& file_loc);

// creates specified global set symbol
template<typename T>
set_sym_ptr create_global_variable(id_index id, bool is_scalar)
Expand Down
4 changes: 2 additions & 2 deletions parser_library/src/debugging/debug_lib_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ void debug_lib_provider::parse_library(
callback(false);
}

bool debug_lib_provider::has_library(std::string_view library, utils::resource::resource_location* loc) const
bool debug_lib_provider::has_library(std::string_view library, utils::resource::resource_location* loc)
{
for (const auto& lib : m_libraries)
if (lib->has_file(library, loc))
Expand All @@ -75,7 +75,7 @@ bool debug_lib_provider::has_library(std::string_view library, utils::resource::
}

void debug_lib_provider::get_library(std::string_view library,
std::function<void(std::optional<std::pair<std::string, utils::resource::resource_location>>)> callback) const
std::function<void(std::optional<std::pair<std::string, utils::resource::resource_location>>)> callback)
{
assert(callback);
utils::resource::resource_location url;
Expand Down
4 changes: 2 additions & 2 deletions parser_library/src/debugging/debug_lib_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ class debug_lib_provider final : public workspaces::parse_lib_provider
workspaces::library_data data,
std::function<void(bool)> callback) override;

bool has_library(std::string_view library, utils::resource::resource_location* loc) const override;
bool has_library(std::string_view library, utils::resource::resource_location* loc) override;

void get_library(std::string_view library,
std::function<void(std::optional<std::pair<std::string, utils::resource::resource_location>>)> callback)
const override;
override;
};

} // namespace hlasm_plugin::parser_library::debugging
Expand Down
16 changes: 9 additions & 7 deletions parser_library/src/processing/opencode_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ opencode_provider::opencode_provider(std::string_view text,
std::unique_ptr<preprocessor> prep,
opencode_provider_options opts,
virtual_file_monitor* virtual_file_monitor,
std::vector<virtual_file_handle>& vf_handles)
std::vector<std::pair<virtual_file_handle, utils::resource::resource_location>>& vf_handles)
: statement_provider(statement_provider_kind::OPEN)
, m_input_document(text)
, m_singleline { parsing::parser_holder::create(&src_proc, ctx.hlasm_ctx.get(), &diag_consumer, false),
Expand Down Expand Up @@ -522,11 +522,12 @@ utils::task opencode_provider::run_preprocessor()
}
else
{
auto file_handle = m_virtual_file_monitor->file_generated(new_file->second);
auto file_location = generate_virtual_file_name(file_handle.file_id(), virtual_file_name.to_string_view());
m_vf_handles.emplace_back(std::move(file_handle), file_location);
return start_nested_parser(new_file->second,
analyzer_options {
generate_virtual_file_name(
m_vf_handles.emplace_back(m_virtual_file_monitor->file_generated(new_file->second)).file_id(),
virtual_file_name.to_string_view()),
std::move(file_location),
m_lib_provider,
*m_ctx,
workspaces::library_data { processing_kind::COPY, virtual_file_name },
Expand Down Expand Up @@ -597,11 +598,12 @@ utils::task opencode_provider::convert_ainsert_buffer_to_copybook()

auto new_file = m_virtual_files.try_emplace(virtual_copy_name, std::move(result)).first;

auto file_handle = m_virtual_file_monitor->file_generated(new_file->second);
auto file_location = generate_virtual_file_name(file_handle.file_id(), virtual_copy_name.to_string_view());
m_vf_handles.emplace_back(std::move(file_handle), file_location);
co_await start_nested_parser(new_file->second,
analyzer_options {
generate_virtual_file_name(
m_vf_handles.emplace_back(m_virtual_file_monitor->file_generated(new_file->second)).file_id(),
virtual_copy_name.to_string_view()),
std::move(file_location),
m_lib_provider,
*m_ctx,
workspaces::library_data { processing_kind::COPY, virtual_copy_name },
Expand Down
5 changes: 3 additions & 2 deletions parser_library/src/processing/opencode_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <string>
#include <string_view>
#include <unordered_map>
#include <utility>
#include <variant>
#include <vector>

Expand Down Expand Up @@ -134,7 +135,7 @@ class opencode_provider final : public statement_provider
std::unique_ptr<preprocessor> m_preprocessor;

virtual_file_monitor* m_virtual_file_monitor;
std::vector<virtual_file_handle>& m_vf_handles;
std::vector<std::pair<virtual_file_handle, utils::resource::resource_location>>& m_vf_handles;

public:
// rewinds position in file
Expand All @@ -152,7 +153,7 @@ class opencode_provider final : public statement_provider
std::unique_ptr<preprocessor> preprocessor,
opencode_provider_options opts,
virtual_file_monitor* virtual_file_monitor,
std::vector<virtual_file_handle>& vf_handles);
std::vector<std::pair<virtual_file_handle, utils::resource::resource_location>>& vf_handles);

parsing::hlasmparser_multiline& parser(); // for testing only

Expand Down
2 changes: 0 additions & 2 deletions parser_library/src/processing/processing_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,6 @@ void processing_manager::finish_preprocessor()

static const context::statement_block stmt_block;

ctx_.hlasm_ctx->add_preprocessor_dependency(inc_member_details->loc);

ctx_.lsp_ctx->add_copy(std::make_shared<context::copy_member>(hlasm_ctx_.ids().add(inc_member_details->name),
stmt_block,
location(position(0, 0), inc_member_details->loc)),
Expand Down
7 changes: 2 additions & 5 deletions parser_library/src/workspaces/parse_lib_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,9 @@ void empty_parse_lib_provider::parse_library(
assert(callback);
callback(false);
};
bool empty_parse_lib_provider::has_library(std::string_view, utils::resource::resource_location*) const
{
return false;
};
bool empty_parse_lib_provider::has_library(std::string_view, utils::resource::resource_location*) { return false; };
void empty_parse_lib_provider::get_library(std::string_view,
std::function<void(std::optional<std::pair<std::string, utils::resource::resource_location>>)> callback) const
std::function<void(std::optional<std::pair<std::string, utils::resource::resource_location>>)> callback)
{
assert(callback);
callback(std::nullopt);
Expand Down
Loading

0 comments on commit 8b576fa

Please sign in to comment.