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 <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>

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],
-      times: {
-        user: cpuValues[c + 1],
-        nice: cpuValues[c + 2],
-        sys: cpuValues[c + 3],
-        idle: cpuValues[c + 4],
-        irq: cpuValues[c + 5]
-      }
-    };
-  }
-}
-
 function cpus() {
-  return getCPUs(addCPUInfo, cpuValues, []);
+  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: data[i + 2],
+        nice: data[i + 3],
+        sys: data[i + 4],
+        idle: data[i + 5],
+        irq: data[i + 6]
+      }
+    });
+  }
+  return result;
 }
 
 function arch() {

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<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];
-  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<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()));
 }
 
 

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<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),                   \

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);