Skip to content

Commit 8ec29f2

Browse files
jasnelljasnell
authored
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 <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 7d0f6be commit 8ec29f2

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;
@@ -2239,58 +2252,35 @@ void StatementSync::Columns(const FunctionCallbackInfo<Value>& args) {
22392252
int num_cols = sqlite3_column_count(stmt->statement_);
22402253
Isolate* isolate = env->isolate();
22412254
LocalVector<Value> cols(isolate);
2242-
LocalVector<Name> col_keys(isolate,
2243-
{env->column_string(),
2244-
env->database_string(),
2245-
env->name_string(),
2246-
env->table_string(),
2247-
env->type_string()});
2248-
Local<Value> value;
2255+
auto sqlite_column_template = env->sqlite_column_template();
2256+
if (sqlite_column_template.IsEmpty()) {
2257+
static constexpr std::string_view col_keys[] = {
2258+
"column", "database", "name", "table", "type"};
2259+
sqlite_column_template = DictionaryTemplate::New(isolate, col_keys);
2260+
env->set_sqlite_column_template(sqlite_column_template);
2261+
}
22492262

22502263
cols.reserve(num_cols);
22512264
for (int i = 0; i < num_cols; ++i) {
2252-
LocalVector<Value> col_values(isolate);
2253-
col_values.reserve(col_keys.size());
2254-
2255-
if (!NullableSQLiteStringToValue(
2256-
isolate, sqlite3_column_origin_name(stmt->statement_, i))
2257-
.ToLocal(&value)) {
2258-
return;
2259-
}
2260-
col_values.emplace_back(value);
2261-
2262-
if (!NullableSQLiteStringToValue(
2263-
isolate, sqlite3_column_database_name(stmt->statement_, i))
2264-
.ToLocal(&value)) {
2265-
return;
2266-
}
2267-
col_values.emplace_back(value);
2268-
2269-
if (!stmt->ColumnNameToName(i).ToLocal(&value)) {
2270-
return;
2271-
}
2272-
col_values.emplace_back(value);
2273-
2274-
if (!NullableSQLiteStringToValue(
2275-
isolate, sqlite3_column_table_name(stmt->statement_, i))
2276-
.ToLocal(&value)) {
2277-
return;
2278-
}
2279-
col_values.emplace_back(value);
2280-
2281-
if (!NullableSQLiteStringToValue(
2282-
isolate, sqlite3_column_decltype(stmt->statement_, i))
2283-
.ToLocal(&value)) {
2265+
MaybeLocal<Value> values[] = {
2266+
NullableSQLiteStringToValue(
2267+
isolate, sqlite3_column_origin_name(stmt->statement_, i)),
2268+
NullableSQLiteStringToValue(
2269+
isolate, sqlite3_column_database_name(stmt->statement_, i)),
2270+
stmt->ColumnNameToName(i),
2271+
NullableSQLiteStringToValue(
2272+
isolate, sqlite3_column_table_name(stmt->statement_, i)),
2273+
NullableSQLiteStringToValue(
2274+
isolate, sqlite3_column_decltype(stmt->statement_, i)),
2275+
};
2276+
2277+
Local<Object> col;
2278+
if (!NewDictionaryInstanceNullProto(
2279+
env->context(), sqlite_column_template, values)
2280+
.ToLocal(&col)) {
22842281
return;
22852282
}
2286-
col_values.emplace_back(value);
2287-
2288-
Local<Object> column = Object::New(isolate,
2289-
Null(isolate),
2290-
col_keys.data(),
2291-
col_values.data(),
2292-
col_keys.size());
2293-
cols.emplace_back(column);
2283+
cols.emplace_back(col);
22942284
}
22952285

22962286
args.GetReturnValue().Set(Array::New(isolate, cols.data(), cols.size()));
@@ -2522,15 +2512,19 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
25222512
THROW_AND_RETURN_ON_BAD_STATE(
25232513
env, iter->stmt_->IsFinalized(), "statement has been finalized");
25242514
Isolate* isolate = env->isolate();
2525-
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
2515+
2516+
auto iter_template = getLazyIterTemplate(env);
25262517

25272518
if (iter->done_) {
2528-
LocalVector<Value> values(isolate,
2529-
{Boolean::New(isolate, true), Null(isolate)});
2530-
DCHECK_EQ(values.size(), keys.size());
2531-
Local<Object> result = Object::New(
2532-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2533-
args.GetReturnValue().Set(result);
2519+
MaybeLocal<Value> values[]{
2520+
Boolean::New(isolate, true),
2521+
Null(isolate),
2522+
};
2523+
Local<Object> result;
2524+
if (NewDictionaryInstanceNullProto(env->context(), iter_template, values)
2525+
.ToLocal(&result)) {
2526+
args.GetReturnValue().Set(result);
2527+
}
25342528
return;
25352529
}
25362530

@@ -2539,12 +2533,12 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
25392533
CHECK_ERROR_OR_THROW(
25402534
env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void());
25412535
sqlite3_reset(iter->stmt_->statement_);
2542-
LocalVector<Value> values(isolate,
2543-
{Boolean::New(isolate, true), Null(isolate)});
2544-
DCHECK_EQ(values.size(), keys.size());
2545-
Local<Object> result = Object::New(
2546-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2547-
args.GetReturnValue().Set(result);
2536+
MaybeLocal<Value> values[] = {Boolean::New(isolate, true), Null(isolate)};
2537+
Local<Object> result;
2538+
if (NewDictionaryInstanceNullProto(env->context(), iter_template, values)
2539+
.ToLocal(&result)) {
2540+
args.GetReturnValue().Set(result);
2541+
}
25482542
return;
25492543
}
25502544

@@ -2572,11 +2566,12 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
25722566
isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
25732567
}
25742568

2575-
LocalVector<Value> values(isolate, {Boolean::New(isolate, false), row_value});
2576-
DCHECK_EQ(keys.size(), values.size());
2577-
Local<Object> result = Object::New(
2578-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2579-
args.GetReturnValue().Set(result);
2569+
MaybeLocal<Value> values[] = {Boolean::New(isolate, false), row_value};
2570+
Local<Object> result;
2571+
if (NewDictionaryInstanceNullProto(env->context(), iter_template, values)
2572+
.ToLocal(&result)) {
2573+
args.GetReturnValue().Set(result);
2574+
}
25802575
}
25812576

25822577
void StatementSyncIterator::Return(const FunctionCallbackInfo<Value>& args) {
@@ -2589,14 +2584,15 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo<Value>& args) {
25892584

25902585
sqlite3_reset(iter->stmt_->statement_);
25912586
iter->done_ = true;
2592-
LocalVector<Name> keys(isolate, {env->done_string(), env->value_string()});
2593-
LocalVector<Value> values(isolate,
2594-
{Boolean::New(isolate, true), Null(isolate)});
25952587

2596-
DCHECK_EQ(keys.size(), values.size());
2597-
Local<Object> result = Object::New(
2598-
isolate, Null(isolate), keys.data(), values.data(), keys.size());
2599-
args.GetReturnValue().Set(result);
2588+
auto iter_template = getLazyIterTemplate(env);
2589+
MaybeLocal<Value> values[] = {Boolean::New(isolate, true), Null(isolate)};
2590+
2591+
Local<Object> result;
2592+
if (NewDictionaryInstanceNullProto(env->context(), iter_template, values)
2593+
.ToLocal(&result)) {
2594+
args.GetReturnValue().Set(result);
2595+
}
26002596
}
26012597

26022598
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)