Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: implement per-process native Debug() printer and use it in mkcodecache #31884

Closed
wants to merge 7 commits into from
Closed

src: implement per-process native Debug() printer and use it in mkcodecache #31884

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4439730
src: make aliased_buffer.h self-contained
joyeecheung Feb 20, 2020
2b80488
src: refactor debug category parsing
joyeecheung Feb 20, 2020
ce2d7fe
src: implement per-process native Debug() printer
joyeecheung Feb 20, 2020
9abbce6
tools: use per-process native Debug() printer in mkcodecache
joyeecheung Feb 20, 2020
c7c01d0
fixup! src: refactor debug category parsing
joyeecheung Feb 25, 2020
d86480a
fixup! src: implement per-process native Debug() printer
joyeecheung Feb 25, 2020
de4b6c7
fixup! tools: use per-process native Debug() printer in mkcodecache
joyeecheung Feb 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/aliased_buffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include <cinttypes>
#include "util.h"
#include "util-inl.h"
#include "v8.h"

namespace node {
Expand Down
29 changes: 29 additions & 0 deletions src/debug_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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__)
Expand Down Expand Up @@ -54,6 +55,34 @@

namespace node {

EnabledDebugList::EnabledDebugList(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 {
Expand Down
87 changes: 78 additions & 9 deletions src/debug_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,86 @@ 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(CODE_CACHE) \
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.
EnabledDebugList(Environment* env);
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved

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;
if (!UNLIKELY(list->enabled(cat))) return;
FPrintF(stderr, format, std::forward<Args>(args)...);
}

inline void FORCE_INLINE Debug(Environment* env,
inline void FORCE_INLINE Debug(EnabledDebugList* list,
DebugCategory cat,
const char* message) {
if (!UNLIKELY(env->debug_enabled(cat)))
return;
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, cat, format.c_str(), std::forward<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
Expand All @@ -86,8 +142,7 @@ inline void FORCE_INLINE Debug(AsyncWrap* async_wrap,
DCHECK_NOT_NULL(async_wrap);
DebugCategory cat =
static_cast<DebugCategory>(async_wrap->provider_type());
if (!UNLIKELY(async_wrap->env()->debug_enabled(cat)))
return;
if (!UNLIKELY(async_wrap->env()->enabled_debug_list()->enabled(cat))) return;
UnconditionalAsyncWrapDebug(async_wrap, format, std::forward<Args>(args)...);
}

Expand Down Expand Up @@ -135,6 +190,20 @@ class NativeSymbolDebuggingContext {
void CheckedUvLoopClose(uv_loop_t* loop);
void PrintLibuvHandleInformation(uv_loop_t* loop, FILE* stream);

namespace per_process {
extern std::shared_ptr<EnabledDebugList> enabled_debug_list;

template <typename... Args>
inline void FORCE_INLINE Debug(DebugCategory cat,
const char* format,
Args&&... args) {
Debug(enabled_debug_list.get(), cat, format, std::forward<Args>(args)...);
}

inline void FORCE_INLINE Debug(DebugCategory cat, const char* message) {
Debug(enabled_debug_list.get(), cat, message);
}
} // namespace per_process
} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
14 changes: 0 additions & 14 deletions src/env-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,20 +609,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_;
}
Expand Down
29 changes: 2 additions & 27 deletions src/env.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -315,6 +316,7 @@ Environment::Environment(IsolateData* isolate_data,
Context::Scope context_scope(context);

set_env_vars(per_process::system_environment);
enabled_debug_list_ = std::make_unique<EnabledDebugList>(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
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -864,29 +862,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,
Expand Down
25 changes: 5 additions & 20 deletions src/env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,20 +547,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.
Expand Down Expand Up @@ -1022,9 +1009,9 @@ 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() const {
return enabled_debug_list_.get();
}

inline AliasedFloat64Array* fs_stats_field_array();
inline AliasedBigUint64Array* fs_stats_field_bigint_array();
Expand Down Expand Up @@ -1384,9 +1371,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};

std::unique_ptr<EnabledDebugList> enabled_debug_list_;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason that this would need to be a pointer? I don’t think that’s necessary, and it comes with one additional layer of indirection for each debug check. One of the goals for the Debug() implementation was to have as close to zero overhead as possible, and I think this might get in the way of it…

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly to avoid the circular dependency - but I guess this can also be achieved by moving more things from debug_utils.h to debug_utils-inl.h so that env.h can just include debug_utils.h and debug_utils.h do not need to include env.h ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that works, then that would be great – otherwise we might need to move more things into env.h…

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, looks like if we want to keep the initialization in the constructor, then it needs to be a pointer since we'd need the environment pointer with initialized env_vars for credentials::SafeGetenv(). So I just moved the initialization into EnableDebugList::Parse(), and call it in InitializeOncePerProcess (and separately, in mkcodecache since it does not need InitializeOncePerProcess()), this also means that for now the printer is not usable until node::Start() is called in the default binary, but I guess that's fine, compared to reordering the whole initialization sequence to work around pointers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, what I had in mind is to use the trivial copy constructor to re-initialize it later, e.g. enabled_debug_list_ = EnabledDebugList(...);, but I guess ::Parse() works just as well :)

AliasedFloat64Array fs_stats_field_array_;
AliasedBigUint64Array fs_stats_field_bigint_array_;

Expand Down
3 changes: 3 additions & 0 deletions src/node_env_var.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "debug_utils-inl.h"
#include "env-inl.h"
#include "node_errors.h"
#include "node_process.h"
Expand Down Expand Up @@ -57,6 +58,8 @@ class MapKVStore final : public KVStore {
namespace per_process {
Mutex env_var_mutex;
std::shared_ptr<KVStore> system_environment = std::make_shared<RealEnvStore>();
std::shared_ptr<EnabledDebugList> enabled_debug_list =
std::make_shared<EnabledDebugList>(nullptr);
addaleax marked this conversation as resolved.
Show resolved Hide resolved
} // namespace per_process

template <typename T>
Expand Down
25 changes: 10 additions & 15 deletions tools/code_cache/cache_builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "cache_builder.h"
#include "debug_utils-inl.h"
#include "node_native_module.h"
#include "util.h"

Expand Down Expand Up @@ -67,8 +68,7 @@ static void GetInitializer(const std::string& id, std::stringstream& ss) {
}

static std::string GenerateCodeCache(
const std::map<std::string, ScriptCompiler::CachedData*>& data,
bool log_progress) {
const std::map<std::string, ScriptCompiler::CachedData*>& data) {
std::stringstream ss;
ss << R"(#include <cinttypes>
#include "node_native_module_env.h"
Expand All @@ -89,11 +89,13 @@ const bool has_code_cache = true;
total += cached_data->length;
std::string def = GetDefinition(id, cached_data->length, cached_data->data);
ss << def << "\n\n";
if (log_progress) {
std::cout << "Generated cache for " << id
<< ", size = " << FormatSize(cached_data->length)
<< ", total = " << FormatSize(total) << "\n";
}
std::string size_str = FormatSize(cached_data->length);
std::string total_str = FormatSize(total);
per_process::Debug(DebugCategory::CODE_CACHE,
"Generated cache for %s, size = %s, total = %s",
id.c_str(),
size_str.c_str(),
total_str.c_str());
}

ss << R"(void NativeModuleEnv::InitializeCodeCache() {
Expand Down Expand Up @@ -142,14 +144,7 @@ std::string CodeCacheBuilder::Generate(Local<Context> context) {
}
}

char env_buf[32];
size_t env_size = sizeof(env_buf);
int ret = uv_os_getenv("NODE_DEBUG", env_buf, &env_size);
bool log_progress = false;
if (ret == 0 && strcmp(env_buf, "mkcodecache") == 0) {
log_progress = true;
}
return GenerateCodeCache(data, log_progress);
return GenerateCodeCache(data);
}

} // namespace native_module
Expand Down