diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index a0cf67b9ead671..85bfc688b391c0 100644 --- a/lib/internal/bootstrap/node.js +++ b/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() { 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/bootstrapper.cc b/src/bootstrapper.cc index d6a8d3854ad6ff..c40b855b9939ca 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -26,12 +26,6 @@ using v8::PromiseRejectMessage; using v8::String; using v8::Value; -void SetupProcessObject(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - CHECK(args[0]->IsFunction()); - env->set_push_values_to_array_function(args[0].As()); -} - void RunMicrotasks(const FunctionCallbackInfo& args) { args.GetIsolate()->RunMicrotasks(); } @@ -142,7 +136,6 @@ void SetupPromises(const FunctionCallbackInfo& args) { void SetupBootstrapObject(Environment* env, Local bootstrapper) { BOOTSTRAP_METHOD(_setupTraceCategoryState, SetupTraceCategoryState); - BOOTSTRAP_METHOD(_setupProcessObject, SetupProcessObject); BOOTSTRAP_METHOD(_setupNextTick, SetupNextTick); BOOTSTRAP_METHOD(_setupPromises, SetupPromises); BOOTSTRAP_METHOD(_chdir, Chdir); diff --git a/src/env.h b/src/env.h index aec2f8fde40213..01f686833eab24 100644 --- a/src/env.h +++ b/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) \ diff --git a/src/node_http2.cc b/src/node_http2.cc index 8ae58d5134e15d..20b634f90fc00d 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1292,9 +1292,6 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) { Local value_str; Local holder = Array::New(isolate); - Local fn = env()->push_values_to_array_function(); - Local 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. @@ -1302,35 +1299,23 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) { // 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(this, item.name).ToLocalChecked(); - value_str = - ExternalHeader::New(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> 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(this, item.name).ToLocalChecked(); + headers_v[i * 2 + 1] = + ExternalHeader::New(this, item.value).ToLocalChecked(); } Local 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(ext.payload); - Local holder = Array::New(isolate); - Local fn = env()->push_values_to_array_function(); - Local argv[NODE_PUSH_VAL_TO_ARRAY_MAX]; + size_t nov = origin->nov; + std::vector> 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 holder = Array::New(isolate, origin_v.data(), origin_v.size()); MakeCallback(env()->onorigin_string(), 1, &holder); } diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index c9e057711d9253..f752a003a25d6d 100644 --- a/src/node_http_parser.cc +++ b/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 CreateHeaders() { - Local headers = Array::New(env()->isolate()); - Local fn = env()->push_values_to_array_function(); - Local 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 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_; 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_process.cc b/src/node_process.cc index 6c12dcc02b2cd0..161833ebbd3843 100644 --- a/src/node_process.cc +++ b/src/node_process.cc @@ -730,40 +730,29 @@ void EnvDeleter(Local property, void EnvEnumerator(const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); Isolate* isolate = env->isolate(); - Local ctx = env->context(); - Local fn = env->push_values_to_array_function(); - Local argv[NODE_PUSH_VAL_TO_ARRAY_MAX]; - size_t idx = 0; Mutex::ScopedLock lock(environ_mutex); + Local envarr; + int env_size = 0; #ifdef __POSIX__ - int size = 0; - while (environ[size]) - size++; - - Local envarr = Array::New(isolate); + while (environ[env_size]) { + env_size++; + } + std::vector> env_v(env_size); - for (int i = 0; i < size; ++i) { + for (int i = 0; i < env_size; ++i) { const char* var = environ[i]; const char* s = strchr(var, '='); const int length = s ? s - var : strlen(var); - argv[idx] = String::NewFromUtf8(isolate, - var, - v8::NewStringType::kNormal, - length).ToLocalChecked(); - if (++idx >= arraysize(argv)) { - fn->Call(ctx, envarr, idx, argv).ToLocalChecked(); - idx = 0; - } - } - if (idx > 0) { - fn->Call(ctx, envarr, idx, argv).ToLocalChecked(); + env_v[i] = + String::NewFromUtf8(isolate, var, v8::NewStringType::kNormal, length) + .ToLocalChecked(); } #else // _WIN32 + std::vector> env_v; WCHAR* environment = GetEnvironmentStringsW(); if (environment == nullptr) return; // This should not happen. - Local envarr = Array::New(isolate); WCHAR* p = environment; while (*p) { WCHAR* s; @@ -789,19 +778,13 @@ void EnvEnumerator(const PropertyCallbackInfo& info) { FreeEnvironmentStringsW(environment); return; } - argv[idx] = rc.ToLocalChecked(); - if (++idx >= arraysize(argv)) { - fn->Call(ctx, envarr, idx, argv).ToLocalChecked(); - idx = 0; - } + env_v.push_back(rc.ToLocalChecked()); p = s + wcslen(s) + 1; } - if (idx > 0) { - fn->Call(ctx, envarr, idx, argv).ToLocalChecked(); - } FreeEnvironmentStringsW(environment); #endif + envarr = Array::New(isolate, env_v.data(), env_v.size()); info.GetReturnValue().Set(envarr); } @@ -814,54 +797,30 @@ void GetActiveRequests(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Local ary = Array::New(args.GetIsolate()); - Local ctx = env->context(); - Local fn = env->push_values_to_array_function(); - Local argv[NODE_PUSH_VAL_TO_ARRAY_MAX]; - size_t idx = 0; - + std::vector> request_v; for (auto w : *env->req_wrap_queue()) { if (w->persistent().IsEmpty()) continue; - argv[idx] = w->GetOwner(); - if (++idx >= arraysize(argv)) { - fn->Call(ctx, ary, idx, argv).ToLocalChecked(); - idx = 0; - } + request_v.push_back(w->GetOwner()); } - if (idx > 0) { - fn->Call(ctx, ary, idx, argv).ToLocalChecked(); - } - - args.GetReturnValue().Set(ary); + args.GetReturnValue().Set( + Array::New(env->isolate(), request_v.data(), request_v.size())); } - // Non-static, friend of HandleWrap. Could have been a HandleWrap method but // implemented here for consistency with GetActiveRequests(). void GetActiveHandles(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - Local ary = Array::New(env->isolate()); - Local ctx = env->context(); - Local fn = env->push_values_to_array_function(); - Local argv[NODE_PUSH_VAL_TO_ARRAY_MAX]; - size_t idx = 0; - + std::vector> handle_v; for (auto w : *env->handle_wrap_queue()) { if (!HandleWrap::HasRef(w)) continue; - argv[idx] = w->GetOwner(); - if (++idx >= arraysize(argv)) { - fn->Call(ctx, ary, idx, argv).ToLocalChecked(); - idx = 0; - } - } - if (idx > 0) { - fn->Call(ctx, ary, idx, argv).ToLocalChecked(); + handle_v.push_back(w->GetOwner()); } - - args.GetReturnValue().Set(ary); + args.GetReturnValue().Set( + Array::New(env->isolate(), handle_v.data(), handle_v.size())); } void DebugPortGetter(Local property, 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); diff --git a/test/parallel/test-process-env.js b/test/parallel/test-process-env.js index 69379f60061985..7f953c12eecb66 100644 --- a/test/parallel/test-process-env.js +++ b/test/parallel/test-process-env.js @@ -91,6 +91,7 @@ assert.strictEqual(5, date.getHours()); // case-sensitive on other platforms. process.env.TEST = 'test'; assert.strictEqual(process.env.TEST, 'test'); + // Check both mixed case and lower case, to avoid any regressions that might // simply convert input to lower case. if (common.isWindows) { @@ -100,3 +101,8 @@ if (common.isWindows) { assert.strictEqual(process.env.test, undefined); assert.strictEqual(process.env.teST, undefined); } + +{ + const keys = Object.keys(process.env); + assert.ok(keys.length > 0); +}