From 031ee32969aceffb01e78424b4304abcb54ccd7d Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Thu, 6 Jul 2023 21:34:14 -0400 Subject: [PATCH] Undoing PR 2004 due to performance concerns. (#2029) * Undoing PR 2004 due to performance concerns. Fixes https://github.com/simdjson/simdjson/issues/2028 Fixes https://github.com/simdjson/simdjson/issues/2021 * Adding final lines. --------- Co-authored-by: Daniel Lemire --- doc/basics.md | 2 +- .../simdjson/generic/ondemand/json_iterator.h | 4 +- .../simdjson/generic/ondemand/logger-inl.h | 59 ++++--------------- include/simdjson/generic/ondemand/logger.h | 14 ++--- .../simdjson/generic/ondemand/object-inl.h | 28 ++------- .../generic/ondemand/token_iterator.h | 4 +- 6 files changed, 27 insertions(+), 84 deletions(-) diff --git a/doc/basics.md b/doc/basics.md index 040fd8057..26e8f8214 100644 --- a/doc/basics.md +++ b/doc/basics.md @@ -1969,4 +1969,4 @@ Performance Tips std::string_view year = data["year"]; std::string_view rating = data["rating"]; ``` -- To better understand the operation of your On Demand parser, and whether it is performing as well as you think it should be, there is a logger feature built in to simdjson! To use it, define the pre-processor directive `SIMDJSON_VERBOSE_LOGGING` prior to including the `simdjson.h` header, which enables logging in simdjson. Run your code. It may generate a lot of logging output; adding printouts from your application that show each section may be helpful. The log’s output will show step-by-step information on state, buffer pointer position, depth, and key retrieval status. The log level is set to `INFO` by default, but can be modified with the environment variable `SIMDJSON_LOG_LEVEL`. Setting the environment variable `SIMDJSON_LOG_LEVEL` to `ERROR` will log only errors. +- To better understand the operation of your On Demand parser, and whether it is performing as well as you think it should be, there is a logger feature built in to simdjson! To use it, define the pre-processor directive `SIMDJSON_VERBOSE_LOGGING` prior to including the `simdjson.h` header, which enables logging in simdjson. Run your code. It may generate a lot of logging output; adding printouts from your application that show each section may be helpful. The log's output will show step-by-step information on state, buffer pointer position, depth, and key retrieval status. Importantly, unless `SIMDJSON_VERBOSE_LOGGING` is defined, logging is entirely disabled and thus carries no overhead. \ No newline at end of file diff --git a/include/simdjson/generic/ondemand/json_iterator.h b/include/simdjson/generic/ondemand/json_iterator.h index acd1a9d2e..32fd30100 100644 --- a/include/simdjson/generic/ondemand/json_iterator.h +++ b/include/simdjson/generic/ondemand/json_iterator.h @@ -298,8 +298,8 @@ protected: friend class raw_json_string; friend class parser; friend class value_iterator; - friend simdjson_inline void logger::log_line(const json_iterator &iter, const char *title_prefix, const char *title, std::string_view detail, int delta, int depth_delta, logger::log_level level) noexcept; - friend simdjson_inline void logger::log_line(const json_iterator &iter, token_position index, depth_t depth, const char *title_prefix, const char *title, std::string_view detail, logger::log_level level) noexcept; + friend simdjson_inline void logger::log_line(const json_iterator &iter, const char *title_prefix, const char *title, std::string_view detail, int delta, int depth_delta) noexcept; + friend simdjson_inline void logger::log_line(const json_iterator &iter, token_position index, depth_t depth, const char *title_prefix, const char *title, std::string_view detail) noexcept; }; // json_iterator } // namespace ondemand diff --git a/include/simdjson/generic/ondemand/logger-inl.h b/include/simdjson/generic/ondemand/logger-inl.h index 89adf516d..fc490af04 100644 --- a/include/simdjson/generic/ondemand/logger-inl.h +++ b/include/simdjson/generic/ondemand/logger-inl.h @@ -1,4 +1,3 @@ -#include namespace simdjson { namespace SIMDJSON_IMPLEMENTATION { namespace ondemand { @@ -19,68 +18,36 @@ static inline char printable_char(char c) { } } -static inline log_level get_log_level_from_env() -{ - SIMDJSON_PUSH_DISABLE_WARNINGS - SIMDJSON_DISABLE_DEPRECATED_WARNING // Disable CRT_SECURE warning on MSVC: manually verified this is safe - char *lvl = getenv("SIMDJSON_LOG_LEVEL"); - SIMDJSON_POP_DISABLE_WARNINGS - if (lvl && simdjson_strcasecmp(lvl, "ERROR") == 0) { return log_level::LOG_ERROR; } - return log_level::LOG_INFO; -} - -static inline log_level log_threshold() -{ - static log_level threshold = get_log_level_from_env(); - return threshold; -} - -static inline bool should_log(log_level level) -{ - return level >= log_threshold(); -} - -template -inline std::string string_format(const std::string& format, const Args&... args) -{ - int size_s = std::snprintf(nullptr, 0, format.c_str(), args...) + 1; - auto size = static_cast(size_s); - if (size <= 0) return std::string(); - std::unique_ptr buf(new char[size]); - std::snprintf(buf.get(), size, format.c_str(), args...); - return std::string(buf.get(), buf.get() + size - 1); -} - inline void log_event(const json_iterator &iter, const char *type, std::string_view detail, int delta, int depth_delta) noexcept { - log_line(iter, "", type, detail, delta, depth_delta, log_level::LOG_INFO); + log_line(iter, "", type, detail, delta, depth_delta); } inline void log_value(const json_iterator &iter, token_position index, depth_t depth, const char *type, std::string_view detail) noexcept { - log_line(iter, index, depth, "", type, detail, log_level::LOG_INFO); + log_line(iter, index, depth, "", type, detail); } inline void log_value(const json_iterator &iter, const char *type, std::string_view detail, int delta, int depth_delta) noexcept { - log_line(iter, "", type, detail, delta, depth_delta, log_level::LOG_INFO); + log_line(iter, "", type, detail, delta, depth_delta); } inline void log_start_value(const json_iterator &iter, token_position index, depth_t depth, const char *type, std::string_view detail) noexcept { - log_line(iter, index, depth, "+", type, detail, log_level::LOG_INFO); + log_line(iter, index, depth, "+", type, detail); if (LOG_ENABLED) { log_depth++; } } inline void log_start_value(const json_iterator &iter, const char *type, int delta, int depth_delta) noexcept { - log_line(iter, "+", type, "", delta, depth_delta, log_level::LOG_INFO); + log_line(iter, "+", type, "", delta, depth_delta); if (LOG_ENABLED) { log_depth++; } } inline void log_end_value(const json_iterator &iter, const char *type, int delta, int depth_delta) noexcept { if (LOG_ENABLED) { log_depth--; } - log_line(iter, "-", type, "", delta, depth_delta, log_level::LOG_INFO); + log_line(iter, "-", type, "", delta, depth_delta); } inline void log_error(const json_iterator &iter, const char *error, const char *detail, int delta, int depth_delta) noexcept { - log_line(iter, "ERROR: ", error, detail, delta, depth_delta, log_level::LOG_ERROR); + log_line(iter, "ERROR: ", error, detail, delta, depth_delta); } inline void log_error(const json_iterator &iter, token_position index, depth_t depth, const char *error, const char *detail) noexcept { - log_line(iter, index, depth, "ERROR: ", error, detail, log_level::LOG_ERROR); + log_line(iter, index, depth, "ERROR: ", error, detail); } inline void log_event(const value_iterator &iter, const char *type, std::string_view detail, int delta, int depth_delta) noexcept { @@ -104,7 +71,7 @@ inline void log_error(const value_iterator &iter, const char *error, const char } inline void log_headers() noexcept { - if (LOG_ENABLED && simdjson_unlikely(should_log(log_level::LOG_INFO))) { + if (LOG_ENABLED) { // Technically a static variable is not thread-safe, but if you are using threads // and logging... well... static bool displayed_hint{false}; @@ -154,11 +121,11 @@ inline void log_headers() noexcept { } } -inline void log_line(const json_iterator &iter, const char *title_prefix, const char *title, std::string_view detail, int delta, int depth_delta, log_level level) noexcept { - log_line(iter, iter.position()+delta, depth_t(iter.depth()+depth_delta), title_prefix, title, detail, level); +inline void log_line(const json_iterator &iter, const char *title_prefix, const char *title, std::string_view detail, int delta, int depth_delta) noexcept { + log_line(iter, iter.position()+delta, depth_t(iter.depth()+depth_delta), title_prefix, title, detail); } -inline void log_line(const json_iterator &iter, token_position index, depth_t depth, const char *title_prefix, const char *title, std::string_view detail, log_level level) noexcept { - if (LOG_ENABLED && simdjson_unlikely(should_log(level))) { +inline void log_line(const json_iterator &iter, token_position index, depth_t depth, const char *title_prefix, const char *title, std::string_view detail) noexcept { + if (LOG_ENABLED) { const int indent = depth*2; const auto buf = iter.token.buf; printf("| %*s%s%-*s ", diff --git a/include/simdjson/generic/ondemand/logger.h b/include/simdjson/generic/ondemand/logger.h index e7cc38c67..88801b194 100644 --- a/include/simdjson/generic/ondemand/logger.h +++ b/include/simdjson/generic/ondemand/logger.h @@ -5,13 +5,11 @@ namespace ondemand { class json_iterator; class value_iterator; +// Logging should be free unless SIMDJSON_VERBOSE_LOGGING is set. Importantly, it is critical +// that the call to the log functions be side-effect free. Thus, for example, you should not +// create temporary std::string instances. namespace logger { -enum class log_level : int32_t { - LOG_INFO = 0, - LOG_ERROR = 1 -}; - #if SIMDJSON_VERBOSE_LOGGING static constexpr const bool LOG_ENABLED = true; #else @@ -21,11 +19,9 @@ enum class log_level : int32_t { // We do not want these functions to be 'really inlined' since real inlining is // for performance purposes and if you are using the loggers, you do not care about // performance (or should not). -template -static inline std::string string_format(const std::string& format, const Args&... args); static inline void log_headers() noexcept; -static inline void log_line(const json_iterator &iter, token_position index, depth_t depth, const char *title_prefix, const char *title, std::string_view detail, log_level level) noexcept; -static inline void log_line(const json_iterator &iter, const char *title_prefix, const char *title, std::string_view detail, int delta, int depth_delta, log_level level) noexcept; +static inline void log_line(const json_iterator &iter, token_position index, depth_t depth, const char *title_prefix, const char *title, std::string_view detail) noexcept; +static inline void log_line(const json_iterator &iter, const char *title_prefix, const char *title, std::string_view detail, int delta, int depth_delta) noexcept; static inline void log_event(const json_iterator &iter, const char *type, std::string_view detail="", int delta=0, int depth_delta=0) noexcept; static inline void log_value(const json_iterator &iter, token_position index, depth_t depth, const char *type, std::string_view detail="") noexcept; static inline void log_value(const json_iterator &iter, const char *type, std::string_view detail="", int delta=-1, int depth_delta=0) noexcept; diff --git a/include/simdjson/generic/ondemand/object-inl.h b/include/simdjson/generic/ondemand/object-inl.h index 935fdda19..e86922406 100644 --- a/include/simdjson/generic/ondemand/object-inl.h +++ b/include/simdjson/generic/ondemand/object-inl.h @@ -5,23 +5,13 @@ namespace ondemand { simdjson_inline simdjson_result object::find_field_unordered(const std::string_view key) & noexcept { bool has_value; SIMDJSON_TRY( iter.find_field_unordered_raw(key).get(has_value) ); - if (!has_value) { - auto key_str = std::string(key.data(), key.size()); - auto msg = logger::string_format("Cannot find key: %s", key_str.c_str()); - logger::log_error(iter, msg.c_str()); - return NO_SUCH_FIELD; - } + if (!has_value) { return NO_SUCH_FIELD; } return value(iter.child()); } simdjson_inline simdjson_result object::find_field_unordered(const std::string_view key) && noexcept { bool has_value; SIMDJSON_TRY( iter.find_field_unordered_raw(key).get(has_value) ); - if (!has_value) { - auto key_str = std::string(key.data(), key.size()); - auto msg = logger::string_format("Cannot find key: %s", key_str.c_str()); - logger::log_error(iter, msg.c_str()); - return NO_SUCH_FIELD; - } + if (!has_value) { return NO_SUCH_FIELD; } return value(iter.child()); } simdjson_inline simdjson_result object::operator[](const std::string_view key) & noexcept { @@ -33,23 +23,13 @@ simdjson_inline simdjson_result object::operator[](const std::string_view simdjson_inline simdjson_result object::find_field(const std::string_view key) & noexcept { bool has_value; SIMDJSON_TRY( iter.find_field_raw(key).get(has_value) ); - if (!has_value) { - auto key_str = std::string(key.data(), key.size()); - auto msg = logger::string_format("Cannot find key: %s", key_str.c_str()); - logger::log_error(iter, msg.c_str()); - return NO_SUCH_FIELD; - } + if (!has_value) { return NO_SUCH_FIELD; } return value(iter.child()); } simdjson_inline simdjson_result object::find_field(const std::string_view key) && noexcept { bool has_value; SIMDJSON_TRY( iter.find_field_raw(key).get(has_value) ); - if (!has_value) { - auto key_str = std::string(key.data(), key.size()); - auto msg = logger::string_format("Cannot find key: %s", key_str.c_str()); - logger::log_error(iter, msg.c_str()); - return NO_SUCH_FIELD; - } + if (!has_value) { return NO_SUCH_FIELD; } return value(iter.child()); } diff --git a/include/simdjson/generic/ondemand/token_iterator.h b/include/simdjson/generic/ondemand/token_iterator.h index bd2d147b0..d85374a16 100644 --- a/include/simdjson/generic/ondemand/token_iterator.h +++ b/include/simdjson/generic/ondemand/token_iterator.h @@ -116,8 +116,8 @@ protected: friend class json_iterator; friend class value_iterator; friend class object; - friend simdjson_inline void logger::log_line(const json_iterator &iter, const char *title_prefix, const char *title, std::string_view detail, int delta, int depth_delta, logger::log_level lvl) noexcept; - friend simdjson_inline void logger::log_line(const json_iterator &iter, token_position index, depth_t depth, const char *title_prefix, const char *title, std::string_view detail, logger::log_level lvl) noexcept; + friend simdjson_inline void logger::log_line(const json_iterator &iter, const char *title_prefix, const char *title, std::string_view detail, int delta, int depth_delta) noexcept; + friend simdjson_inline void logger::log_line(const json_iterator &iter, token_position index, depth_t depth, const char *title_prefix, const char *title, std::string_view detail) noexcept; }; } // namespace ondemand