From 0576ad133921a6fea4e6fba4bb86f1d0a7941b20 Mon Sep 17 00:00:00 2001 From: Kai Hermann Date: Thu, 23 Feb 2023 13:02:14 +0100 Subject: [PATCH] Add a few simple performance enhancements (#72) * Default initialize validate_schema with false * Only validate var subscription if validate_data_with_schema is set * Add some caching of module info * Make some data accesses a bit more efficient * Pass topic to check_topic_matches as const ref Signed-off-by: Kai-Uwe Hermann --------- Signed-off-by: Kai-Uwe Hermann --- include/utils/config.hpp | 8 ++++ include/utils/config_cache.hpp | 22 +++++++++ include/utils/mqtt_abstraction_impl.hpp | 2 +- lib/config.cpp | 27 +++++++++-- lib/everest.cpp | 63 +++++++++++++------------ lib/mqtt_abstraction_impl.cpp | 11 +++-- lib/runtime.cpp | 1 + 7 files changed, 95 insertions(+), 39 deletions(-) create mode 100644 include/utils/config_cache.hpp diff --git a/include/utils/config.hpp b/include/utils/config.hpp index 84e26080..82c26650 100644 --- a/include/utils/config.hpp +++ b/include/utils/config.hpp @@ -11,6 +11,7 @@ #include +#include #include namespace Everest { @@ -85,7 +86,14 @@ class Config { json extract_implementation_info(const std::string& module_id, const std::string& impl_id); void resolve_all_requirements(); + // experimental caches + std::unordered_map module_names; + std::unordered_map module_config_cache; + public: + std::string get_module_name(const std::string& module_id); + bool module_provides(const std::string& module_name, const std::string& impl_id); + json get_module_cmds(const std::string& module_name, const std::string& impl_id); /// /// \brief creates a new Config object, looking for the config.json and schemes folder relative to the provided \p /// main_dir diff --git a/include/utils/config_cache.hpp b/include/utils/config_cache.hpp new file mode 100644 index 00000000..26a59d12 --- /dev/null +++ b/include/utils/config_cache.hpp @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2020 - 2023 Pionix GmbH and Contributors to EVerest +#ifndef UTILS_CONFIG_CACHE_HPP +#define UTILS_CONFIG_CACHE_HPP + +#include +#include +#include + +#include + +namespace Everest { +using json = nlohmann::json; + +struct ConfigCache { + std::set provides_impl; + std::unordered_map cmds; +}; + +} // namespace Everest + +#endif // UTILS_CONFIG_CACHE_HPP diff --git a/include/utils/mqtt_abstraction_impl.hpp b/include/utils/mqtt_abstraction_impl.hpp index 9fc007b8..08ccf6b7 100644 --- a/include/utils/mqtt_abstraction_impl.hpp +++ b/include/utils/mqtt_abstraction_impl.hpp @@ -122,7 +122,7 @@ class MQTTAbstractionImpl { /// wildcards /// /// \returns true if the topic matches, false otherwise - static bool check_topic_matches(std::string full_topic, std::string wildcard_topic); + static bool check_topic_matches(const std::string& full_topic, const std::string& wildcard_topic); /// /// \brief callback that is called from the mqtt implementation whenever a message is received diff --git a/lib/config.cpp b/lib/config.cpp index d43ed25d..4d0be77d 100644 --- a/lib/config.cpp +++ b/lib/config.cpp @@ -187,6 +187,8 @@ Config::Config(std::string schemas_dir, std::string config_file, std::string mod auto& module_config = element.value(); std::string module_name = module_config["module"]; + this->module_config_cache[module_id] = ConfigCache(); + this->module_names[module_id] = module_name; EVLOG_debug << fmt::format("Found module {}, loading and verifying manifest...", printable_identifier(module_id)); @@ -232,12 +234,15 @@ Config::Config(std::string schemas_dir, std::string config_file, std::string mod std::set provided_impls = Config::keys(this->manifests[module_name]["provides"]); this->interfaces[module_name] = json({}); + this->module_config_cache[module_name].provides_impl = provided_impls; for (const auto& impl_id : provided_impls) { EVLOG_debug << fmt::format("Loading interface for implementation: {}", impl_id); auto intf_name = this->manifests[module_name]["provides"][impl_id]["interface"].get(); auto seen_interfaces = std::set(); this->interfaces[module_name][impl_id] = resolve_interface(intf_name); + this->module_config_cache[module_name].cmds[impl_id] = + this->interfaces.at(module_name).at(impl_id).at("cmds"); } // check if config only contains impl_ids listed in manifest file @@ -325,6 +330,19 @@ Config::Config(std::string schemas_dir, std::string config_file, std::string mod resolve_all_requirements(); } +std::string Config::get_module_name(const std::string& module_id) { + return this->module_names.at(module_id); +} + +bool Config::module_provides(const std::string& module_name, const std::string& impl_id) { + auto& provides = this->module_config_cache.at(module_name).provides_impl; + return (provides.find(impl_id) != provides.end()); +} + +json Config::get_module_cmds(const std::string& module_name, const std::string& impl_id) { + return this->module_config_cache.at(module_name).cmds.at(impl_id); +} + json Config::resolve_interface(const std::string& intf_name) { // load and validate interface.json and mark interface as seen auto intf_definition = load_interface_file(intf_name); @@ -378,14 +396,15 @@ json Config::resolve_requirement(const std::string& module_id, const std::string // isn't even listed in the module manifest // FIXME (aw): the following if doesn't check for the requirement id // at all - if (!this->main.contains(module_id)) { + auto module_name_it = this->module_names.find(module_id); + if (module_name_it == this->module_names.end()) { EVLOG_AND_THROW(EverestApiError(fmt::format("Requested requirement id '{}' of module {} not found in config!", requirement_id, printable_identifier(module_id)))); } // check for connections for this requirement - json module_config = this->main[module_id]; - std::string module_name = module_config["module"].get(); + auto& module_config = this->main[module_id]; + std::string module_name = module_name_it->second; auto& requirement = this->manifests[module_name]["requires"][requirement_id]; if (!module_config["connections"].contains(requirement_id)) { return json::array(); // return an empty array if our config does not contain any connections for this @@ -687,7 +706,7 @@ json Config::extract_implementation_info(const std::string& module_id, const std json info; info["module_id"] = module_id; - info["module_name"] = this->main[module_id]["module"]; + info["module_name"] = get_module_name(module_id); info["impl_id"] = impl_id; info["impl_intf"] = ""; if (!impl_id.empty()) { diff --git a/lib/everest.cpp b/lib/everest.cpp index 286cef01..aa3a0cbb 100644 --- a/lib/everest.cpp +++ b/lib/everest.cpp @@ -220,20 +220,21 @@ json Everest::call_cmd(const Requirement& req, const std::string& cmd_name, json std::future res_future = res_promise.get_future(); Handler res_handler = [this, &res_promise, call_id, connection, cmd_name, return_type](json data) { - if (data["id"] != call_id) { - EVLOG_debug << fmt::format("RES: data_id != call_id ({} != {})", data["id"], call_id); + auto& data_id = data.at("id"); + if (data_id != call_id) { + EVLOG_debug << fmt::format("RES: data_id != call_id ({} != {})", data_id, call_id); return; } EVLOG_debug << fmt::format( - "Incoming res {} for {}->{}()", data["id"], + "Incoming res {} for {}->{}()", data_id, this->config.printable_identifier(connection["module_id"], connection["implementation_id"]), cmd_name); // make sure to only return the intended parts of the incoming result to not open up the api to internals res_promise.set_value(json::object({{"retval", data["retval"]}, {"return_type", return_type}, {"origin", data["origin"]}, - {"id", data["id"]}})); + {"id", data_id}})); }; const auto cmd_topic = @@ -338,7 +339,7 @@ void Everest::subscribe_var(const Requirement& req, const std::string& var_name, } auto requirement_module_id = connection["module_id"].get(); - auto module_name = this->config.get_main_config()[requirement_module_id]["module"].get(); + auto module_name = this->config.get_module_name(requirement_module_id); auto requirement_impl_id = connection["implementation_id"].get(); auto requirement_impl_manifest = this->config.get_interfaces()[module_name][requirement_impl_id]; @@ -355,17 +356,19 @@ void Everest::subscribe_var(const Requirement& req, const std::string& var_name, EVLOG_debug << fmt::format( "Incoming {}->{}", this->config.printable_identifier(requirement_module_id, requirement_impl_id), var_name); - // check data and ignore it if not matching (publishing it should have been prohibited already) - try { - json_validator validator( - [this](const json_uri& uri, json& schema) { this->config.ref_loader(uri, schema); }, - Config::format_checker); - validator.set_root_schema(requirement_manifest_vardef); - validator.validate(data); - } catch (const std::exception& e) { - EVLOG_warning << fmt::format("Ignoring incoming var '{}' because not matching manifest schema: {}", - var_name, e.what()); - return; + if (this->validate_data_with_schema) { + // check data and ignore it if not matching (publishing it should have been prohibited already) + try { + json_validator validator( + [this](const json_uri& uri, json& schema) { this->config.ref_loader(uri, schema); }, + Config::format_checker); + validator.set_root_schema(requirement_manifest_vardef); + validator.validate(data); + } catch (const std::exception& e) { + EVLOG_warning << fmt::format("Ignoring incoming var '{}' because not matching manifest schema: {}", + var_name, e.what()); + return; + } } callback(data); @@ -389,7 +392,7 @@ void Everest::subscribe_var(const Requirement& req, const std::string& var_name, } auto requirement_module_id = connection["module_id"].get(); - auto module_name = this->config.get_main_config()[requirement_module_id]["module"].get(); + auto module_name = this->config.get_module_name(requirement_module_id); auto requirement_impl_id = connection["implementation_id"].get(); auto requirement_impl_manifest = this->config.get_interfaces()[module_name][requirement_impl_id]; @@ -687,12 +690,10 @@ json Everest::get_cmd_definition(const std::string& module_id, const std::string bool is_call) { BOOST_LOG_FUNCTION(); - std::string module_name = this->config.get_main_config()[module_id]["module"].get(); - auto module_manifest = this->config.get_manifests()[module_name]; - auto module = this->config.get_interfaces()[module_name]; - auto impl_intf = module[impl_id]; + std::string module_name = this->config.get_module_name(module_id); + auto cmds = this->config.get_module_cmds(module_name, impl_id); - if (!module_manifest["provides"].contains(impl_id)) { + if (!this->config.module_provides(module_name, impl_id)) { if (!is_call) { EVLOG_AND_THROW(EverestApiError(fmt::format( "Module {} tries to provide implementation '{}' not declared in manifest!", module_name, impl_id))); @@ -703,7 +704,7 @@ json Everest::get_cmd_definition(const std::string& module_id, const std::string } } - if (!impl_intf["cmds"].contains(cmd_name)) { + if (!cmds.contains(cmd_name)) { if (!is_call) { EVLOG_AND_THROW( EverestApiError(fmt::format("{} tries to provide cmd '{}' not declared in manifest!", @@ -715,7 +716,7 @@ json Everest::get_cmd_definition(const std::string& module_id, const std::string } } - return impl_intf["cmds"][cmd_name]; + return cmds.at(cmd_name); } json Everest::get_cmd_definition(const std::string& module_id, const std::string& impl_id, @@ -751,8 +752,10 @@ bool Everest::check_arg(ArgumentType arg_types, json manifest_arg) { // FIXME (aw): the error messages here need to be taken into the // correct context! - if (manifest_arg["type"].is_string()) { - if (manifest_arg["type"] == "null") { + auto& manifest_arg_type = manifest_arg.at("type"); + + if (manifest_arg_type.is_string()) { + if (manifest_arg_type == "null") { // arg_types should be empty if the type is null (void) if (arg_types.size()) { EVLOG_error << "expeceted 'null' type, but got another type"; @@ -762,16 +765,16 @@ bool Everest::check_arg(ArgumentType arg_types, json manifest_arg) { } // direct comparison // FIXME (aw): arg_types[0] access should be checked, otherwise core dumps - if (arg_types[0] != manifest_arg["type"]) { - EVLOG_error << fmt::format("types do not match: {} != {}", arg_types[0], manifest_arg["type"]); + if (arg_types[0] != manifest_arg_type) { + EVLOG_error << fmt::format("types do not match: {} != {}", arg_types[0], manifest_arg_type); return false; } return true; } for (size_t i = 0; i < arg_types.size(); i++) { - if (arg_types[i] != manifest_arg["type"][i]) { - EVLOG_error << fmt::format("types do not match: {} != {}", arg_types[i], manifest_arg["type"][i]); + if (arg_types[i] != manifest_arg_type.at(i)) { + EVLOG_error << fmt::format("types do not match: {} != {}", arg_types[i], manifest_arg_type.at(i)); return false; } } diff --git a/lib/mqtt_abstraction_impl.cpp b/lib/mqtt_abstraction_impl.cpp index afae0b1e..1b84b5df 100644 --- a/lib/mqtt_abstraction_impl.cpp +++ b/lib/mqtt_abstraction_impl.cpp @@ -318,9 +318,12 @@ void MQTTAbstractionImpl::unregister_handler(const std::string& topic, const Tok const std::lock_guard lock(handlers_mutex); auto number_of_handlers = 0; - if (this->message_handlers.count(topic) > 0 && this->message_handlers[topic].count_handlers() != 0) { - this->message_handlers[topic].remove_handler(token); - number_of_handlers = this->message_handlers[topic].count_handlers(); + if (this->message_handlers.find(topic) != this->message_handlers.end()) { + auto& topic_message_handler = this->message_handlers.at(topic); + if (topic_message_handler.count_handlers() != 0) { + topic_message_handler.remove_handler(token); + number_of_handlers = topic_message_handler.count_handlers(); + } } // unsubscribe if this was the last handler for this topic @@ -411,7 +414,7 @@ int MQTTAbstractionImpl::open_nb_socket(const char* addr, const char* port) { } // NOLINTNEXTLINE(misc-no-recursion) -bool MQTTAbstractionImpl::check_topic_matches(std::string full_topic, std::string wildcard_topic) { +bool MQTTAbstractionImpl::check_topic_matches(const std::string& full_topic, const std::string& wildcard_topic) { BOOST_LOG_FUNCTION(); // verbatim topic diff --git a/lib/runtime.cpp b/lib/runtime.cpp index 20bbf996..dd9d315d 100644 --- a/lib/runtime.cpp +++ b/lib/runtime.cpp @@ -295,6 +295,7 @@ RuntimeSettings::RuntimeSettings(const std::string& prefix_, const std::string& } else { telemetry_enabled = defaults::TELEMETRY_ENABLED; } + validate_schema = false; } ModuleCallbacks::ModuleCallbacks(const std::function& register_module_adapter,