Skip to content

Commit f674b09

Browse files
bnoordhuisbnoordhuis
committed
src: use v8::String::NewFrom*() functions
* Change calls to String::New() and String::NewSymbol() to their respective one-byte, two-byte and UTF-8 counterparts. * Add a FIXED_ONE_BYTE_STRING macro that takes a string literal and turns it into a v8::Local<v8::String>. * Add helper functions that make v8::String::NewFromOneByte() easier to work with. Said function expects a `const uint8_t*` but almost every call site deals with `const char*` or `const unsigned char*`. Helps us avoid doing reinterpret_casts all over the place. * Code that handles file system paths keeps using UTF-8 for backwards compatibility reasons. At least now the use of UTF-8 is explicit. * Remove v8::String::NewSymbol() entirely. Almost all call sites were effectively minor de-optimizations. If you create a string only once, there is no point in making it a symbol. If you are create the same string repeatedly, it should probably be cached in a persistent handle.
1 parent c0e7035 commit f674b09

29 files changed

Lines changed: 749 additions & 514 deletions

‎src/cares_wrap.cc‎

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ static Local<Array> HostentToAddresses(struct hostent* host) {
206206
for (int i = 0; host->h_addr_list[i]; ++i) {
207207
uv_inet_ntop(host->h_addrtype, host->h_addr_list[i], ip, sizeof(ip));
208208

209-
Local<String> address = String::New(ip);
209+
Local<String> address = OneByteString(node_isolate, ip);
210210
addresses->Set(Integer::New(i, node_isolate), address);
211211
}
212212

@@ -219,7 +219,7 @@ static Local<Array> HostentToNames(struct hostent* host) {
219219
Local<Array> names = Array::New();
220220

221221
for (int i = 0; host->h_aliases[i]; ++i) {
222-
Local<String> address = String::New(host->h_aliases[i]);
222+
Local<String> address = OneByteString(node_isolate, host->h_aliases[i]);
223223
names->Set(Integer::New(i, node_isolate), address);
224224
}
225225

@@ -426,7 +426,7 @@ class QueryCnameWrap: public QueryWrap {
426426
// A cname lookup always returns a single record but we follow the
427427
// common API here.
428428
Local<Array> result = Array::New(1);
429-
result->Set(0, String::New(host->h_name));
429+
result->Set(0, OneByteString(node_isolate, host->h_name));
430430
ares_free_hostent(host);
431431

432432
this->CallOnComplete(result);
@@ -456,14 +456,18 @@ class QueryMxWrap: public QueryWrap {
456456
}
457457

458458
Local<Array> mx_records = Array::New();
459-
Local<String> exchange_symbol = String::NewSymbol("exchange");
460-
Local<String> priority_symbol = String::NewSymbol("priority");
459+
Local<String> exchange_symbol =
460+
FIXED_ONE_BYTE_STRING(node_isolate, "exchange");
461+
Local<String> priority_symbol =
462+
FIXED_ONE_BYTE_STRING(node_isolate, "priority");
463+
461464
int i = 0;
462465
for (struct ares_mx_reply* mx_current = mx_start;
463466
mx_current;
464467
mx_current = mx_current->next) {
465468
Local<Object> mx_record = Object::New();
466-
mx_record->Set(exchange_symbol, String::New(mx_current->host));
469+
mx_record->Set(exchange_symbol,
470+
OneByteString(node_isolate, mx_current->host));
467471
mx_record->Set(priority_symbol,
468472
Integer::New(mx_current->priority, node_isolate));
469473
mx_records->Set(Integer::New(i++, node_isolate), mx_record);
@@ -528,7 +532,7 @@ class QueryTxtWrap: public QueryWrap {
528532

529533
struct ares_txt_reply *current = txt_out;
530534
for (int i = 0; current; ++i, current = current->next) {
531-
Local<String> txt = String::New(reinterpret_cast<char*>(current->txt));
535+
Local<String> txt = OneByteString(node_isolate, current->txt);
532536
txt_records->Set(Integer::New(i, node_isolate), txt);
533537
}
534538

@@ -566,16 +570,22 @@ class QuerySrvWrap: public QueryWrap {
566570
}
567571

568572
Local<Array> srv_records = Array::New();
569-
Local<String> name_symbol = String::NewSymbol("name");
570-
Local<String> port_symbol = String::NewSymbol("port");
571-
Local<String> priority_symbol = String::NewSymbol("priority");
572-
Local<String> weight_symbol = String::NewSymbol("weight");
573+
Local<String> name_symbol =
574+
FIXED_ONE_BYTE_STRING(node_isolate, "name");
575+
Local<String> port_symbol =
576+
FIXED_ONE_BYTE_STRING(node_isolate, "port");
577+
Local<String> priority_symbol =
578+
FIXED_ONE_BYTE_STRING(node_isolate, "priority");
579+
Local<String> weight_symbol =
580+
FIXED_ONE_BYTE_STRING(node_isolate, "weight");
581+
573582
int i = 0;
574583
for (struct ares_srv_reply* srv_current = srv_start;
575584
srv_current;
576585
srv_current = srv_current->next) {
577586
Local<Object> srv_record = Object::New();
578-
srv_record->Set(name_symbol, String::New(srv_current->host));
587+
srv_record->Set(name_symbol,
588+
OneByteString(node_isolate, srv_current->host));
579589
srv_record->Set(port_symbol,
580590
Integer::New(srv_current->port, node_isolate));
581591
srv_record->Set(priority_symbol,
@@ -620,12 +630,18 @@ class QueryNaptrWrap: public QueryWrap {
620630
}
621631

622632
Local<Array> naptr_records = Array::New();
623-
Local<String> flags_symbol = String::NewSymbol("flags");
624-
Local<String> service_symbol = String::NewSymbol("service");
625-
Local<String> regexp_symbol = String::NewSymbol("regexp");
626-
Local<String> replacement_symbol = String::NewSymbol("replacement");
627-
Local<String> order_symbol = String::NewSymbol("order");
628-
Local<String> preference_symbol = String::NewSymbol("preference");
633+
Local<String> flags_symbol =
634+
FIXED_ONE_BYTE_STRING(node_isolate, "flags");
635+
Local<String> service_symbol =
636+
FIXED_ONE_BYTE_STRING(node_isolate, "service");
637+
Local<String> regexp_symbol =
638+
FIXED_ONE_BYTE_STRING(node_isolate, "regexp");
639+
Local<String> replacement_symbol =
640+
FIXED_ONE_BYTE_STRING(node_isolate, "replacement");
641+
Local<String> order_symbol =
642+
FIXED_ONE_BYTE_STRING(node_isolate, "order");
643+
Local<String> preference_symbol =
644+
FIXED_ONE_BYTE_STRING(node_isolate, "preference");
629645

630646
int i = 0;
631647
for (ares_naptr_reply* naptr_current = naptr_start;
@@ -634,13 +650,14 @@ class QueryNaptrWrap: public QueryWrap {
634650
Local<Object> naptr_record = Object::New();
635651

636652
naptr_record->Set(flags_symbol,
637-
String::New(reinterpret_cast<char*>(naptr_current->flags)));
653+
OneByteString(node_isolate, naptr_current->flags));
638654
naptr_record->Set(service_symbol,
639-
String::New(reinterpret_cast<char*>(naptr_current->service)));
655+
OneByteString(node_isolate, naptr_current->service));
640656
naptr_record->Set(regexp_symbol,
641-
String::New(reinterpret_cast<char*>(naptr_current->regexp)));
657+
OneByteString(node_isolate, naptr_current->regexp));
642658
naptr_record->Set(replacement_symbol,
643-
String::New(naptr_current->replacement));
659+
OneByteString(node_isolate,
660+
naptr_current->replacement));
644661
naptr_record->Set(order_symbol, Integer::New(naptr_current->order,
645662
node_isolate));
646663
naptr_record->Set(preference_symbol,
@@ -814,7 +831,7 @@ void AfterGetAddrInfo(uv_getaddrinfo_t* req, int status, struct addrinfo* res) {
814831
continue;
815832

816833
// Create JavaScript string
817-
Local<String> s = String::New(ip);
834+
Local<String> s = OneByteString(node_isolate, ip);
818835
results->Set(n, s);
819836
n++;
820837
}
@@ -841,7 +858,7 @@ void AfterGetAddrInfo(uv_getaddrinfo_t* req, int status, struct addrinfo* res) {
841858
continue;
842859

843860
// Create JavaScript string
844-
Local<String> s = String::New(ip);
861+
Local<String> s = OneByteString(node_isolate, ip);
845862
results->Set(n, s);
846863
n++;
847864
}
@@ -943,7 +960,7 @@ static void GetServers(const FunctionCallbackInfo<Value>& args) {
943960
int err = uv_inet_ntop(cur->family, caddr, ip, sizeof(ip));
944961
assert(err == 0);
945962

946-
Local<String> addr = String::New(ip);
963+
Local<String> addr = OneByteString(node_isolate, ip);
947964
server_array->Set(i, addr);
948965
}
949966

@@ -1024,7 +1041,7 @@ static void SetServers(const FunctionCallbackInfo<Value>& args) {
10241041
static void StrError(const FunctionCallbackInfo<Value>& args) {
10251042
HandleScope scope(node_isolate);
10261043
const char* errmsg = ares_strerror(args[0]->Int32Value());
1027-
args.GetReturnValue().Set(String::New(errmsg));
1044+
args.GetReturnValue().Set(OneByteString(node_isolate, errmsg));
10281045
}
10291046

10301047

@@ -1069,14 +1086,14 @@ static void Initialize(Handle<Object> target) {
10691086
NODE_SET_METHOD(target, "getServers", GetServers);
10701087
NODE_SET_METHOD(target, "setServers", SetServers);
10711088

1072-
target->Set(String::NewSymbol("AF_INET"),
1089+
target->Set(FIXED_ONE_BYTE_STRING(node_isolate, "AF_INET"),
10731090
Integer::New(AF_INET, node_isolate));
1074-
target->Set(String::NewSymbol("AF_INET6"),
1091+
target->Set(FIXED_ONE_BYTE_STRING(node_isolate, "AF_INET6"),
10751092
Integer::New(AF_INET6, node_isolate));
1076-
target->Set(String::NewSymbol("AF_UNSPEC"),
1093+
target->Set(FIXED_ONE_BYTE_STRING(node_isolate, "AF_UNSPEC"),
10771094
Integer::New(AF_UNSPEC, node_isolate));
10781095

1079-
oncomplete_sym = String::New("oncomplete");
1096+
oncomplete_sym = FIXED_ONE_BYTE_STRING(node_isolate, "oncomplete");
10801097
}
10811098

10821099
} // namespace cares_wrap

‎src/fs_event_wrap.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,16 @@ void FSEventWrap::Initialize(Handle<Object> target) {
7575

7676
Local<FunctionTemplate> t = FunctionTemplate::New(New);
7777
t->InstanceTemplate()->SetInternalFieldCount(1);
78-
t->SetClassName(String::NewSymbol("FSEvent"));
78+
t->SetClassName(FIXED_ONE_BYTE_STRING(node_isolate, "FSEvent"));
7979

8080
NODE_SET_PROTOTYPE_METHOD(t, "start", Start);
8181
NODE_SET_PROTOTYPE_METHOD(t, "close", Close);
8282

83-
target->Set(String::New("FSEvent"), t->GetFunction());
83+
target->Set(FIXED_ONE_BYTE_STRING(node_isolate, "FSEvent"), t->GetFunction());
8484

85-
change_sym = String::New("change");
86-
onchange_sym = String::New("onchange");
87-
rename_sym = String::New("rename");
85+
change_sym = FIXED_ONE_BYTE_STRING(node_isolate, "change");
86+
onchange_sym = FIXED_ONE_BYTE_STRING(node_isolate, "onchange");
87+
rename_sym = FIXED_ONE_BYTE_STRING(node_isolate, "rename");
8888
}
8989

9090

@@ -161,7 +161,7 @@ void FSEventWrap::OnEvent(uv_fs_event_t* handle, const char* filename,
161161
};
162162

163163
if (filename != NULL) {
164-
argv[2] = String::New(filename);
164+
argv[2] = OneByteString(node_isolate, filename);
165165
}
166166

167167
MakeCallback(wrap->object(), onchange_sym, ARRAY_SIZE(argv), argv);

‎src/handle_wrap.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ void HandleWrap::Close(const FunctionCallbackInfo<Value>& args) {
7676
wrap->handle__ = NULL;
7777

7878
if (args[0]->IsFunction()) {
79-
if (close_sym.IsEmpty() == true) close_sym = String::New("close");
79+
if (close_sym.IsEmpty() == true) {
80+
close_sym = FIXED_ONE_BYTE_STRING(node_isolate, "close");
81+
}
8082
wrap->object()->Set(close_sym, args[0]);
8183
wrap->flags_ |= kCloseCallback;
8284
}

0 commit comments

Comments
 (0)