From 11604c73958a33124373e6c9f84f1962f8b1d44f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 9 Nov 2018 12:22:30 +0800 Subject: [PATCH] os: do not call into JS to push values to an array in GetCPUInfo Instead of calling into JS from C++ to push values into an array, use the new Array::New API that takes a pointer and a length directly. PR-URL: https://github.com/nodejs/node/pull/24264 Reviewed-By: Anna Henningsen Reviewed-By: Daniel Bevenius Reviewed-By: Refael Ackermann --- lib/os.js | 32 +++++++++++------------- src/node_os.cc | 54 ++++++++++++++-------------------------- src/node_util.cc | 7 ------ test/parallel/test-os.js | 9 +++++++ 4 files changed, 42 insertions(+), 60 deletions(-) diff --git a/lib/os.js b/lib/os.js index 797201c4b5e732..2c806908eeac98 100644 --- a/lib/os.js +++ b/lib/os.js @@ -21,7 +21,7 @@ 'use strict'; -const { pushValToArrayMax, safeGetenv } = internalBinding('util'); +const { safeGetenv } = internalBinding('util'); const constants = internalBinding('constants').os; const { deprecate } = require('internal/util'); const isWindows = process.platform === 'win32'; @@ -82,31 +82,29 @@ const getNetworkInterfacesDepMsg = 'os.getNetworkInterfaces is deprecated. Use os.networkInterfaces instead.'; const avgValues = new Float64Array(3); -const cpuValues = new Float64Array(6 * pushValToArrayMax); function loadavg() { getLoadAvg(avgValues); return [avgValues[0], avgValues[1], avgValues[2]]; } -function addCPUInfo() { - for (var i = 0, c = 0; i < arguments.length; ++i, c += 6) { - this[this.length] = { - model: arguments[i], - speed: cpuValues[c], +function cpus() { + const data = getCPUs(); + const result = []; + for (var i = 0; i < data.length; i += 7) { + result.push({ + model: data[i], + speed: data[i + 1], times: { - user: cpuValues[c + 1], - nice: cpuValues[c + 2], - sys: cpuValues[c + 3], - idle: cpuValues[c + 4], - irq: cpuValues[c + 5] + user: data[i + 2], + nice: data[i + 3], + sys: data[i + 4], + idle: data[i + 5], + irq: data[i + 6] } - }; + }); } -} - -function cpus() { - return getCPUs(addCPUInfo, cpuValues, []); + return result; } function arch() { diff --git a/src/node_os.cc b/src/node_os.cc index 7d058fcc9a2036..0b46af95f4ba78 100644 --- a/src/node_os.cc +++ b/src/node_os.cc @@ -54,6 +54,7 @@ using v8::Function; using v8::FunctionCallbackInfo; using v8::Int32; using v8::Integer; +using v8::Isolate; using v8::Local; using v8::MaybeLocal; using v8::Null; @@ -148,52 +149,33 @@ static void GetOSRelease(const FunctionCallbackInfo& args) { static void GetCPUInfo(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + uv_cpu_info_t* cpu_infos; - int count, i, field_idx; + int count; int err = uv_cpu_info(&cpu_infos, &count); if (err) return; - CHECK(args[0]->IsFunction()); - Local addfn = args[0].As(); - - CHECK(args[1]->IsFloat64Array()); - Local array = args[1].As(); - CHECK_EQ(array->Length(), 6 * NODE_PUSH_VAL_TO_ARRAY_MAX); - Local ab = array->Buffer(); - double* fields = static_cast(ab->GetContents().Data()); - - CHECK(args[2]->IsArray()); - Local cpus = args[2].As(); - - Local model_argv[NODE_PUSH_VAL_TO_ARRAY_MAX]; - unsigned int model_idx = 0; - - for (i = 0, field_idx = 0; i < count; i++) { + // It's faster to create an array packed with all the data and + // assemble them into objects in JS than to call Object::Set() repeatedly + // The array is in the format + // [model, speed, (5 entries of cpu_times), model2, speed2, ...] + std::vector> result(count * 7); + for (size_t i = 0; i < count; i++) { uv_cpu_info_t* ci = cpu_infos + i; - - fields[field_idx++] = ci->speed; - fields[field_idx++] = ci->cpu_times.user; - fields[field_idx++] = ci->cpu_times.nice; - fields[field_idx++] = ci->cpu_times.sys; - fields[field_idx++] = ci->cpu_times.idle; - fields[field_idx++] = ci->cpu_times.irq; - model_argv[model_idx++] = OneByteString(env->isolate(), ci->model); - - if (model_idx >= NODE_PUSH_VAL_TO_ARRAY_MAX) { - addfn->Call(env->context(), cpus, model_idx, model_argv).ToLocalChecked(); - model_idx = 0; - field_idx = 0; - } - } - - if (model_idx > 0) { - addfn->Call(env->context(), cpus, model_idx, model_argv).ToLocalChecked(); + result[i * 7] = OneByteString(isolate, ci->model); + result[i * 7 + 1] = Number::New(isolate, ci->speed); + result[i * 7 + 2] = Number::New(isolate, ci->cpu_times.user); + result[i * 7 + 3] = Number::New(isolate, ci->cpu_times.nice); + result[i * 7 + 4] = Number::New(isolate, ci->cpu_times.sys); + result[i * 7 + 5] = Number::New(isolate, ci->cpu_times.idle); + result[i * 7 + 6] = Number::New(isolate, ci->cpu_times.irq); } uv_free_cpu_info(cpu_infos, count); - args.GetReturnValue().Set(cpus); + args.GetReturnValue().Set(Array::New(isolate, result.data(), result.size())); } diff --git a/src/node_util.cc b/src/node_util.cc index 62af7a1115784e..1a08c0e255f8d3 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -24,7 +24,6 @@ using v8::Private; using v8::Promise; using v8::PropertyFilter; using v8::Proxy; -using v8::ReadOnly; using v8::SKIP_STRINGS; using v8::SKIP_SYMBOLS; using v8::String; @@ -206,12 +205,6 @@ void Initialize(Local target, } #undef V - target->DefineOwnProperty( - env->context(), - OneByteString(env->isolate(), "pushValToArrayMax"), - Integer::NewFromUnsigned(env->isolate(), NODE_PUSH_VAL_TO_ARRAY_MAX), - ReadOnly).FromJust(); - #define V(name) \ target->Set(context, \ FIXED_ONE_BYTE_STRING(env->isolate(), #name), \ diff --git a/test/parallel/test-os.js b/test/parallel/test-os.js index 3591ee52104ea4..9b86d3af8b263e 100644 --- a/test/parallel/test-os.js +++ b/test/parallel/test-os.js @@ -92,6 +92,15 @@ assert.ok(uptime > 0); const cpus = os.cpus(); is.array(cpus); assert.ok(cpus.length > 0); +for (const cpu of cpus) { + assert.strictEqual(typeof cpu.model, 'string'); + assert.strictEqual(typeof cpu.speed, 'number'); + assert.strictEqual(typeof cpu.times.user, 'number'); + assert.strictEqual(typeof cpu.times.nice, 'number'); + assert.strictEqual(typeof cpu.times.sys, 'number'); + assert.strictEqual(typeof cpu.times.idle, 'number'); + assert.strictEqual(typeof cpu.times.irq, 'number'); +} const type = os.type(); is.string(type);