Skip to content

Commit 92128a8

Browse files
jasnelltargos
jasnell
authored and
targos
committed
src: use DictionaryTemplate for node_url_pattern
Improved API and better performance: ``` url/urlpattern-parse.js n=100000 ... *** 11.59 % ±0.96% ±1.28% ±1.66% url/urlpattern-parse.js n=100000 ... *** 9.28 % ±0.94% ±1.25% ±1.63% url/urlpattern-parse.js n=100000 ... *** 6.97 % ±0.97% ±1.29% ±1.70% url/urlpattern-parse.js n=100000 ... *** 7.56 % ±0.92% ±1.22% ±1.59% url/urlpattern-test.js n=100000 ... *** 2.84 % ±1.50% ±2.00% ±2.61% url/urlpattern-test.js n=100000 ... *** 4.13 % ±2.14% ±2.84% ±3.70% url/urlpattern-test.js n=100000 ... *** 4.76 % ±1.43% ±1.91% ±2.49% url/urlpattern-test.js n=100000 ... *** 4.44 % ±1.26% ±1.68% ±2.19% ``` PR-URL: #59802 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
1 parent 2865d2a commit 92128a8

File tree

8 files changed

+319
-284
lines changed

8 files changed

+319
-284
lines changed

‎src/env_properties.h‎

Lines changed: 12 additions & 48 deletions
Large diffs are not rendered by default.

‎src/node_sqlite.cc‎

Lines changed: 68 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ using v8::BigInt;
2424
using v8::Boolean;
2525
using v8::ConstructorBehavior;
2626
using v8::Context;
27+
using v8::DictionaryTemplate;
2728
using v8::DontDelete;
2829
using v8::Exception;
2930
using v8::Function;
@@ -119,6 +120,18 @@ using v8::Value;
119120
} \
120121
} while (0)
121122

123+
namespace {
124+
Local<DictionaryTemplate> getLazyIterTemplate(Environment* env) {
125+
auto iter_template = env->iter_template();
126+
if (iter_template.IsEmpty()) {
127+
static constexpr std::string_view iter_keys[] = {"done", "value"};
128+
iter_template = DictionaryTemplate::New(env->isolate(), iter_keys);
129+
env->set_iter_template(iter_template);
130+
}
131+
return iter_template;
132+
}
133+
} // namespace
134+
122135
inline MaybeLocal<Object> CreateSQLiteError(Isolate* isolate,
123136
const char* message) {
124137
Local<String> js_msg;
@@ -2231,58 +2244,35 @@ void StatementSync::Columns(const FunctionCallbackInfo<Value>& args) {
22312244
int num_cols = sqlite3_column_count(stmt->statement_);
22322245
Isolate* isolate = env->isolate();
22332246
LocalVector<Value> cols(isolate);
2234-
LocalVector<Name> col_keys(isolate,
2235-
{env->column_string(),
2236-
env->database_string(),
2237-
env->name_string(),
2238-
env->table_string(),
2239-
env->type_string()});
2240-
Local<Value> value;
2247+
auto sqlite_column_template = env->sqlite_column_template();
2248+
if (sqlite_column_template.IsEmpty()) {
2249+
static constexpr std::string_view col_keys[] = {
2250+
"column", "database", "name", "table", "type"};
2251+
sqlite_column_template = DictionaryTemplate::New(isolate, col_keys);
2252+
env->set_sqlite_column_template(sqlite_column_template);
2253+
}
22412254

22422255
cols.reserve(num_cols);
22432256
for (int i = 0; i < num_cols; ++i) {
2244-
LocalVector<Value> col_values(isolate);
2245-
col_values.reserve(col_keys.size());
2246-
2247-
if (!NullableSQLiteStringToValue(
2248-
isolate, sqlite3_column_origin_name(stmt->statement_, i))
2249-
.ToLocal(&value)) {
2250-
return;
2251-
}
2252-
col_values.emplace_back(value);
2253-
2254-
if (!NullableSQLiteStringToValue(
2255-
isolate, sqlite3_column_database_name(stmt->statement_, i))
2256-
.ToLocal(&value)) {
2257-
return;
2258-
}
2259-
col_values.emplace_back(value);
2260-
2261-
if (!stmt->ColumnNameToName(i).ToLocal(&value)) {
2262-
return;
2263-
}
2264-
col_values.emplace_back(value);
2265-
2266-
if (!NullableSQLiteStringToValue(
2267-
isolate, sqlite3_column_table_name(stmt->statement_, i))
2268-
.ToLocal(&value)) {
2269-
return;
2270-
}
2271-
col_values.emplace_back(value);
2272-
2273-
if (!NullableSQLiteStringToValue(
2274-
isolate, sqlite3_column_decltype(stmt->statement_, i))
2275-
.ToLocal(&value)) {
2257+
MaybeLocal<Value> values[] = {
2258+
NullableSQLiteStringToValue(
2259+
isolate, sqlite3_column_origin_name(stmt->statement_, i)),
2260+
NullableSQLiteStringToValue(
2261+
isolate, sqlite3_column_database_name(stmt->statement_, i)),
2262+
stmt->ColumnNameToName(i),
2263+
NullableSQLiteStringToValue(
2264+
isolate, sqlite3_column_table_name(stmt->statement_, i)),
2265+
NullableSQLiteStringToValue(
2266+
isolate, sqlite3_column_decltype(stmt->statement_, i)),
2267+
};
2268+
2269+
Local<Object> col;
2270+
if (!NewDictionaryInstanceNullProto(
2271+
env->context(), sqlite_column_template, values)
2272+
.ToLocal(&col)) {
22762273
return;
22772274
}
2278-
col_values.emplace_back(value);
2279-
2280-
Local<Object> column = Object::New(isolate,
2281-
Null(isolate),
2282-
col_keys.data(),
2283-
col_values.data(),
2284-
col_keys.size());
2285-
cols.emplace_back(column);
2275+
cols.emplace_back(col);
22862276
}
22872277

22882278
args.GetReturnValue().Set(Array::New(isolate, cols.data(), cols.size()));
@@ -2514,15 +2504,19 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
25142504
THROW_AND_RETURN_ON_BAD_STATE(
25152505
env, iter->stmt_->IsFinalized(), "statement has been finalized");
25162506
Isolate* isolate = env->isolate();
2517-
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
2507+
2508+
auto iter_template = getLazyIterTemplate(env);
25182509

25192510
if (iter->done_) {
2520-
LocalVector<Value> values(isolate,
2521-
{Boolean::New(isolate, true), Null(isolate)});
2522-
DCHECK_EQ(values.size(), keys.size());
2523-
Local<Object> result = Object::New(
2524-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2525-
args.GetReturnValue().Set(result);
2511+
MaybeLocal<Value> values[]{
2512+
Boolean::New(isolate, true),
2513+
Null(isolate),
2514+
};
2515+
Local<Object> result;
2516+
if (NewDictionaryInstanceNullProto(env->context(), iter_template, values)
2517+
.ToLocal(&result)) {
2518+
args.GetReturnValue().Set(result);
2519+
}
25262520
return;
25272521
}
25282522

@@ -2531,12 +2525,12 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
25312525
CHECK_ERROR_OR_THROW(
25322526
env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void());
25332527
sqlite3_reset(iter->stmt_->statement_);
2534-
LocalVector<Value> values(isolate,
2535-
{Boolean::New(isolate, true), Null(isolate)});
2536-
DCHECK_EQ(values.size(), keys.size());
2537-
Local<Object> result = Object::New(
2538-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2539-
args.GetReturnValue().Set(result);
2528+
MaybeLocal<Value> values[] = {Boolean::New(isolate, true), Null(isolate)};
2529+
Local<Object> result;
2530+
if (NewDictionaryInstanceNullProto(env->context(), iter_template, values)
2531+
.ToLocal(&result)) {
2532+
args.GetReturnValue().Set(result);
2533+
}
25402534
return;
25412535
}
25422536

@@ -2564,11 +2558,12 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
25642558
isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
25652559
}
25662560

2567-
LocalVector<Value> values(isolate, {Boolean::New(isolate, false), row_value});
2568-
DCHECK_EQ(keys.size(), values.size());
2569-
Local<Object> result = Object::New(
2570-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2571-
args.GetReturnValue().Set(result);
2561+
MaybeLocal<Value> values[] = {Boolean::New(isolate, false), row_value};
2562+
Local<Object> result;
2563+
if (NewDictionaryInstanceNullProto(env->context(), iter_template, values)
2564+
.ToLocal(&result)) {
2565+
args.GetReturnValue().Set(result);
2566+
}
25722567
}
25732568

25742569
void StatementSyncIterator::Return(const FunctionCallbackInfo<Value>& args) {
@@ -2581,14 +2576,15 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo<Value>& args) {
25812576

25822577
sqlite3_reset(iter->stmt_->statement_);
25832578
iter->done_ = true;
2584-
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
2585-
LocalVector<Value> values(isolate,
2586-
{Boolean::New(isolate, true), Null(isolate)});
25872579

2588-
DCHECK_EQ(keys.size(), values.size());
2589-
Local<Object> result = Object::New(
2590-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2591-
args.GetReturnValue().Set(result);
2580+
auto iter_template = getLazyIterTemplate(env);
2581+
MaybeLocal<Value> values[] = {Boolean::New(isolate, true), Null(isolate)};
2582+
2583+
Local<Object> result;
2584+
if (NewDictionaryInstanceNullProto(env->context(), iter_template, values)
2585+
.ToLocal(&result)) {
2586+
args.GetReturnValue().Set(result);
2587+
}
25922588
}
25932589

25942590
Session::Session(Environment* env,

‎src/node_url_pattern.cc‎

Lines changed: 43 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,13 @@ namespace node::url_pattern {
5454

5555
using v8::Array;
5656
using v8::Context;
57+
using v8::DictionaryTemplate;
5758
using v8::DontDelete;
5859
using v8::FunctionCallbackInfo;
5960
using v8::FunctionTemplate;
6061
using v8::Global;
6162
using v8::Isolate;
6263
using v8::Local;
63-
using v8::LocalVector;
6464
using v8::MaybeLocal;
6565
using v8::Name;
6666
using v8::NewStringType;
@@ -396,56 +396,49 @@ MaybeLocal<Object> URLPattern::URLPatternComponentResult::ToJSObject(
396396
MaybeLocal<Value> URLPattern::URLPatternResult::ToJSValue(
397397
Environment* env, const ada::url_pattern_result& result) {
398398
auto isolate = env->isolate();
399-
Local<Name> names[] = {
400-
env->inputs_string(),
401-
env->protocol_string(),
402-
env->username_string(),
403-
env->password_string(),
404-
env->hostname_string(),
405-
env->port_string(),
406-
env->pathname_string(),
407-
env->search_string(),
408-
env->hash_string(),
409-
};
410-
LocalVector<Value> inputs(isolate, result.inputs.size());
411-
size_t index = 0;
412-
for (auto& input : result.inputs) {
413-
if (std::holds_alternative<std::string_view>(input)) {
414-
auto input_str = std::get<std::string_view>(input);
415-
if (!ToV8Value(env->context(), input_str).ToLocal(&inputs[index])) {
416-
return {};
417-
}
418-
} else {
419-
DCHECK(std::holds_alternative<ada::url_pattern_init>(input));
420-
auto init = std::get<ada::url_pattern_init>(input);
421-
if (!URLPatternInit::ToJsObject(env, init).ToLocal(&inputs[index])) {
422-
return {};
423-
}
424-
}
425-
index++;
426-
}
427-
LocalVector<Value> values(isolate, arraysize(names));
428-
values[0] = Array::New(isolate, inputs.data(), inputs.size());
429-
if (!URLPatternComponentResult::ToJSObject(env, result.protocol)
430-
.ToLocal(&values[1]) ||
431-
!URLPatternComponentResult::ToJSObject(env, result.username)
432-
.ToLocal(&values[2]) ||
433-
!URLPatternComponentResult::ToJSObject(env, result.password)
434-
.ToLocal(&values[3]) ||
435-
!URLPatternComponentResult::ToJSObject(env, result.hostname)
436-
.ToLocal(&values[4]) ||
437-
!URLPatternComponentResult::ToJSObject(env, result.port)
438-
.ToLocal(&values[5]) ||
439-
!URLPatternComponentResult::ToJSObject(env, result.pathname)
440-
.ToLocal(&values[6]) ||
441-
!URLPatternComponentResult::ToJSObject(env, result.search)
442-
.ToLocal(&values[7]) ||
443-
!URLPatternComponentResult::ToJSObject(env, result.hash)
444-
.ToLocal(&values[8])) {
445-
return {};
399+
400+
auto tmpl = env->urlpatternresult_template();
401+
if (tmpl.IsEmpty()) {
402+
static constexpr std::string_view namesVec[] = {
403+
"inputs",
404+
"protocol",
405+
"username",
406+
"password",
407+
"hostname",
408+
"port",
409+
"pathname",
410+
"search",
411+
"hash",
412+
};
413+
tmpl = DictionaryTemplate::New(isolate, namesVec);
414+
env->set_urlpatternresult_template(tmpl);
446415
}
447-
return Object::New(
448-
isolate, Object::New(isolate), names, values.data(), values.size());
416+
417+
size_t index = 0;
418+
MaybeLocal<Value> vals[] = {
419+
Array::New(env->context(),
420+
result.inputs.size(),
421+
[&index, &inputs = result.inputs, env]() {
422+
auto& input = inputs[index++];
423+
if (std::holds_alternative<std::string_view>(input)) {
424+
auto input_str = std::get<std::string_view>(input);
425+
return ToV8Value(env->context(), input_str);
426+
} else {
427+
DCHECK(
428+
std::holds_alternative<ada::url_pattern_init>(input));
429+
auto init = std::get<ada::url_pattern_init>(input);
430+
return URLPatternInit::ToJsObject(env, init);
431+
}
432+
}),
433+
URLPatternComponentResult::ToJSObject(env, result.protocol),
434+
URLPatternComponentResult::ToJSObject(env, result.username),
435+
URLPatternComponentResult::ToJSObject(env, result.password),
436+
URLPatternComponentResult::ToJSObject(env, result.hostname),
437+
URLPatternComponentResult::ToJSObject(env, result.port),
438+
URLPatternComponentResult::ToJSObject(env, result.pathname),
439+
URLPatternComponentResult::ToJSObject(env, result.search),
440+
URLPatternComponentResult::ToJSObject(env, result.hash)};
441+
return NewDictionaryInstanceNullProto(env->context(), tmpl, vals);
449442
}
450443

451444
std::optional<ada::url_pattern_options>

‎src/node_util.cc‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ using v8::BigInt;
1515
using v8::Boolean;
1616
using v8::CFunction;
1717
using v8::Context;
18+
using v8::DictionaryTemplate;
1819
using v8::External;
1920
using v8::FunctionCallbackInfo;
2021
using v8::IndexFilter;
@@ -23,6 +24,7 @@ using v8::Isolate;
2324
using v8::KeyCollectionMode;
2425
using v8::Local;
2526
using v8::LocalVector;
27+
using v8::MaybeLocal;
2628
using v8::Name;
2729
using v8::Object;
2830
using v8::ObjectTemplate;
@@ -263,6 +265,20 @@ static void GetCallSites(const FunctionCallbackInfo<Value>& args) {
263265
const int frame_count = stack->GetFrameCount();
264266
LocalVector<Value> callsite_objects(isolate);
265267

268+
auto callsite_template = env->callsite_template();
269+
if (callsite_template.IsEmpty()) {
270+
static constexpr std::string_view names[] = {
271+
"functionName",
272+
"scriptId",
273+
"scriptName",
274+
"lineNumber",
275+
"columnNumber",
276+
// TODO(legendecas): deprecate CallSite.column.
277+
"column"};
278+
callsite_template = DictionaryTemplate::New(isolate, names);
279+
env->set_callsite_template(callsite_template);
280+
}
281+
266282
// Frame 0 is node:util. It should be skipped.
267283
for (int i = 1; i < frame_count; ++i) {
268284
Local<StackFrame> stack_frame = stack->GetFrame(isolate, i);
@@ -279,16 +295,7 @@ static void GetCallSites(const FunctionCallbackInfo<Value>& args) {
279295

280296
std::string script_id = std::to_string(stack_frame->GetScriptId());
281297

282-
Local<Name> names[] = {
283-
env->function_name_string(),
284-
env->script_id_string(),
285-
env->script_name_string(),
286-
env->line_number_string(),
287-
env->column_number_string(),
288-
// TODO(legendecas): deprecate CallSite.column.
289-
env->column_string(),
290-
};
291-
Local<Value> values[] = {
298+
MaybeLocal<Value> values[] = {
292299
function_name,
293300
OneByteString(isolate, script_id),
294301
script_name,
@@ -297,10 +304,14 @@ static void GetCallSites(const FunctionCallbackInfo<Value>& args) {
297304
// TODO(legendecas): deprecate CallSite.column.
298305
Integer::NewFromUnsigned(isolate, stack_frame->GetColumn()),
299306
};
300-
Local<Object> obj = Object::New(
301-
isolate, v8::Null(isolate), names, values, arraysize(names));
302307

303-
callsite_objects.push_back(obj);
308+
Local<Object> callsite;
309+
if (!NewDictionaryInstanceNullProto(
310+
env->context(), callsite_template, values)
311+
.ToLocal(&callsite)) {
312+
return;
313+
}
314+
callsite_objects.push_back(callsite);
304315
}
305316

306317
Local<Array> callsites =

0 commit comments

Comments
 (0)