From 66767175e8e259ed8340b20680e07fe87d9f3d4d Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 10 Jun 2018 15:12:11 -0700 Subject: [PATCH 1/2] workers,trace_events: set thread name for workers 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 --- src/env-inl.h | 4 +-- src/env.h | 6 ++-- src/node_worker.cc | 15 +++++++-- src/node_worker.h | 2 +- .../test-trace-events-worker-metadata.js | 33 +++++++++++++++++++ 5 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 test/parallel/test-trace-events-worker-metadata.js diff --git a/src/env-inl.h b/src/env-inl.h index c84fdf0bb8215b..bbb80c6f7ae916 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -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; } diff --git a/src/env.h b/src/env.h index 1a96abe106a752..786f0846f4ae1a 100644 --- a/src/env.h +++ b/src/env.h @@ -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); @@ -881,7 +881,7 @@ class Environment { std::unordered_map performance_marks_; bool can_call_into_js_ = true; - double thread_id_ = 0; + uint64_t thread_id_ = 0; std::unordered_set sub_worker_contexts_; diff --git a/src/node_worker.cc b/src/node_worker.cc index 320b6703d40d21..c628e6295c46af 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -9,6 +9,8 @@ #include "async_wrap.h" #include "async_wrap-inl.h" +#include + using v8::ArrayBuffer; using v8::Context; using v8::Function; @@ -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 @@ -44,7 +46,8 @@ Worker::Worker(Environment* env, Local wrap) } wrap->Set(env->context(), env->thread_id_string(), - Number::New(env->isolate(), thread_id_)).FromJust(); + Number::New(env->isolate(), + static_cast(thread_id_))).FromJust(); // Set up everything that needs to be set up in the parent environment. parent_port_ = MessagePort::New(env, env->context()); @@ -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); @@ -418,7 +426,8 @@ void InitWorker(Local 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(env->thread_id()))).FromJust(); } } // anonymous namespace diff --git a/src/node_worker.h b/src/node_worker.h index 0a98d2f11ef00f..d802b5cfefdf77 100644 --- a/src/node_worker.h +++ b/src/node_worker.h @@ -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 child_port_data_; diff --git a/test/parallel/test-trace-events-worker-metadata.js b/test/parallel/test-trace-events-worker-metadata.js new file mode 100644 index 00000000000000..4c9ffea76d9d81 --- /dev/null +++ b/test/parallel/test-trace-events-worker-metadata.js @@ -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.replace(/\\/g,'/')}')`; + 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. +} From af20dd0a6d024061c8b1d88c0e08d2f95994d545 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 18 Jun 2018 14:18:36 -0700 Subject: [PATCH 2/2] squash! add a space to appease the linter --- test/parallel/test-trace-events-worker-metadata.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-trace-events-worker-metadata.js b/test/parallel/test-trace-events-worker-metadata.js index 4c9ffea76d9d81..8d21d6b7ab9e0b 100644 --- a/test/parallel/test-trace-events-worker-metadata.js +++ b/test/parallel/test-trace-events-worker-metadata.js @@ -8,7 +8,7 @@ const { isMainThread } = require('worker_threads'); if (isMainThread) { const CODE = 'const { Worker } = require(\'worker_threads\'); ' + - `new Worker('${__filename.replace(/\\/g,'/')}')`; + `new Worker('${__filename.replace(/\\/g, '/')}')`; const FILE_NAME = 'node_trace.1.log'; const tmpdir = require('../common/tmpdir'); tmpdir.refresh();