Skip to content

Commit e6debf5

Browse files
addaleaxcodebytere
addaleax
authored and
codebytere
committed
src: prefer 3-argument Array::New()
This is nicer, because: 1. It reduces overall code size, 2. It’s faster, because `Object::Set()` calls are relatively slow, and 3. It helps avoid invalid `.Check()`/`.FromJust()` calls. PR-URL: #31775 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
1 parent de275d0 commit e6debf5

File tree

7 files changed

+85
-103
lines changed

7 files changed

+85
-103
lines changed

‎src/cares_wrap.cc‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -751,16 +751,12 @@ Local<Array> AddrTTLToArray(Environment* env,
751751
const T* addrttls,
752752
size_t naddrttls) {
753753
auto isolate = env->isolate();
754-
EscapableHandleScope escapable_handle_scope(isolate);
755-
auto context = env->context();
756754

757-
Local<Array> ttls = Array::New(isolate, naddrttls);
758-
for (size_t i = 0; i < naddrttls; i++) {
759-
auto value = Integer::NewFromUnsigned(isolate, addrttls[i].ttl);
760-
ttls->Set(context, i, value).Check();
761-
}
755+
MaybeStackBuffer<Local<Value>, 8> ttls(naddrttls);
756+
for (size_t i = 0; i < naddrttls; i++)
757+
ttls[i] = Integer::NewFromUnsigned(isolate, addrttls[i].ttl);
762758

763-
return escapable_handle_scope.Escape(ttls);
759+
return Array::New(isolate, ttls.out(), naddrttls);
764760
}
765761

766762

@@ -2041,6 +2037,7 @@ void GetServers(const FunctionCallbackInfo<Value>& args) {
20412037

20422038
int r = ares_get_servers_ports(channel->cares_channel(), &servers);
20432039
CHECK_EQ(r, ARES_SUCCESS);
2040+
auto cleanup = OnScopeLeave([&]() { ares_free_data(servers); });
20442041

20452042
ares_addr_port_node* cur = servers;
20462043

@@ -2051,17 +2048,18 @@ void GetServers(const FunctionCallbackInfo<Value>& args) {
20512048
int err = uv_inet_ntop(cur->family, caddr, ip, sizeof(ip));
20522049
CHECK_EQ(err, 0);
20532050

2054-
Local<Array> ret = Array::New(env->isolate(), 2);
2055-
ret->Set(env->context(), 0, OneByteString(env->isolate(), ip)).Check();
2056-
ret->Set(env->context(),
2057-
1,
2058-
Integer::New(env->isolate(), cur->udp_port)).Check();
2051+
Local<Value> ret[] = {
2052+
OneByteString(env->isolate(), ip),
2053+
Integer::New(env->isolate(), cur->udp_port)
2054+
};
20592055

2060-
server_array->Set(env->context(), i, ret).Check();
2056+
if (server_array->Set(env->context(), i,
2057+
Array::New(env->isolate(), ret, arraysize(ret)))
2058+
.IsNothing()) {
2059+
return;
2060+
}
20612061
}
20622062

2063-
ares_free_data(servers);
2064-
20652063
args.GetReturnValue().Set(server_array);
20662064
}
20672065

‎src/js_stream.cc‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,16 +116,15 @@ int JSStream::DoWrite(WriteWrap* w,
116116
HandleScope scope(env()->isolate());
117117
Context::Scope context_scope(env()->context());
118118

119-
Local<Array> bufs_arr = Array::New(env()->isolate(), count);
120-
Local<Object> buf;
119+
MaybeStackBuffer<Local<Value>, 16> bufs_arr(count);
121120
for (size_t i = 0; i < count; i++) {
122-
buf = Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocalChecked();
123-
bufs_arr->Set(env()->context(), i, buf).Check();
121+
bufs_arr[i] =
122+
Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocalChecked();
124123
}
125124

126125
Local<Value> argv[] = {
127126
w->object(),
128-
bufs_arr
127+
Array::New(env()->isolate(), bufs_arr.out(), count)
129128
};
130129

131130
TryCatchScope try_catch(env());

‎src/module_wrap.cc‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -236,11 +236,11 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
236236
Local<Context> mod_context = obj->context_.Get(isolate);
237237
Local<Module> module = obj->module_.Get(isolate);
238238

239-
Local<Array> promises = Array::New(isolate,
240-
module->GetModuleRequestsLength());
239+
const int module_requests_length = module->GetModuleRequestsLength();
240+
MaybeStackBuffer<Local<Value>, 16> promises(module_requests_length);
241241

242242
// call the dependency resolve callbacks
243-
for (int i = 0; i < module->GetModuleRequestsLength(); i++) {
243+
for (int i = 0; i < module_requests_length; i++) {
244244
Local<String> specifier = module->GetModuleRequest(i);
245245
Utf8Value specifier_utf8(env->isolate(), specifier);
246246
std::string specifier_std(*specifier_utf8, specifier_utf8.length());
@@ -262,10 +262,11 @@ void ModuleWrap::Link(const FunctionCallbackInfo<Value>& args) {
262262
Local<Promise> resolve_promise = resolve_return_value.As<Promise>();
263263
obj->resolve_cache_[specifier_std].Reset(env->isolate(), resolve_promise);
264264

265-
promises->Set(mod_context, i, resolve_promise).Check();
265+
promises[i] = resolve_promise;
266266
}
267267

268-
args.GetReturnValue().Set(promises);
268+
args.GetReturnValue().Set(
269+
Array::New(isolate, promises.out(), promises.length()));
269270
}
270271

271272
void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
@@ -398,12 +399,13 @@ void ModuleWrap::GetStaticDependencySpecifiers(
398399

399400
int count = module->GetModuleRequestsLength();
400401

401-
Local<Array> specifiers = Array::New(env->isolate(), count);
402+
MaybeStackBuffer<Local<Value>, 16> specifiers(count);
402403

403404
for (int i = 0; i < count; i++)
404-
specifiers->Set(env->context(), i, module->GetModuleRequest(i)).Check();
405+
specifiers[i] = module->GetModuleRequest(i);
405406

406-
args.GetReturnValue().Set(specifiers);
407+
args.GetReturnValue().Set(
408+
Array::New(env->isolate(), specifiers.out(), count));
407409
}
408410

409411
void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {

‎src/node_crypto.cc‎

Lines changed: 34 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,19 +1011,19 @@ static X509_STORE* NewRootCertStore() {
10111011

10121012
void GetRootCertificates(const FunctionCallbackInfo<Value>& args) {
10131013
Environment* env = Environment::GetCurrent(args);
1014-
Local<Array> result = Array::New(env->isolate(), arraysize(root_certs));
1014+
Local<Value> result[arraysize(root_certs)];
10151015

10161016
for (size_t i = 0; i < arraysize(root_certs); i++) {
1017-
Local<Value> value;
1018-
if (!String::NewFromOneByte(env->isolate(),
1019-
reinterpret_cast<const uint8_t*>(root_certs[i]),
1020-
NewStringType::kNormal).ToLocal(&value) ||
1021-
!result->Set(env->context(), i, value).FromMaybe(false)) {
1017+
if (!String::NewFromOneByte(
1018+
env->isolate(),
1019+
reinterpret_cast<const uint8_t*>(root_certs[i]),
1020+
NewStringType::kNormal).ToLocal(&result[i])) {
10221021
return;
10231022
}
10241023
}
10251024

1026-
args.GetReturnValue().Set(result);
1025+
args.GetReturnValue().Set(
1026+
Array::New(env->isolate(), result, arraysize(root_certs)));
10271027
}
10281028

10291029

@@ -2138,22 +2138,22 @@ static Local<Object> X509ToObject(Environment* env, X509* cert) {
21382138
StackOfASN1 eku(static_cast<STACK_OF(ASN1_OBJECT)*>(
21392139
X509_get_ext_d2i(cert, NID_ext_key_usage, nullptr, nullptr)));
21402140
if (eku) {
2141-
Local<Array> ext_key_usage = Array::New(env->isolate());
2141+
const int count = sk_ASN1_OBJECT_num(eku.get());
2142+
MaybeStackBuffer<Local<Value>, 16> ext_key_usage(count);
21422143
char buf[256];
21432144

21442145
int j = 0;
2145-
for (int i = 0; i < sk_ASN1_OBJECT_num(eku.get()); i++) {
2146+
for (int i = 0; i < count; i++) {
21462147
if (OBJ_obj2txt(buf,
21472148
sizeof(buf),
21482149
sk_ASN1_OBJECT_value(eku.get(), i), 1) >= 0) {
2149-
ext_key_usage->Set(context,
2150-
j++,
2151-
OneByteString(env->isolate(), buf)).Check();
2150+
ext_key_usage[j++] = OneByteString(env->isolate(), buf);
21522151
}
21532152
}
21542153

21552154
eku.reset();
2156-
info->Set(context, env->ext_key_usage_string(), ext_key_usage).Check();
2155+
info->Set(context, env->ext_key_usage_string(),
2156+
Array::New(env->isolate(), ext_key_usage.out(), count)).Check();
21572157
}
21582158

21592159
if (ASN1_INTEGER* serial_number = X509_get_serialNumber(cert)) {
@@ -6729,15 +6729,8 @@ void GenerateKeyPair(const FunctionCallbackInfo<Value>& args,
67296729
Local<Value> err, pubkey, privkey;
67306730
job->ToResult(&err, &pubkey, &privkey);
67316731

6732-
bool (*IsNotTrue)(Maybe<bool>) = [](Maybe<bool> maybe) {
6733-
return maybe.IsNothing() || !maybe.ToChecked();
6734-
};
6735-
Local<Array> ret = Array::New(env->isolate(), 3);
6736-
if (IsNotTrue(ret->Set(env->context(), 0, err)) ||
6737-
IsNotTrue(ret->Set(env->context(), 1, pubkey)) ||
6738-
IsNotTrue(ret->Set(env->context(), 2, privkey)))
6739-
return;
6740-
args.GetReturnValue().Set(ret);
6732+
Local<Value> ret[] = { err, pubkey, privkey };
6733+
args.GetReturnValue().Set(Array::New(env->isolate(), ret, arraysize(ret)));
67416734
}
67426735

67436736
void GenerateKeyPairRSA(const FunctionCallbackInfo<Value>& args) {
@@ -6837,17 +6830,6 @@ void GetSSLCiphers(const FunctionCallbackInfo<Value>& args) {
68376830
CHECK(ssl);
68386831

68396832
STACK_OF(SSL_CIPHER)* ciphers = SSL_get_ciphers(ssl.get());
6840-
int n = sk_SSL_CIPHER_num(ciphers);
6841-
Local<Array> arr = Array::New(env->isolate(), n);
6842-
6843-
for (int i = 0; i < n; ++i) {
6844-
const SSL_CIPHER* cipher = sk_SSL_CIPHER_value(ciphers, i);
6845-
arr->Set(env->context(),
6846-
i,
6847-
OneByteString(args.GetIsolate(),
6848-
SSL_CIPHER_get_name(cipher))).Check();
6849-
}
6850-
68516833
// TLSv1.3 ciphers aren't listed by EVP. There are only 5, we could just
68526834
// document them, but since there are only 5, easier to just add them manually
68536835
// and not have to explain their absence in the API docs. They are lower-cased
@@ -6860,13 +6842,20 @@ void GetSSLCiphers(const FunctionCallbackInfo<Value>& args) {
68606842
"tls_aes_128_ccm_sha256"
68616843
};
68626844

6845+
const int n = sk_SSL_CIPHER_num(ciphers);
6846+
std::vector<Local<Value>> arr(n + arraysize(TLS13_CIPHERS));
6847+
6848+
for (int i = 0; i < n; ++i) {
6849+
const SSL_CIPHER* cipher = sk_SSL_CIPHER_value(ciphers, i);
6850+
arr[i] = OneByteString(env->isolate(), SSL_CIPHER_get_name(cipher));
6851+
}
6852+
68636853
for (unsigned i = 0; i < arraysize(TLS13_CIPHERS); ++i) {
68646854
const char* name = TLS13_CIPHERS[i];
6865-
arr->Set(env->context(),
6866-
arr->Length(), OneByteString(args.GetIsolate(), name)).Check();
6855+
arr[n + i] = OneByteString(env->isolate(), name);
68676856
}
68686857

6869-
args.GetReturnValue().Set(arr);
6858+
args.GetReturnValue().Set(Array::New(env->isolate(), arr.data(), arr.size()));
68706859
}
68716860

68726861

@@ -6917,22 +6906,23 @@ void GetHashes(const FunctionCallbackInfo<Value>& args) {
69176906
void GetCurves(const FunctionCallbackInfo<Value>& args) {
69186907
Environment* env = Environment::GetCurrent(args);
69196908
const size_t num_curves = EC_get_builtin_curves(nullptr, 0);
6920-
Local<Array> arr = Array::New(env->isolate(), num_curves);
69216909

69226910
if (num_curves) {
69236911
std::vector<EC_builtin_curve> curves(num_curves);
69246912

69256913
if (EC_get_builtin_curves(curves.data(), num_curves)) {
6926-
for (size_t i = 0; i < num_curves; i++) {
6927-
arr->Set(env->context(),
6928-
i,
6929-
OneByteString(env->isolate(),
6930-
OBJ_nid2sn(curves[i].nid))).Check();
6931-
}
6914+
std::vector<Local<Value>> arr(num_curves);
6915+
6916+
for (size_t i = 0; i < num_curves; i++)
6917+
arr[i] = OneByteString(env->isolate(), OBJ_nid2sn(curves[i].nid));
6918+
6919+
args.GetReturnValue().Set(
6920+
Array::New(env->isolate(), arr.data(), arr.size()));
6921+
return;
69326922
}
69336923
}
69346924

6935-
args.GetReturnValue().Set(arr);
6925+
args.GetReturnValue().Set(Array::New(env->isolate()));
69366926
}
69376927

69386928

‎src/node_file.cc‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -700,16 +700,11 @@ void AfterScanDirWithTypes(uv_fs_t* req) {
700700
type_v.emplace_back(Integer::New(isolate, ent.type));
701701
}
702702

703-
Local<Array> result = Array::New(isolate, 2);
704-
result->Set(env->context(),
705-
0,
706-
Array::New(isolate, name_v.data(),
707-
name_v.size())).Check();
708-
result->Set(env->context(),
709-
1,
710-
Array::New(isolate, type_v.data(),
711-
type_v.size())).Check();
712-
req_wrap->Resolve(result);
703+
Local<Value> result[] = {
704+
Array::New(isolate, name_v.data(), name_v.size()),
705+
Array::New(isolate, type_v.data(), type_v.size())
706+
};
707+
req_wrap->Resolve(Array::New(isolate, result, arraysize(result)));
713708
}
714709

715710
void Access(const FunctionCallbackInfo<Value>& args) {
@@ -1519,13 +1514,11 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
15191514

15201515
Local<Array> names = Array::New(isolate, name_v.data(), name_v.size());
15211516
if (with_types) {
1522-
Local<Array> result = Array::New(isolate, 2);
1523-
result->Set(env->context(), 0, names).Check();
1524-
result->Set(env->context(),
1525-
1,
1526-
Array::New(isolate, type_v.data(),
1527-
type_v.size())).Check();
1528-
args.GetReturnValue().Set(result);
1517+
Local<Value> result[] = {
1518+
names,
1519+
Array::New(isolate, type_v.data(), type_v.size())
1520+
};
1521+
args.GetReturnValue().Set(Array::New(isolate, result, arraysize(result)));
15291522
} else {
15301523
args.GetReturnValue().Set(names);
15311524
}

‎src/node_v8.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -218,19 +218,19 @@ void Initialize(Local<Object> target,
218218
// Heap space names are extracted once and exposed to JavaScript to
219219
// avoid excessive creation of heap space name Strings.
220220
HeapSpaceStatistics s;
221-
const Local<Array> heap_spaces = Array::New(env->isolate(),
222-
number_of_heap_spaces);
221+
MaybeStackBuffer<Local<Value>, 16> heap_spaces(number_of_heap_spaces);
223222
for (size_t i = 0; i < number_of_heap_spaces; i++) {
224223
env->isolate()->GetHeapSpaceStatistics(&s, i);
225-
Local<String> heap_space_name = String::NewFromUtf8(env->isolate(),
226-
s.space_name(),
227-
NewStringType::kNormal)
228-
.ToLocalChecked();
229-
heap_spaces->Set(env->context(), i, heap_space_name).Check();
224+
heap_spaces[i] = String::NewFromUtf8(env->isolate(),
225+
s.space_name(),
226+
NewStringType::kNormal)
227+
.ToLocalChecked();
230228
}
231229
target->Set(env->context(),
232230
FIXED_ONE_BYTE_STRING(env->isolate(), "kHeapSpaces"),
233-
heap_spaces).Check();
231+
Array::New(env->isolate(),
232+
heap_spaces.out(),
233+
number_of_heap_spaces)).Check();
234234

235235
env->SetMethod(target,
236236
"updateHeapSpaceStatisticsArrayBuffer",

‎src/spawn_sync.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -721,18 +721,18 @@ Local<Array> SyncProcessRunner::BuildOutputArray() {
721721
CHECK(!stdio_pipes_.empty());
722722

723723
EscapableHandleScope scope(env()->isolate());
724-
Local<Context> context = env()->context();
725-
Local<Array> js_output = Array::New(env()->isolate(), stdio_count_);
724+
MaybeStackBuffer<Local<Value>, 8> js_output(stdio_pipes_.size());
726725

727726
for (uint32_t i = 0; i < stdio_pipes_.size(); i++) {
728727
SyncProcessStdioPipe* h = stdio_pipes_[i].get();
729728
if (h != nullptr && h->writable())
730-
js_output->Set(context, i, h->GetOutputAsBuffer(env())).Check();
729+
js_output[i] = h->GetOutputAsBuffer(env());
731730
else
732-
js_output->Set(context, i, Null(env()->isolate())).Check();
731+
js_output[i] = Null(env()->isolate());
733732
}
734733

735-
return scope.Escape(js_output);
734+
return scope.Escape(
735+
Array::New(env()->isolate(), js_output.out(), js_output.length()));
736736
}
737737

738738
Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {

0 commit comments

Comments
 (0)