src: refactor debug category parsing

joyeecheung · MylesBorins · commit 5127c700d09a · 2020-03-10T18:00:31.000-04:00
Move the debug category parsing into a new EnabledDebugList class, so that it can be reused outside of Environment. PR-URL: #31884 Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git 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 <type_traits>
 
@@ -90,6 +91,79 @@ void COLD_NOINLINE FPrintF(FILE* file, const char* format, Args&&... args) {
   FWrite(file, SPrintF(format, std::forward<Args>(args)...));
 }
 
+template <typename... Args>
+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>(args)...);
+}
+
+inline void FORCE_INLINE Debug(EnabledDebugList* list,
+                               DebugCategory cat,
+                               const char* message) {
+  if (!UNLIKELY(list->enabled(cat))) return;
+  FPrintF(stderr, "%s", message);
+}
+
+template <typename... Args>
+inline void FORCE_INLINE
+Debug(Environment* env, DebugCategory cat, const char* format, Args&&... args) {
+  Debug(env->enabled_debug_list(), cat, format, std::forward<Args>(args)...);
+}
+
+inline void FORCE_INLINE Debug(Environment* env,
+                               DebugCategory cat,
+                               const char* message) {
+  Debug(env->enabled_debug_list(), cat, message);
+}
+
+template <typename... Args>
+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>(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 <typename... Args>
+void COLD_NOINLINE UnconditionalAsyncWrapDebug(AsyncWrap* async_wrap,
+                                               const char* format,
+                                               Args&&... args) {
+  Debug(async_wrap->env(),
+        static_cast<DebugCategory>(async_wrap->provider_type()),
+        async_wrap->diagnostic_name() + " " + format + "\n",
+        std::forward<Args>(args)...);
+}
+
+template <typename... Args>
+inline void FORCE_INLINE Debug(AsyncWrap* async_wrap,
+                               const char* format,
+                               Args&&... args) {
+  DCHECK_NOT_NULL(async_wrap);
+  DebugCategory cat = static_cast<DebugCategory>(async_wrap->provider_type());
+  if (!UNLIKELY(async_wrap->env()->enabled_debug_list()->enabled(cat))) return;
+  UnconditionalAsyncWrapDebug(async_wrap, format, std::forward<Args>(args)...);
+}
+
+template <typename... Args>
+inline void FORCE_INLINE Debug(AsyncWrap* async_wrap,
+                               const std::string& format,
+                               Args&&... args) {
+  Debug(async_wrap, format.c_str(), std::forward<Args>(args)...);
+}
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
diff --git 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
@@ -4,7 +4,6 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include "async_wrap.h"
-#include "env.h"
 
 #include <sstream>
 #include <string>
@@ -21,6 +20,7 @@
 #endif
 
 namespace node {
+class Environment;
 
 template <typename T>
 inline std::string ToString(const T& value);
@@ -36,31 +36,71 @@ template <typename... Args>
 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<int>(category), 0);
+    DCHECK_LT(static_cast<int>(category),
+              static_cast<int>(DebugCategory::CATEGORY_COUNT));
+    return enabled_[static_cast<int>(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<int>(category), 0);
+    DCHECK_LT(static_cast<int>(category),
+              static_cast<int>(DebugCategory::CATEGORY_COUNT));
+    enabled_[static_cast<int>(category)] = true;
+  }
+
+  bool enabled_[static_cast<int>(DebugCategory::CATEGORY_COUNT)] = {false};
+};
+
 template <typename... Args>
-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&&... args);
+
+inline void FORCE_INLINE Debug(EnabledDebugList* list,
+                               DebugCategory cat,
+                               const char* message);
+
+template <typename... Args>
+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 <typename... Args>
 inline void Debug(Environment* env,
                   DebugCategory cat,
                   const std::string& format,
-                  Args&&... args) {
-  Debug(env, cat, format.c_str(), std::forward<Args>(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 <typename... Args>
 void COLD_NOINLINE UnconditionalAsyncWrapDebug(AsyncWrap* async_wrap,
                                                const char* format,
-                                               Args&&... args) {
-  Debug(async_wrap->env(),
-        static_cast<DebugCategory>(async_wrap->provider_type()),
-        async_wrap->diagnostic_name() + " " + format + "\n",
-        std::forward<Args>(args)...);
-}
+                                               Args&&... args);
 
 template <typename... Args>
 inline void FORCE_INLINE Debug(AsyncWrap* async_wrap,
                                const char* format,
-                               Args&&... args) {
-  DCHECK_NOT_NULL(async_wrap);
-  DebugCategory cat =
-      static_cast<DebugCategory>(async_wrap->provider_type());
-  if (!UNLIKELY(async_wrap->env()->debug_enabled(cat)))
-    return;
-  UnconditionalAsyncWrapDebug(async_wrap, format, std::forward<Args>(args)...);
-}
+                               Args&&... args);
 
 template <typename... Args>
 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&&... args);
 
 // Debug helper for inspecting the currently running `node` executable.
 class NativeSymbolDebuggingContext {
diff --git 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<int>(category), 0);
-  DCHECK_LT(static_cast<int>(category),
-           static_cast<int>(DebugCategory::CATEGORY_COUNT));
-  return debug_enabled_[static_cast<int>(category)];
-}
-
-void Environment::set_debug_enabled(DebugCategory category, bool enabled) {
-  DCHECK_GE(static_cast<int>(category), 0);
-  DCHECK_LT(static_cast<int>(category),
-           static_cast<int>(DebugCategory::CATEGORY_COUNT));
-  debug_enabled_[static_cast<int>(category)] = enabled;
-}
-
 inline AliasedFloat64Array* Environment::fs_stats_field_array() {
   return &fs_stats_field_array_;
 }
diff --git 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<Value> Environment::GetNow() {
     return Number::New(isolate(), static_cast<double>(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<Object> obj,
                           int errorno,
diff --git 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<http2::Http2State> 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::Http2State> http2_state_;
 
-  bool debug_enabled_[static_cast<int>(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
