Skip to content

Commit b61bbbb

Browse files
ofrobotsjasnell
authored andcommitted
src: trace_event: secondary storage for metadata
Metadata trace-events should be held in secondary storage so that they can be periodically reemitted. This change establishes the secondary storage and ensures that events are reemitted on each flush. PR-URL: #20900 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent bd88c98 commit b61bbbb

10 files changed

+96
-35
lines changed

src/node.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,9 @@ static struct {
271271
#if NODE_USE_V8_PLATFORM
272272
void Initialize(int thread_pool_size) {
273273
tracing_agent_.reset(new tracing::Agent());
274+
node::tracing::TraceEventHelper::SetAgent(tracing_agent_.get());
274275
auto controller = tracing_agent_->GetTracingController();
275276
controller->AddTraceStateObserver(new NodeTraceStateObserver(controller));
276-
tracing::TraceEventHelper::SetTracingController(controller);
277277
StartTracingAgent();
278278
// Tracing must be initialized before platform threads are created.
279279
platform_ = new NodePlatform(thread_pool_size, controller);
@@ -2763,7 +2763,7 @@ MultiIsolatePlatform* GetMainThreadMultiIsolatePlatform() {
27632763

27642764
MultiIsolatePlatform* CreatePlatform(
27652765
int thread_pool_size,
2766-
TracingController* tracing_controller) {
2766+
node::tracing::TracingController* tracing_controller) {
27672767
return new NodePlatform(thread_pool_size, tracing_controller);
27682768
}
27692769

src/node_platform.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ using v8::Local;
1414
using v8::Object;
1515
using v8::Platform;
1616
using v8::Task;
17-
using v8::TracingController;
17+
using node::tracing::TracingController;
1818

1919
namespace {
2020

src/node_platform.h

+5-3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "libplatform/libplatform.h"
1212
#include "node.h"
1313
#include "node_mutex.h"
14+
#include "tracing/agent.h"
1415
#include "uv.h"
1516

1617
namespace node {
@@ -124,7 +125,8 @@ class WorkerThreadsTaskRunner {
124125

125126
class NodePlatform : public MultiIsolatePlatform {
126127
public:
127-
NodePlatform(int thread_pool_size, v8::TracingController* tracing_controller);
128+
NodePlatform(int thread_pool_size,
129+
node::tracing::TracingController* tracing_controller);
128130
virtual ~NodePlatform() {}
129131

130132
void DrainTasks(v8::Isolate* isolate) override;
@@ -142,7 +144,7 @@ class NodePlatform : public MultiIsolatePlatform {
142144
bool IdleTasksEnabled(v8::Isolate* isolate) override;
143145
double MonotonicallyIncreasingTime() override;
144146
double CurrentClockTimeMillis() override;
145-
v8::TracingController* GetTracingController() override;
147+
node::tracing::TracingController* GetTracingController() override;
146148
bool FlushForegroundTasks(v8::Isolate* isolate) override;
147149

148150
void RegisterIsolate(v8::Isolate* isolate, uv_loop_t* loop) override;
@@ -158,7 +160,7 @@ class NodePlatform : public MultiIsolatePlatform {
158160
std::unordered_map<v8::Isolate*,
159161
std::shared_ptr<PerIsolatePlatformData>> per_isolate_;
160162

161-
v8::TracingController* tracing_controller_;
163+
node::tracing::TracingController* tracing_controller_;
162164
std::shared_ptr<WorkerThreadsTaskRunner> worker_thread_task_runner_;
163165
};
164166

src/tracing/agent.cc

+27
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "tracing/agent.h"
22

33
#include <string>
4+
#include "trace_event.h"
45
#include "tracing/node_trace_buffer.h"
56
#include "debug_utils.h"
67
#include "env-inl.h"
@@ -207,9 +208,35 @@ void Agent::AppendTraceEvent(TraceObject* trace_event) {
207208
}
208209

209210
void Agent::Flush(bool blocking) {
211+
for (const auto& event : metadata_events_)
212+
AppendTraceEvent(event.get());
213+
210214
for (const auto& id_writer : writers_)
211215
id_writer.second->Flush(blocking);
212216
}
213217

218+
void TracingController::AddMetadataEvent(
219+
const unsigned char* category_group_enabled,
220+
const char* name,
221+
int num_args,
222+
const char** arg_names,
223+
const unsigned char* arg_types,
224+
const uint64_t* arg_values,
225+
std::unique_ptr<v8::ConvertableToTraceFormat>* convertable_values,
226+
unsigned int flags) {
227+
std::unique_ptr<TraceObject> trace_event(new TraceObject);
228+
trace_event->Initialize(
229+
TRACE_EVENT_PHASE_METADATA, category_group_enabled, name,
230+
node::tracing::kGlobalScope, // scope
231+
node::tracing::kNoId, // id
232+
node::tracing::kNoId, // bind_id
233+
num_args, arg_names, arg_types, arg_values, convertable_values,
234+
TRACE_EVENT_FLAG_NONE,
235+
CurrentTimestampMicroseconds(),
236+
CurrentCpuTimestampMicroseconds());
237+
node::tracing::TraceEventHelper::GetAgent()->AddMetadataEvent(
238+
std::move(trace_event));
239+
}
240+
214241
} // namespace tracing
215242
} // namespace node

src/tracing/agent.h

+15
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include "util.h"
88
#include "node_mutex.h"
99

10+
#include <list>
1011
#include <set>
1112
#include <string>
1213
#include <unordered_map>
@@ -34,6 +35,15 @@ class TracingController : public v8::platform::tracing::TracingController {
3435
int64_t CurrentTimestampMicroseconds() override {
3536
return uv_hrtime() / 1000;
3637
}
38+
void AddMetadataEvent(
39+
const unsigned char* category_group_enabled,
40+
const char* name,
41+
int num_args,
42+
const char** arg_names,
43+
const unsigned char* arg_types,
44+
const uint64_t* arg_values,
45+
std::unique_ptr<v8::ConvertableToTraceFormat>* convertable_values,
46+
unsigned int flags);
3747
};
3848

3949
class AgentWriterHandle {
@@ -93,6 +103,10 @@ class Agent {
93103

94104
// Writes to all writers registered through AddClient().
95105
void AppendTraceEvent(TraceObject* trace_event);
106+
107+
void AddMetadataEvent(std::unique_ptr<TraceObject> event) {
108+
metadata_events_.push_back(std::move(event));
109+
}
96110
// Flushes all writers registered through AddClient().
97111
void Flush(bool blocking);
98112

@@ -131,6 +145,7 @@ class Agent {
131145
ConditionVariable initialize_writer_condvar_;
132146
uv_async_t initialize_writer_async_;
133147
std::set<AsyncTraceWriter*> to_be_initialized_;
148+
std::list<std::unique_ptr<TraceObject>> metadata_events_;
134149
};
135150

136151
void AgentWriterHandle::reset() {

src/tracing/trace_event.cc

+9-5
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,18 @@
33
namespace node {
44
namespace tracing {
55

6-
v8::TracingController* g_controller = nullptr;
6+
Agent* g_agent = nullptr;
77

8-
void TraceEventHelper::SetTracingController(v8::TracingController* controller) {
9-
g_controller = controller;
8+
void TraceEventHelper::SetAgent(Agent* agent) {
9+
g_agent = agent;
1010
}
1111

12-
v8::TracingController* TraceEventHelper::GetTracingController() {
13-
return g_controller;
12+
Agent* TraceEventHelper::GetAgent() {
13+
return g_agent;
14+
}
15+
16+
TracingController* TraceEventHelper::GetTracingController() {
17+
return g_agent->GetTracingController();
1418
}
1519

1620
} // namespace tracing

src/tracing/trace_event.h

+27-13
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,9 @@ const uint64_t kNoId = 0;
310310

311311
class TraceEventHelper {
312312
public:
313-
static v8::TracingController* GetTracingController();
314-
static void SetTracingController(v8::TracingController* controller);
313+
static TracingController* GetTracingController();
314+
static Agent* GetAgent();
315+
static void SetAgent(Agent* agent);
315316
};
316317

317318
// TraceID encapsulates an ID that can either be an integer or pointer. Pointers
@@ -487,6 +488,26 @@ static V8_INLINE uint64_t AddTraceEventWithTimestampImpl(
487488
arg_names, arg_types, arg_values, arg_convertables, flags, timestamp);
488489
}
489490

491+
static V8_INLINE void AddMetadataEventImpl(
492+
const uint8_t* category_group_enabled, const char* name, int32_t num_args,
493+
const char** arg_names, const uint8_t* arg_types,
494+
const uint64_t* arg_values, unsigned int flags) {
495+
std::unique_ptr<v8::ConvertableToTraceFormat> arg_convertibles[2];
496+
if (num_args > 0 && arg_types[0] == TRACE_VALUE_TYPE_CONVERTABLE) {
497+
arg_convertibles[0].reset(reinterpret_cast<v8::ConvertableToTraceFormat*>(
498+
static_cast<intptr_t>(arg_values[0])));
499+
}
500+
if (num_args > 1 && arg_types[1] == TRACE_VALUE_TYPE_CONVERTABLE) {
501+
arg_convertibles[1].reset(reinterpret_cast<v8::ConvertableToTraceFormat*>(
502+
static_cast<intptr_t>(arg_values[1])));
503+
}
504+
node::tracing::TracingController* controller =
505+
node::tracing::TraceEventHelper::GetTracingController();
506+
return controller->AddMetadataEvent(
507+
category_group_enabled, name, num_args, arg_names, arg_types, arg_values,
508+
arg_convertibles, flags);
509+
}
510+
490511
// Define SetTraceValue for each allowed type. It stores the type and
491512
// value in the return arguments. This allows this API to avoid declaring any
492513
// structures so that it is portable to third_party libraries.
@@ -632,23 +653,16 @@ static V8_INLINE uint64_t AddTraceEventWithTimestamp(
632653
}
633654

634655
template <class ARG1_TYPE>
635-
static V8_INLINE uint64_t AddMetadataEvent(
656+
static V8_INLINE void AddMetadataEvent(
636657
const uint8_t* category_group_enabled, const char* name,
637658
const char* arg1_name, ARG1_TYPE&& arg1_val) {
638659
const int num_args = 1;
639660
uint8_t arg_type;
640661
uint64_t arg_value;
641662
SetTraceValue(std::forward<ARG1_TYPE>(arg1_val), &arg_type, &arg_value);
642-
// TODO(ofrobots): It would be good to add metadata events to a separate
643-
// buffer so that they can be periodically reemitted. For now, we have a
644-
// single buffer, so we just add them to the main buffer.
645-
return TRACE_EVENT_API_ADD_TRACE_EVENT(
646-
TRACE_EVENT_PHASE_METADATA,
647-
category_group_enabled, name,
648-
node::tracing::kGlobalScope, // scope
649-
node::tracing::kNoId, // id
650-
node::tracing::kNoId, // bind_id
651-
num_args, &arg1_name, &arg_type, &arg_value, TRACE_EVENT_FLAG_NONE);
663+
AddMetadataEventImpl(
664+
category_group_enabled, name, num_args, &arg1_name, &arg_type, &arg_value,
665+
TRACE_EVENT_FLAG_NONE);
652666
}
653667

654668
// Used by TRACE_EVENTx macros. Do not use directly.

test/cctest/node_test_fixture.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
ArrayBufferUniquePtr NodeTestFixture::allocator{nullptr, nullptr};
44
uv_loop_t NodeTestFixture::current_loop;
55
NodePlatformUniquePtr NodeTestFixture::platform;
6-
TracingControllerUniquePtr NodeTestFixture::tracing_controller;
6+
TracingAgentUniquePtr NodeTestFixture::tracing_agent;

test/cctest/node_test_fixture.h

+4-5
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,20 @@ struct Argv {
5555

5656
using ArrayBufferUniquePtr = std::unique_ptr<node::ArrayBufferAllocator,
5757
decltype(&node::FreeArrayBufferAllocator)>;
58-
using TracingControllerUniquePtr = std::unique_ptr<v8::TracingController>;
58+
using TracingAgentUniquePtr = std::unique_ptr<node::tracing::Agent>;
5959
using NodePlatformUniquePtr = std::unique_ptr<node::NodePlatform>;
6060

6161
class NodeTestFixture : public ::testing::Test {
6262
protected:
6363
static ArrayBufferUniquePtr allocator;
64-
static TracingControllerUniquePtr tracing_controller;
64+
static TracingAgentUniquePtr tracing_agent;
6565
static NodePlatformUniquePtr platform;
6666
static uv_loop_t current_loop;
6767
v8::Isolate* isolate_;
6868

6969
static void SetUpTestCase() {
70-
tracing_controller.reset(new v8::TracingController());
71-
node::tracing::TraceEventHelper::SetTracingController(
72-
tracing_controller.get());
70+
tracing_agent.reset(new node::tracing::Agent());
71+
node::tracing::TraceEventHelper::SetAgent(tracing_agent.get());
7372
CHECK_EQ(0, uv_loop_init(&current_loop));
7473
platform.reset(static_cast<node::NodePlatform*>(
7574
node::InitializeV8Platform(4)));

test/parallel/test-trace-events-dynamic-enable.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,12 @@ function post(message, data) {
2525
async function test() {
2626
session.connect();
2727

28-
let traceNotification = null;
28+
const events = [];
2929
let tracingComplete = false;
30-
session.on('NodeTracing.dataCollected', (n) => traceNotification = n);
30+
session.on('NodeTracing.dataCollected', (n) => {
31+
assert.ok(n && n.data && n.data.value);
32+
events.push(...n.data.value); // append the events.
33+
});
3134
session.on('NodeTracing.tracingComplete', () => tracingComplete = true);
3235

3336
// Generate a node.perf event before tracing is enabled.
@@ -47,10 +50,7 @@ async function test() {
4750
session.disconnect();
4851

4952
assert.ok(tracingComplete);
50-
assert.ok(traceNotification);
51-
assert.ok(traceNotification.data && traceNotification.data.value);
5253

53-
const events = traceNotification.data.value;
5454
const marks = events.filter((t) => null !== /node\.perf\.usertim/.exec(t.cat));
5555
assert.strictEqual(marks.length, 1);
5656
assert.strictEqual(marks[0].name, 'mark2');

0 commit comments

Comments
 (0)