src: remove pushValueToArray and setupProcessObject

lib/internal/bootstrap/node.js

@@ -17,7 +17,7 @@
                               // avoid retaining a reference to the bootstrap
                               // object.
                               { _setupTraceCategoryState,
-                                _setupProcessObject, _setupNextTick,
+                                _setupNextTick,
                                 _setupPromises, _chdir, _cpuUsage,
                                 _hrtime, _hrtimeBigInt,
                                 _memoryUsage, _rawDebug,
@@ -376,13 +376,6 @@
     const origProcProto = Object.getPrototypeOf(process);
     Object.setPrototypeOf(origProcProto, EventEmitter.prototype);
     EventEmitter.call(process);
-
-    _setupProcessObject(pushValueToArray);
-
-    function pushValueToArray() {
-      for (var i = 0; i < arguments.length; i++)
-        this.push(arguments[i]);
-    }
   }
 
   function setupGlobalVariables() {

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() {

src/bootstrapper.cc

@@ -26,12 +26,6 @@ using v8::PromiseRejectMessage;
 using v8::String;
 using v8::Value;
 
-void SetupProcessObject(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  CHECK(args[0]->IsFunction());
-  env->set_push_values_to_array_function(args[0].As<Function>());
-}
-
 void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
   args.GetIsolate()->RunMicrotasks();
 }
@@ -142,7 +136,6 @@ void SetupPromises(const FunctionCallbackInfo<Value>& args) {
 void SetupBootstrapObject(Environment* env,
                           Local<Object> bootstrapper) {
   BOOTSTRAP_METHOD(_setupTraceCategoryState, SetupTraceCategoryState);
-  BOOTSTRAP_METHOD(_setupProcessObject, SetupProcessObject);
   BOOTSTRAP_METHOD(_setupNextTick, SetupNextTick);
   BOOTSTRAP_METHOD(_setupPromises, SetupPromises);
   BOOTSTRAP_METHOD(_chdir, Chdir);

src/env.h

@@ -82,10 +82,6 @@ struct PackageConfig {
 };
 }  // namespace loader
 
-// The number of items passed to push_values_to_array_function has diminishing
-// returns around 8. This should be used at all call sites using said function.
-constexpr size_t NODE_PUSH_VAL_TO_ARRAY_MAX = 8;
-
 // Stat fields buffers contain twice the number of entries in an uv_stat_t
 // because `fs.StatWatcher` needs room to store 2 `fs.Stats` instances.
 constexpr size_t kFsStatsFieldsNumber = 14;
@@ -353,7 +349,6 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
   V(process_object, v8::Object)                                               \
   V(promise_handler_function, v8::Function)                                   \
   V(promise_wrap_template, v8::ObjectTemplate)                                \
-  V(push_values_to_array_function, v8::Function)                              \
   V(sab_lifetimepartner_constructor_template, v8::FunctionTemplate)           \
   V(script_context_constructor_template, v8::FunctionTemplate)                \
   V(script_data_constructor_function, v8::Function)                           \

src/node_http2.cc

@@ -1292,45 +1292,30 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
   Local<String> value_str;
 
   Local<Array> holder = Array::New(isolate);
-  Local<Function> fn = env()->push_values_to_array_function();
-  Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX * 2];
-
   // The headers are passed in above as a queue of nghttp2_header structs.
   // The following converts that into a JS array with the structure:
   // [name1, value1, name2, value2, name3, value3, name3, value4] and so on.
   // That array is passed up to the JS layer and converted into an Object form
   // like {name1: value1, name2: value2, name3: [value3, value4]}. We do it
   // this way for performance reasons (it's faster to generate and pass an
   // array than it is to generate and pass the object).
-  size_t n = 0;
-  while (n < headers.size()) {
-    size_t j = 0;
-    while (n < headers.size() && j < arraysize(argv) / 2) {
-      nghttp2_header item = headers[n++];
-      // The header name and value are passed as external one-byte strings
-      name_str =
-          ExternalHeader::New<true>(this, item.name).ToLocalChecked();
-      value_str =
-          ExternalHeader::New<false>(this, item.value).ToLocalChecked();
-      argv[j * 2] = name_str;
-      argv[j * 2 + 1] = value_str;
-      j++;
-    }
-    // For performance, we pass name and value pairs to array.protototype.push
-    // in batches of size NODE_PUSH_VAL_TO_ARRAY_MAX * 2 until there are no
-    // more items to push.
-    if (j > 0) {
-      fn->Call(env()->context(), holder, j * 2, argv).ToLocalChecked();
-    }
+  size_t headers_size = headers.size();
+  std::vector<Local<Value>> headers_v(headers_size * 2);
+  for (size_t i = 0; i < headers_size; ++i) {
+    const nghttp2_header& item = headers[i];
+    // The header name and value are passed as external one-byte strings
+    headers_v[i * 2] =
+        ExternalHeader::New<true>(this, item.name).ToLocalChecked();
+    headers_v[i * 2 + 1] =
+        ExternalHeader::New<false>(this, item.value).ToLocalChecked();
   }
 
   Local<Value> args[5] = {
-    stream->object(),
-    Integer::New(isolate, id),
-    Integer::New(isolate, stream->headers_category()),
-    Integer::New(isolate, frame->hd.flags),
-    holder
-  };
+      stream->object(),
+      Integer::New(isolate, id),
+      Integer::New(isolate, stream->headers_category()),
+      Integer::New(isolate, frame->hd.flags),
+      Array::New(isolate, headers_v.data(), headers_size * 2)};
   MakeCallback(env()->onheaders_string(), arraysize(args), args);
 }
 
@@ -1439,25 +1424,17 @@ void Http2Session::HandleOriginFrame(const nghttp2_frame* frame) {
   nghttp2_extension ext = frame->ext;
   nghttp2_ext_origin* origin = static_cast<nghttp2_ext_origin*>(ext.payload);
 
-  Local<Value> holder = Array::New(isolate);
-  Local<Function> fn = env()->push_values_to_array_function();
-  Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
+  size_t nov = origin->nov;
+  std::vector<Local<Value>> origin_v(nov);
 
-  size_t n = 0;
-  while (n < origin->nov) {
-    size_t j = 0;
-    while (n < origin->nov && j < arraysize(argv)) {
-      auto entry = origin->ov[n++];
-      argv[j++] =
-        String::NewFromOneByte(isolate,
-                               entry.origin,
-                               v8::NewStringType::kNormal,
-                               entry.origin_len).ToLocalChecked();
-    }
-    if (j > 0)
-      fn->Call(context, holder, j, argv).ToLocalChecked();
+  for (size_t i = 0; i < nov; ++i) {
+    const nghttp2_origin_entry& entry = origin->ov[i];
+    origin_v[i] =
+        String::NewFromOneByte(
+            isolate, entry.origin, v8::NewStringType::kNormal, entry.origin_len)
+            .ToLocalChecked();
   }
-
+  Local<Value> holder = Array::New(isolate, origin_v.data(), origin_v.size());
   MakeCallback(env()->onorigin_string(), 1, &holder);
 }
 

src/node_http_parser.cc

@@ -73,7 +73,8 @@ const uint32_t kOnHeadersComplete = 1;
 const uint32_t kOnBody = 2;
 const uint32_t kOnMessageComplete = 3;
 const uint32_t kOnExecute = 4;
-
+// Any more fields than this will be flushed into JS
+const size_t kMaxHeaderFieldsCount = 32;
 
 // helper class for the Parser
 struct StringPtr {
@@ -203,7 +204,7 @@ class Parser : public AsyncWrap, public StreamListener {
     if (num_fields_ == num_values_) {
       // start of new field name
       num_fields_++;
-      if (num_fields_ == arraysize(fields_)) {
+      if (num_fields_ == kMaxHeaderFieldsCount) {
         // ran out of space - flush to javascript land
         Flush();
         num_fields_ = 1;
@@ -212,7 +213,7 @@ class Parser : public AsyncWrap, public StreamListener {
       fields_[num_fields_ - 1].Reset();
     }
 
-    CHECK_LT(num_fields_, arraysize(fields_));
+    CHECK_LT(num_fields_, kMaxHeaderFieldsCount);
     CHECK_EQ(num_fields_, num_values_ + 1);
 
     fields_[num_fields_ - 1].Update(at, length);
@@ -731,25 +732,15 @@ class Parser : public AsyncWrap, public StreamListener {
   }
 
   Local<Array> CreateHeaders() {
-    Local<Array> headers = Array::New(env()->isolate());
-    Local<Function> fn = env()->push_values_to_array_function();
-    Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX * 2];
-    size_t i = 0;
-
-    do {
-      size_t j = 0;
-      while (i < num_values_ && j < arraysize(argv) / 2) {
-        argv[j * 2] = fields_[i].ToString(env());
-        argv[j * 2 + 1] = values_[i].ToString(env());
-        i++;
-        j++;
-      }
-      if (j > 0) {
-        fn->Call(env()->context(), headers, j * 2, argv).ToLocalChecked();
-      }
-    } while (i < num_values_);
+    // There could be extra entries but the max size should be fixed
+    Local<Value> headers_v[kMaxHeaderFieldsCount * 2];
+
+    for (size_t i = 0; i < num_values_; ++i) {
+      headers_v[i * 2] = fields_[i].ToString(env());
+      headers_v[i * 2 + 1] = values_[i].ToString(env());
+    }
 
-    return headers;
+    return Array::New(env()->isolate(), headers_v, num_values_ * 2);
   }
 
 
@@ -824,8 +815,8 @@ class Parser : public AsyncWrap, public StreamListener {
   }
 
   parser_t parser_;
-  StringPtr fields_[32];  // header fields
-  StringPtr values_[32];  // header values
+  StringPtr fields_[kMaxHeaderFieldsCount];  // header fields
+  StringPtr values_[kMaxHeaderFieldsCount];  // header values
   StringPtr url_;
   StringPtr status_message_;
   size_t num_fields_;

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