From 5127c700d09a1b2d15a71f09c0916d64c5471dcd Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 20 Feb 2020 17:20:56 +0800 Subject: [PATCH] src: refactor debug category parsing Move the debug category parsing into a new EnabledDebugList class, so that it can be reused outside of Environment. PR-URL: https://github.com/nodejs/node/pull/31884 Reviewed-By: Anna Henningsen --- src/debug_utils-inl.h | 74 +++++++++++++++++++++++++++++++++++ src/debug_utils.cc | 29 ++++++++++++++ src/debug_utils.h | 90 ++++++++++++++++++++++++++++--------------- src/env-inl.h | 14 ------- src/env.cc | 29 +------------- src/env.h | 24 ++---------- src/node_env_var.cc | 1 + 7 files changed, 168 insertions(+), 93 deletions(-) diff --git a/src/debug_utils-inl.h b/src/debug_utils-inl.h index 2f6137700d8a37..5593957c5b2c6b 100644 --- a/src/debug_utils-inl.h +++ b/src/debug_utils-inl.h @@ -4,6 +4,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include "debug_utils.h" +#include "env.h" #include @@ -90,6 +91,79 @@ void COLD_NOINLINE FPrintF(FILE* file, const char* format, Args&&... args) { FWrite(file, SPrintF(format, std::forward(args)...)); } +template +inline void FORCE_INLINE Debug(EnabledDebugList* list, + DebugCategory cat, + const char* format, + Args&&... args) { + if (!UNLIKELY(list->enabled(cat))) return; + FPrintF(stderr, format, std::forward(args)...); +} + +inline void FORCE_INLINE Debug(EnabledDebugList* list, + DebugCategory cat, + const char* message) { + if (!UNLIKELY(list->enabled(cat))) return; + FPrintF(stderr, "%s", message); +} + +template +inline void FORCE_INLINE +Debug(Environment* env, DebugCategory cat, const char* format, Args&&... args) { + Debug(env->enabled_debug_list(), cat, format, std::forward(args)...); +} + +inline void FORCE_INLINE Debug(Environment* env, + DebugCategory cat, + const char* message) { + Debug(env->enabled_debug_list(), cat, message); +} + +template +inline void Debug(Environment* env, + DebugCategory cat, + const std::string& format, + Args&&... args) { + Debug(env->enabled_debug_list(), + cat, + format.c_str(), + std::forward(args)...); +} + +// Used internally by the 'real' Debug(AsyncWrap*, ...) functions below, so that +// the FORCE_INLINE flag on them doesn't apply to the contents of this function +// as well. +// We apply COLD_NOINLINE to tell the compiler that it's not worth optimizing +// this function for speed and it should rather focus on keeping it out of +// hot code paths. In particular, we want to keep the string concatenating code +// out of the function containing the original `Debug()` call. +template +void COLD_NOINLINE UnconditionalAsyncWrapDebug(AsyncWrap* async_wrap, + const char* format, + Args&&... args) { + Debug(async_wrap->env(), + static_cast(async_wrap->provider_type()), + async_wrap->diagnostic_name() + " " + format + "\n", + std::forward(args)...); +} + +template +inline void FORCE_INLINE Debug(AsyncWrap* async_wrap, + const char* format, + Args&&... args) { + DCHECK_NOT_NULL(async_wrap); + DebugCategory cat = static_cast(async_wrap->provider_type()); + if (!UNLIKELY(async_wrap->env()->enabled_debug_list()->enabled(cat))) return; + UnconditionalAsyncWrapDebug(async_wrap, format, std::forward(args)...); +} + +template +inline void FORCE_INLINE Debug(AsyncWrap* async_wrap, + const std::string& format, + Args&&... args) { + Debug(async_wrap, format.c_str(), std::forward(args)...); +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/debug_utils.cc b/src/debug_utils.cc index 4553b642b65837..0f25a70e787bce 100644 --- a/src/debug_utils.cc +++ b/src/debug_utils.cc @@ -1,5 +1,6 @@ #include "debug_utils-inl.h" // NOLINT(build/include) #include "env-inl.h" +#include "node_internals.h" #ifdef __POSIX__ #if defined(__linux__) @@ -54,6 +55,34 @@ namespace node { +void EnabledDebugList::Parse(Environment* env) { + std::string cats; + credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env); + Parse(cats, true); +} + +void EnabledDebugList::Parse(const std::string& cats, bool enabled) { + std::string debug_categories = cats; + while (!debug_categories.empty()) { + std::string::size_type comma_pos = debug_categories.find(','); + std::string wanted = ToLower(debug_categories.substr(0, comma_pos)); + +#define V(name) \ + { \ + static const std::string available_category = ToLower(#name); \ + if (available_category.find(wanted) != std::string::npos) \ + set_enabled(DebugCategory::name, enabled); \ + } + + DEBUG_CATEGORY_NAMES(V) +#undef V + + if (comma_pos == std::string::npos) break; + // Use everything after the `,` as the list for the next iteration. + debug_categories = debug_categories.substr(comma_pos + 1); + } +} + #ifdef __POSIX__ #if HAVE_EXECINFO_H class PosixSymbolDebuggingContext final : public NativeSymbolDebuggingContext { diff --git a/src/debug_utils.h b/src/debug_utils.h index c745cbe0a1a74b..527e3549fa0e43 100644 --- a/src/debug_utils.h +++ b/src/debug_utils.h @@ -4,7 +4,6 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include "async_wrap.h" -#include "env.h" #include #include @@ -21,6 +20,7 @@ #endif namespace node { +class Environment; template inline std::string ToString(const T& value); @@ -36,31 +36,71 @@ template inline void FPrintF(FILE* file, const char* format, Args&&... args); void FWrite(FILE* file, const std::string& str); +// Listing the AsyncWrap provider types first enables us to cast directly +// from a provider type to a debug category. +#define DEBUG_CATEGORY_NAMES(V) \ + NODE_ASYNC_PROVIDER_TYPES(V) \ + V(INSPECTOR_SERVER) \ + V(INSPECTOR_PROFILER) \ + V(WASI) + +enum class DebugCategory { +#define V(name) name, + DEBUG_CATEGORY_NAMES(V) +#undef V + CATEGORY_COUNT +}; + +class EnabledDebugList { + public: + bool enabled(DebugCategory category) const { + DCHECK_GE(static_cast(category), 0); + DCHECK_LT(static_cast(category), + static_cast(DebugCategory::CATEGORY_COUNT)); + return enabled_[static_cast(category)]; + } + + // Uses NODE_DEBUG_NATIVE to initialize the categories. When env is not a + // nullptr, the environment variables set in the Environment are used. + // Otherwise the system environment variables are used. + void Parse(Environment* env); + + private: + // Set all categories matching cats to the value of enabled. + void Parse(const std::string& cats, bool enabled); + void set_enabled(DebugCategory category, bool enabled) { + DCHECK_GE(static_cast(category), 0); + DCHECK_LT(static_cast(category), + static_cast(DebugCategory::CATEGORY_COUNT)); + enabled_[static_cast(category)] = true; + } + + bool enabled_[static_cast(DebugCategory::CATEGORY_COUNT)] = {false}; +}; + template -inline void FORCE_INLINE Debug(Environment* env, +inline void FORCE_INLINE Debug(EnabledDebugList* list, DebugCategory cat, const char* format, - Args&&... args) { - if (!UNLIKELY(env->debug_enabled(cat))) - return; - FPrintF(stderr, format, std::forward(args)...); -} + Args&&... args); + +inline void FORCE_INLINE Debug(EnabledDebugList* list, + DebugCategory cat, + const char* message); + +template +inline void FORCE_INLINE +Debug(Environment* env, DebugCategory cat, const char* format, Args&&... args); inline void FORCE_INLINE Debug(Environment* env, DebugCategory cat, - const char* message) { - if (!UNLIKELY(env->debug_enabled(cat))) - return; - FPrintF(stderr, "%s", message); -} + const char* message); template inline void Debug(Environment* env, DebugCategory cat, const std::string& format, - Args&&... args) { - Debug(env, cat, format.c_str(), std::forward(args)...); -} + Args&&... args); // Used internally by the 'real' Debug(AsyncWrap*, ...) functions below, so that // the FORCE_INLINE flag on them doesn't apply to the contents of this function @@ -72,31 +112,17 @@ inline void Debug(Environment* env, template void COLD_NOINLINE UnconditionalAsyncWrapDebug(AsyncWrap* async_wrap, const char* format, - Args&&... args) { - Debug(async_wrap->env(), - static_cast(async_wrap->provider_type()), - async_wrap->diagnostic_name() + " " + format + "\n", - std::forward(args)...); -} + Args&&... args); template inline void FORCE_INLINE Debug(AsyncWrap* async_wrap, const char* format, - Args&&... args) { - DCHECK_NOT_NULL(async_wrap); - DebugCategory cat = - static_cast(async_wrap->provider_type()); - if (!UNLIKELY(async_wrap->env()->debug_enabled(cat))) - return; - UnconditionalAsyncWrapDebug(async_wrap, format, std::forward(args)...); -} + Args&&... args); template inline void FORCE_INLINE Debug(AsyncWrap* async_wrap, const std::string& format, - Args&&... args) { - Debug(async_wrap, format.c_str(), std::forward(args)...); -} + Args&&... args); // Debug helper for inspecting the currently running `node` executable. class NativeSymbolDebuggingContext { diff --git a/src/env-inl.h b/src/env-inl.h index 88c027107020b6..558e257c72ec5a 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -606,20 +606,6 @@ inline void Environment::set_http2_state( http2_state_ = std::move(buffer); } -bool Environment::debug_enabled(DebugCategory category) const { - DCHECK_GE(static_cast(category), 0); - DCHECK_LT(static_cast(category), - static_cast(DebugCategory::CATEGORY_COUNT)); - return debug_enabled_[static_cast(category)]; -} - -void Environment::set_debug_enabled(DebugCategory category, bool enabled) { - DCHECK_GE(static_cast(category), 0); - DCHECK_LT(static_cast(category), - static_cast(DebugCategory::CATEGORY_COUNT)); - debug_enabled_[static_cast(category)] = enabled; -} - inline AliasedFloat64Array* Environment::fs_stats_field_array() { return &fs_stats_field_array_; } diff --git a/src/env.cc b/src/env.cc index 64c60f3ab16cc7..850b96d6857fa2 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1,6 +1,7 @@ #include "env.h" #include "async_wrap.h" +#include "debug_utils-inl.h" #include "memory_tracker-inl.h" #include "node_buffer.h" #include "node_context_data.h" @@ -315,6 +316,7 @@ Environment::Environment(IsolateData* isolate_data, Context::Scope context_scope(context); set_env_vars(per_process::system_environment); + enabled_debug_list_.Parse(this); // We create new copies of the per-Environment option sets, so that it is // easier to modify them after Environment creation. The defaults are @@ -375,10 +377,6 @@ Environment::Environment(IsolateData* isolate_data, // By default, always abort when --abort-on-uncaught-exception was passed. should_abort_on_uncaught_toggle_[0] = 1; - std::string debug_cats; - credentials::SafeGetenv("NODE_DEBUG_NATIVE", &debug_cats, this); - set_debug_categories(debug_cats, true); - if (options_->no_force_async_hooks_checks) { async_hooks_.no_force_checks(); } @@ -867,29 +865,6 @@ Local Environment::GetNow() { return Number::New(isolate(), static_cast(now)); } -void Environment::set_debug_categories(const std::string& cats, bool enabled) { - std::string debug_categories = cats; - while (!debug_categories.empty()) { - std::string::size_type comma_pos = debug_categories.find(','); - std::string wanted = ToLower(debug_categories.substr(0, comma_pos)); - -#define V(name) \ - { \ - static const std::string available_category = ToLower(#name); \ - if (available_category.find(wanted) != std::string::npos) \ - set_debug_enabled(DebugCategory::name, enabled); \ - } - - DEBUG_CATEGORY_NAMES(V) -#undef V - - if (comma_pos == std::string::npos) - break; - // Use everything after the `,` as the list for the next iteration. - debug_categories = debug_categories.substr(comma_pos + 1); - } -} - void CollectExceptionInfo(Environment* env, Local obj, int errorno, diff --git a/src/env.h b/src/env.h index 3bed3c64196ee0..7fbd42e71a1c64 100644 --- a/src/env.h +++ b/src/env.h @@ -29,6 +29,7 @@ #include "inspector_agent.h" #include "inspector_profiler.h" #endif +#include "debug_utils.h" #include "handle_wrap.h" #include "node.h" #include "node_binding.h" @@ -550,20 +551,7 @@ struct ContextInfo { bool is_default = false; }; -// Listing the AsyncWrap provider types first enables us to cast directly -// from a provider type to a debug category. -#define DEBUG_CATEGORY_NAMES(V) \ - NODE_ASYNC_PROVIDER_TYPES(V) \ - V(INSPECTOR_SERVER) \ - V(INSPECTOR_PROFILER) \ - V(WASI) - -enum class DebugCategory { -#define V(name) name, - DEBUG_CATEGORY_NAMES(V) -#undef V - CATEGORY_COUNT -}; +class EnabledDebugList; // A unique-pointer-ish object that is compatible with the JS engine's // ArrayBuffer::Allocator. @@ -1024,9 +1012,7 @@ class Environment : public MemoryRetainer { inline http2::Http2State* http2_state() const; inline void set_http2_state(std::unique_ptr state); - inline bool debug_enabled(DebugCategory category) const; - inline void set_debug_enabled(DebugCategory category, bool enabled); - void set_debug_categories(const std::string& cats, bool enabled); + EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; } inline AliasedFloat64Array* fs_stats_field_array(); inline AliasedBigUint64Array* fs_stats_field_bigint_array(); @@ -1382,9 +1368,7 @@ class Environment : public MemoryRetainer { bool http_parser_buffer_in_use_ = false; std::unique_ptr http2_state_; - bool debug_enabled_[static_cast(DebugCategory::CATEGORY_COUNT)] = { - false}; - + EnabledDebugList enabled_debug_list_; AliasedFloat64Array fs_stats_field_array_; AliasedBigUint64Array fs_stats_field_bigint_array_; diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 6928b595ae5652..208bb6981c2dc8 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -1,3 +1,4 @@ +#include "debug_utils-inl.h" #include "env-inl.h" #include "node_errors.h" #include "node_process.h"