Skip to content

Commit 966546c

Browse files
joyeecheungrvagg
authored andcommitted
process: simplify the setup of async hooks trace events
- Remove `trace_category_state` from `Environment` - since this is only accessed in the bootstrap process and later in the trace category update handler, we could just pass the initial values into JS land via the trace_events binding, and pass the dynamic values directly to the handler later, instead of accessing them out-of-band via the AliasedBuffer. - Instead of creating the hooks directly in `trace_events_async_hooks.js`, export the hook factory and create the hooks in trace category state toggle. PR-URL: #26062 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 5cc2574 commit 966546c

File tree

6 files changed

+35
-38
lines changed

6 files changed

+35
-38
lines changed

lib/internal/bootstrap/pre_execution.js

+18-20
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
'use strict';
22

33
const { getOptionValue } = require('internal/options');
4+
// Lazy load internal/trace_events_async_hooks only if the async_hooks
5+
// trace event category is enabled.
6+
let traceEventsAsyncHook;
47

58
function prepareMainThreadExecution() {
69
setupTraceCategoryState();
@@ -27,32 +30,27 @@ function prepareMainThreadExecution() {
2730

2831
function setupTraceCategoryState() {
2932
const {
30-
traceCategoryState,
33+
asyncHooksEnabledInitial,
3134
setTraceCategoryStateUpdateHandler
3235
} = internalBinding('trace_events');
33-
const kCategoryAsyncHooks = 0;
34-
let traceEventsAsyncHook;
35-
36-
function toggleTraceCategoryState() {
37-
// Dynamically enable/disable the traceEventsAsyncHook
38-
const asyncHooksEnabled = !!traceCategoryState[kCategoryAsyncHooks];
39-
40-
if (asyncHooksEnabled) {
41-
// Lazy load internal/trace_events_async_hooks only if the async_hooks
42-
// trace event category is enabled.
43-
if (!traceEventsAsyncHook) {
44-
traceEventsAsyncHook = require('internal/trace_events_async_hooks');
45-
}
46-
traceEventsAsyncHook.enable();
47-
} else if (traceEventsAsyncHook) {
48-
traceEventsAsyncHook.disable();
49-
}
50-
}
5136

52-
toggleTraceCategoryState();
37+
toggleTraceCategoryState(asyncHooksEnabledInitial);
5338
setTraceCategoryStateUpdateHandler(toggleTraceCategoryState);
5439
}
5540

41+
// Dynamically enable/disable the traceEventsAsyncHook
42+
function toggleTraceCategoryState(asyncHooksEnabled) {
43+
if (asyncHooksEnabled) {
44+
if (!traceEventsAsyncHook) {
45+
traceEventsAsyncHook =
46+
require('internal/trace_events_async_hooks').createHook();
47+
}
48+
traceEventsAsyncHook.enable();
49+
} else if (traceEventsAsyncHook) {
50+
traceEventsAsyncHook.disable();
51+
}
52+
}
53+
5654
// In general deprecations are intialized wherever the APIs are implemented,
5755
// this is used to deprecate APIs implemented in C++ where the deprecation
5856
// utitlities are not easily accessible.

lib/internal/trace_events_async_hooks.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,4 @@ function createHook() {
8383
};
8484
}
8585

86-
module.exports = createHook();
86+
exports.createHook = createHook;

src/env-inl.h

-5
Original file line numberDiff line numberDiff line change
@@ -455,11 +455,6 @@ Environment::should_abort_on_uncaught_toggle() {
455455
return should_abort_on_uncaught_toggle_;
456456
}
457457

458-
inline AliasedBuffer<uint8_t, v8::Uint8Array>&
459-
Environment::trace_category_state() {
460-
return trace_category_state_;
461-
}
462-
463458
inline AliasedBuffer<int32_t, v8::Int32Array>&
464459
Environment::stream_base_state() {
465460
return stream_base_state_;

src/env.cc

+6-6
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
namespace node {
2222

2323
using errors::TryCatchScope;
24+
using v8::Boolean;
2425
using v8::Context;
2526
using v8::EmbedderGraph;
2627
using v8::External;
@@ -152,9 +153,8 @@ void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() {
152153
return;
153154
}
154155

155-
env_->trace_category_state()[0] =
156-
*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
157-
TRACING_CATEGORY_NODE1(async_hooks));
156+
bool async_hooks_enabled = (*(TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
157+
TRACING_CATEGORY_NODE1(async_hooks)))) != 0;
158158

159159
Isolate* isolate = env_->isolate();
160160
HandleScope handle_scope(isolate);
@@ -163,8 +163,9 @@ void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() {
163163
return;
164164
TryCatchScope try_catch(env_);
165165
try_catch.SetVerbose(true);
166-
cb->Call(env_->context(), Undefined(isolate),
167-
0, nullptr).ToLocalChecked();
166+
Local<Value> args[] = {Boolean::New(isolate, async_hooks_enabled)};
167+
cb->Call(env_->context(), Undefined(isolate), arraysize(args), args)
168+
.ToLocalChecked();
168169
}
169170

170171
static std::atomic<uint64_t> next_thread_id{0};
@@ -183,7 +184,6 @@ Environment::Environment(IsolateData* isolate_data,
183184
tick_info_(context->GetIsolate()),
184185
timer_base_(uv_now(isolate_data->event_loop())),
185186
should_abort_on_uncaught_toggle_(isolate_, 1),
186-
trace_category_state_(isolate_, kTraceCategoryCount),
187187
stream_base_state_(isolate_, StreamBase::kNumStreamBaseStateFields),
188188
flags_(flags),
189189
thread_id_(thread_id == kNoThreadId ? AllocateThreadId() : thread_id),

src/env.h

-3
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,6 @@ class Environment {
705705
inline AliasedBuffer<uint32_t, v8::Uint32Array>&
706706
should_abort_on_uncaught_toggle();
707707

708-
inline AliasedBuffer<uint8_t, v8::Uint8Array>& trace_category_state();
709708
inline AliasedBuffer<int32_t, v8::Int32Array>& stream_base_state();
710709

711710
// The necessary API for async_hooks.
@@ -1026,8 +1025,6 @@ class Environment {
10261025
AliasedBuffer<uint32_t, v8::Uint32Array> should_abort_on_uncaught_toggle_;
10271026
int should_not_abort_scope_counter_ = 0;
10281027

1029-
// Attached to a Uint8Array that tracks the state of trace category
1030-
AliasedBuffer<uint8_t, v8::Uint8Array> trace_category_state_;
10311028
std::unique_ptr<TrackingTraceStateObserver> trace_state_observer_;
10321029

10331030
AliasedBuffer<int32_t, v8::Int32Array> stream_base_state_;

src/node_trace_events.cc

+10-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "base_object-inl.h"
22
#include "env.h"
33
#include "node.h"
4+
#include "node_internals.h"
45
#include "node_v8_platform-inl.h"
56
#include "tracing/agent.h"
67

@@ -10,6 +11,7 @@
1011
namespace node {
1112

1213
using v8::Array;
14+
using v8::Boolean;
1315
using v8::Context;
1416
using v8::Function;
1517
using v8::FunctionCallbackInfo;
@@ -148,9 +150,14 @@ void NodeCategorySet::Initialize(Local<Object> target,
148150
target->Set(context, trace,
149151
binding->Get(context, trace).ToLocalChecked()).FromJust();
150152

151-
target->Set(context,
152-
FIXED_ONE_BYTE_STRING(env->isolate(), "traceCategoryState"),
153-
env->trace_category_state().GetJSArray()).FromJust();
153+
// Initial value of async hook trace events
154+
bool async_hooks_enabled = (*(TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
155+
TRACING_CATEGORY_NODE1(async_hooks)))) != 0;
156+
target
157+
->Set(context,
158+
FIXED_ONE_BYTE_STRING(env->isolate(), "asyncHooksEnabledInitial"),
159+
Boolean::New(env->isolate(), async_hooks_enabled))
160+
.FromJust();
154161
}
155162

156163
} // namespace node

0 commit comments

Comments
 (0)