Skip to content

Commit

Permalink
workers,trace_events: set thread name for workers
Browse files Browse the repository at this point in the history
Set the thread name for workers in trace events. Also,
use uint64_t for thread_id_ because there's really no
reason for those to be doubles

PR-URL: nodejs#21246
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
  • Loading branch information
jasnell committed Jun 15, 2018
1 parent b55f6a0 commit a24b691
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 9 deletions.
4 changes: 2 additions & 2 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -598,11 +598,11 @@ inline bool Environment::is_main_thread() const {
return thread_id_ == 0;
}

inline double Environment::thread_id() const {
inline uint64_t Environment::thread_id() const {
return thread_id_;
}

inline void Environment::set_thread_id(double id) {
inline void Environment::set_thread_id(uint64_t id) {
thread_id_ = id;
}

Expand Down
6 changes: 3 additions & 3 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -726,8 +726,8 @@ class Environment {
bool is_stopping_worker() const;

inline bool is_main_thread() const;
inline double thread_id() const;
inline void set_thread_id(double id);
inline uint64_t thread_id() const;
inline void set_thread_id(uint64_t id);
inline worker::Worker* worker_context() const;
inline void set_worker_context(worker::Worker* context);
inline void add_sub_worker_context(worker::Worker* context);
Expand Down Expand Up @@ -881,7 +881,7 @@ class Environment {
std::unordered_map<std::string, uint64_t> performance_marks_;

bool can_call_into_js_ = true;
double thread_id_ = 0;
uint64_t thread_id_ = 0;
std::unordered_set<worker::Worker*> sub_worker_contexts_;


Expand Down
15 changes: 12 additions & 3 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "async_wrap.h"
#include "async_wrap-inl.h"

#include <string>

using v8::ArrayBuffer;
using v8::Context;
using v8::Function;
Expand All @@ -30,7 +32,7 @@ namespace worker {

namespace {

double next_thread_id = 1;
uint64_t next_thread_id = 1;
Mutex next_thread_id_mutex;

} // anonymous namespace
Expand All @@ -44,7 +46,8 @@ Worker::Worker(Environment* env, Local<Object> wrap)
}
wrap->Set(env->context(),
env->thread_id_string(),
Number::New(env->isolate(), thread_id_)).FromJust();
Number::New(env->isolate(),
static_cast<double>(thread_id_))).FromJust();

// Set up everything that needs to be set up in the parent environment.
parent_port_ = MessagePort::New(env, env->context());
Expand Down Expand Up @@ -112,6 +115,11 @@ bool Worker::is_stopped() const {
}

void Worker::Run() {
std::string name = "WorkerThread ";
name += std::to_string(thread_id_);
TRACE_EVENT_METADATA1(
"__metadata", "thread_name", "name",
TRACE_STR_COPY(name.c_str()));
MultiIsolatePlatform* platform = isolate_data_->platform();
CHECK_NE(platform, nullptr);

Expand Down Expand Up @@ -418,7 +426,8 @@ void InitWorker(Local<Object> target,
auto thread_id_string = FIXED_ONE_BYTE_STRING(env->isolate(), "threadId");
target->Set(env->context(),
thread_id_string,
Number::New(env->isolate(), env->thread_id())).FromJust();
Number::New(env->isolate(),
static_cast<double>(env->thread_id()))).FromJust();
}

} // anonymous namespace
Expand Down
2 changes: 1 addition & 1 deletion src/node_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class Worker : public AsyncWrap {

bool thread_joined_ = true;
int exit_code_ = 0;
double thread_id_ = -1;
uint64_t thread_id_ = -1;

std::unique_ptr<MessagePortData> child_port_data_;

Expand Down
33 changes: 33 additions & 0 deletions test/parallel/test-trace-events-worker-metadata.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Flags: --experimental-worker
'use strict';
const common = require('../common');
const assert = require('assert');
const cp = require('child_process');
const fs = require('fs');
const { isMainThread } = require('worker_threads');

if (isMainThread) {
const CODE = 'const { Worker } = require(\'worker_threads\'); ' +
`new Worker('${__filename}')`;
const FILE_NAME = 'node_trace.1.log';
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
process.chdir(tmpdir.path);

const proc = cp.spawn(process.execPath,
[ '--experimental-worker',
'--trace-event-categories', 'node',
'-e', CODE ]);
proc.once('exit', common.mustCall(() => {
assert(common.fileExists(FILE_NAME));
fs.readFile(FILE_NAME, common.mustCall((err, data) => {
const traces = JSON.parse(data.toString()).traceEvents;
assert(traces.length > 0);
assert(traces.some((trace) =>
trace.cat === '__metadata' && trace.name === 'thread_name' &&
trace.args.name === 'WorkerThread 1'));
}));
}));
} else {
// Do nothing here.
}

0 comments on commit a24b691

Please sign in to comment.