Skip to content

Commit

Permalink
os: do not call into JS to push values to an array in GetCPUInfo
Browse files Browse the repository at this point in the history
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: #24264
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
  • Loading branch information
joyeecheung authored and BridgeAR committed Nov 13, 2018
1 parent ccc3bb7 commit 293983a
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 60 deletions.
32 changes: 15 additions & 17 deletions lib/os.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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() {
Expand Down
54 changes: 18 additions & 36 deletions src/node_os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -148,52 +149,33 @@ static void GetOSRelease(const FunctionCallbackInfo<Value>& args) {

static void GetCPUInfo(const FunctionCallbackInfo<Value>& 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<Function> addfn = args[0].As<Function>();

CHECK(args[1]->IsFloat64Array());
Local<Float64Array> array = args[1].As<Float64Array>();
CHECK_EQ(array->Length(), 6 * NODE_PUSH_VAL_TO_ARRAY_MAX);
Local<ArrayBuffer> ab = array->Buffer();
double* fields = static_cast<double*>(ab->GetContents().Data());

CHECK(args[2]->IsArray());
Local<Array> cpus = args[2].As<Array>();

Local<Value> model_argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
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<Local<Value>> 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()));
}


Expand Down
7 changes: 0 additions & 7 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -206,12 +205,6 @@ void Initialize(Local<Object> 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), \
Expand Down
9 changes: 9 additions & 0 deletions test/parallel/test-os.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 293983a

Please sign in to comment.