From 8b738c18d5763fab8878e97a15c60374dcb81c1d Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 7 Sep 2025 19:33:06 -0700 Subject: [PATCH 1/7] src: use DictionaryTemplate for node_url_pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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% ``` --- src/env_properties.h | 1 + src/node_url_pattern.cc | 95 ++++++++++++++++++++--------------------- 2 files changed, 47 insertions(+), 49 deletions(-) diff --git a/src/env_properties.h b/src/env_properties.h index 96e60c12d2b47d..df528027cd2b57 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -498,6 +498,7 @@ V(qlogoutputstream_constructor_template, v8::ObjectTemplate) \ V(tcp_constructor_template, v8::FunctionTemplate) \ V(tty_constructor_template, v8::FunctionTemplate) \ + V(urlpatternresult_template, v8::DictionaryTemplate) \ V(write_wrap_template, v8::ObjectTemplate) \ V(worker_cpu_profile_taker_template, v8::ObjectTemplate) \ V(worker_cpu_usage_taker_template, v8::ObjectTemplate) \ diff --git a/src/node_url_pattern.cc b/src/node_url_pattern.cc index e84e9b0de9ab20..551989c814df04 100644 --- a/src/node_url_pattern.cc +++ b/src/node_url_pattern.cc @@ -54,6 +54,7 @@ namespace node::url_pattern { using v8::Array; using v8::Context; +using v8::DictionaryTemplate; using v8::DontDelete; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -396,56 +397,52 @@ MaybeLocal URLPattern::URLPatternComponentResult::ToJSObject( MaybeLocal URLPattern::URLPatternResult::ToJSValue( Environment* env, const ada::url_pattern_result& result) { auto isolate = env->isolate(); - Local names[] = { - env->inputs_string(), - env->protocol_string(), - env->username_string(), - env->password_string(), - env->hostname_string(), - env->port_string(), - env->pathname_string(), - env->search_string(), - env->hash_string(), - }; - LocalVector inputs(isolate, result.inputs.size()); - size_t index = 0; - for (auto& input : result.inputs) { - if (std::holds_alternative(input)) { - auto input_str = std::get(input); - if (!ToV8Value(env->context(), input_str).ToLocal(&inputs[index])) { - return {}; - } - } else { - DCHECK(std::holds_alternative(input)); - auto init = std::get(input); - if (!URLPatternInit::ToJsObject(env, init).ToLocal(&inputs[index])) { - return {}; - } - } - index++; - } - LocalVector values(isolate, arraysize(names)); - values[0] = Array::New(isolate, inputs.data(), inputs.size()); - if (!URLPatternComponentResult::ToJSObject(env, result.protocol) - .ToLocal(&values[1]) || - !URLPatternComponentResult::ToJSObject(env, result.username) - .ToLocal(&values[2]) || - !URLPatternComponentResult::ToJSObject(env, result.password) - .ToLocal(&values[3]) || - !URLPatternComponentResult::ToJSObject(env, result.hostname) - .ToLocal(&values[4]) || - !URLPatternComponentResult::ToJSObject(env, result.port) - .ToLocal(&values[5]) || - !URLPatternComponentResult::ToJSObject(env, result.pathname) - .ToLocal(&values[6]) || - !URLPatternComponentResult::ToJSObject(env, result.search) - .ToLocal(&values[7]) || - !URLPatternComponentResult::ToJSObject(env, result.hash) - .ToLocal(&values[8])) { - return {}; + + auto tmpl = env->urlpatternresult_template(); + if (tmpl.IsEmpty()) { + std::string_view namesVec[] = { + "inputs", + "protocol", + "username", + "password", + "hostname", + "port", + "pathname", + "search", + "hash", + }; + tmpl = DictionaryTemplate::New(isolate, namesVec); + env->set_urlpatternresult_template(tmpl); } - return Object::New( - isolate, Object::New(isolate), names, values.data(), values.size()); + + size_t index = 0; + auto context = isolate->GetCurrentContext(); + + MaybeLocal vals[] = { + Array::New(context, + result.inputs.size(), + [&index, &inputs = result.inputs, env]() { + auto& input = inputs[index++]; + if (std::holds_alternative(input)) { + auto input_str = std::get(input); + return ToV8Value(env->context(), input_str); + } else { + DCHECK( + std::holds_alternative(input)); + auto init = std::get(input); + return URLPatternInit::ToJsObject(env, init); + } + }), + URLPatternComponentResult::ToJSObject(env, result.protocol), + URLPatternComponentResult::ToJSObject(env, result.username), + URLPatternComponentResult::ToJSObject(env, result.password), + URLPatternComponentResult::ToJSObject(env, result.hostname), + URLPatternComponentResult::ToJSObject(env, result.port), + URLPatternComponentResult::ToJSObject(env, result.pathname), + URLPatternComponentResult::ToJSObject(env, result.search), + URLPatternComponentResult::ToJSObject(env, result.hash), + }; + return tmpl->NewInstance(env->context(), vals); } std::optional From 22c57aaba7468880c26e5a829e68ea9fbc238ebe Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 7 Sep 2025 20:07:40 -0700 Subject: [PATCH 2/7] src: use DictionaryTemplate for worker cpuUsage and getHeapStatistics --- src/env_properties.h | 2 ++ src/node_worker.cc | 69 ++++++++++++++++++++++++-------------------- 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/env_properties.h b/src/env_properties.h index df528027cd2b57..10499d620d5fdc 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -459,6 +459,7 @@ V(blocklist_constructor_template, v8::FunctionTemplate) \ V(contextify_global_template, v8::ObjectTemplate) \ V(contextify_wrapper_template, v8::ObjectTemplate) \ + V(cpu_usage_template, v8::DictionaryTemplate) \ V(crypto_key_object_handle_constructor, v8::FunctionTemplate) \ V(env_proxy_template, v8::ObjectTemplate) \ V(env_proxy_ctor_template, v8::FunctionTemplate) \ @@ -469,6 +470,7 @@ V(filehandlereadwrap_template, v8::ObjectTemplate) \ V(fsreqpromise_constructor_template, v8::ObjectTemplate) \ V(handle_wrap_ctor_template, v8::FunctionTemplate) \ + V(heap_statistics_template, v8::DictionaryTemplate) \ V(histogram_ctor_template, v8::FunctionTemplate) \ V(http2settings_constructor_template, v8::ObjectTemplate) \ V(http2stream_constructor_template, v8::ObjectTemplate) \ diff --git a/src/node_worker.cc b/src/node_worker.cc index 0b606092a466e2..03fba36a098128 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -27,6 +27,7 @@ using v8::Context; using v8::CpuProfile; using v8::CpuProfilingResult; using v8::CpuProfilingStatus; +using v8::DictionaryTemplate; using v8::Float64Array; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -36,6 +37,7 @@ using v8::Isolate; using v8::Local; using v8::Locker; using v8::Maybe; +using v8::MaybeLocal; using v8::Name; using v8::NewStringType; using v8::Null; @@ -874,11 +876,17 @@ void Worker::CpuUsage(const FunctionCallbackInfo& args) { argv[0] = UVException( isolate, err, "uv_getrusage_thread", nullptr, nullptr, nullptr); } else { - Local names[] = { - FIXED_ONE_BYTE_STRING(isolate, "user"), - FIXED_ONE_BYTE_STRING(isolate, "system"), - }; - Local values[] = { + auto tmpl = env->cpu_usage_template(); + if (tmpl.IsEmpty()) { + std::string_view names[] = { + "user", + "system", + }; + tmpl = DictionaryTemplate::New(isolate, names); + env->set_cpu_usage_template(tmpl); + } + + MaybeLocal values[] = { Number::New(isolate, 1e6 * cpu_usage_stats->ru_utime.tv_sec + cpu_usage_stats->ru_utime.tv_usec), @@ -886,8 +894,7 @@ void Worker::CpuUsage(const FunctionCallbackInfo& args) { 1e6 * cpu_usage_stats->ru_stime.tv_sec + cpu_usage_stats->ru_stime.tv_usec), }; - argv[1] = Object::New( - isolate, Null(isolate), names, values, arraysize(names)); + argv[1] = tmpl->NewInstance(env->context(), values); } taker->MakeCallback(env->ondone_string(), arraysize(argv), argv); @@ -1071,24 +1078,30 @@ void Worker::GetHeapStatistics(const FunctionCallbackInfo& args) { AsyncHooks::DefaultTriggerAsyncIdScope trigger_id_scope(taker->get()); - Local heap_stats_names[] = { - FIXED_ONE_BYTE_STRING(isolate, "total_heap_size"), - FIXED_ONE_BYTE_STRING(isolate, "total_heap_size_executable"), - FIXED_ONE_BYTE_STRING(isolate, "total_physical_size"), - FIXED_ONE_BYTE_STRING(isolate, "total_available_size"), - FIXED_ONE_BYTE_STRING(isolate, "used_heap_size"), - FIXED_ONE_BYTE_STRING(isolate, "heap_size_limit"), - FIXED_ONE_BYTE_STRING(isolate, "malloced_memory"), - FIXED_ONE_BYTE_STRING(isolate, "peak_malloced_memory"), - FIXED_ONE_BYTE_STRING(isolate, "does_zap_garbage"), - FIXED_ONE_BYTE_STRING(isolate, "number_of_native_contexts"), - FIXED_ONE_BYTE_STRING(isolate, "number_of_detached_contexts"), - FIXED_ONE_BYTE_STRING(isolate, "total_global_handles_size"), - FIXED_ONE_BYTE_STRING(isolate, "used_global_handles_size"), - FIXED_ONE_BYTE_STRING(isolate, "external_memory")}; + auto tmpl = env->heap_statistics_template(); + if (tmpl.IsEmpty()) { + std::string_view heap_stats_names[] = { + "total_heap_size", + "total_heap_size_executable", + "total_physical_size", + "total_available_size", + "used_heap_size", + "heap_size_limit", + "malloced_memory", + "peak_malloced_memory", + "does_zap_garbage", + "number_of_native_contexts", + "number_of_detached_contexts", + "total_global_handles_size", + "used_global_handles_size", + "external_memory", + }; + tmpl = v8::DictionaryTemplate::New(isolate, heap_stats_names); + env->set_heap_statistics_template(tmpl); + } // Define an array of property values - Local heap_stats_values[] = { + MaybeLocal heap_stats_values[] = { Number::New(isolate, heap_stats->total_heap_size()), Number::New(isolate, heap_stats->total_heap_size_executable()), Number::New(isolate, heap_stats->total_physical_size()), @@ -1106,14 +1119,8 @@ void Worker::GetHeapStatistics(const FunctionCallbackInfo& args) { DCHECK_EQ(arraysize(heap_stats_names), arraysize(heap_stats_values)); - // Create the object with the property names and values - Local stats = Object::New(isolate, - Null(isolate), - heap_stats_names, - heap_stats_values, - arraysize(heap_stats_names)); - - Local args[] = {stats}; + Local args[] = { + tmpl->NewInstance(env->context(), heap_stats_values)}; taker->get()->MakeCallback( env->ondone_string(), arraysize(args), args); // implicitly delete `taker` From 795d80bb3f6b32e374fcc40b713f9e248442ddc0 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 7 Sep 2025 20:38:27 -0700 Subject: [PATCH 3/7] src: update v8 to use DictionaryTemplate caching --- src/env_properties.h | 5 ++ src/node_v8.cc | 127 +++++++++++++++++++++---------------------- 2 files changed, 67 insertions(+), 65 deletions(-) diff --git a/src/env_properties.h b/src/env_properties.h index 10499d620d5fdc..a908f50d538b8b 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -468,9 +468,11 @@ V(fdclose_constructor_template, v8::ObjectTemplate) \ V(fdentry_constructor_template, v8::FunctionTemplate) \ V(filehandlereadwrap_template, v8::ObjectTemplate) \ + V(free_list_statistics_template, v8::DictionaryTemplate) \ V(fsreqpromise_constructor_template, v8::ObjectTemplate) \ V(handle_wrap_ctor_template, v8::FunctionTemplate) \ V(heap_statistics_template, v8::DictionaryTemplate) \ + V(v8_heap_statistics_template, v8::DictionaryTemplate) \ V(histogram_ctor_template, v8::FunctionTemplate) \ V(http2settings_constructor_template, v8::ObjectTemplate) \ V(http2stream_constructor_template, v8::ObjectTemplate) \ @@ -483,6 +485,8 @@ V(message_port_constructor_template, v8::FunctionTemplate) \ V(module_wrap_constructor_template, v8::FunctionTemplate) \ V(microtask_queue_ctor_template, v8::FunctionTemplate) \ + V(object_stats_template, v8::DictionaryTemplate) \ + V(page_stats_template, v8::DictionaryTemplate) \ V(pipe_constructor_template, v8::FunctionTemplate) \ V(promise_wrap_template, v8::ObjectTemplate) \ V(sab_lifetimepartner_constructor_template, v8::FunctionTemplate) \ @@ -490,6 +494,7 @@ V(secure_context_constructor_template, v8::FunctionTemplate) \ V(shutdown_wrap_template, v8::ObjectTemplate) \ V(socketaddress_constructor_template, v8::FunctionTemplate) \ + V(space_stats_template, v8::DictionaryTemplate) \ V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \ V(sqlite_statement_sync_iterator_constructor_template, v8::FunctionTemplate) \ V(sqlite_session_constructor_template, v8::FunctionTemplate) \ diff --git a/src/node_v8.cc b/src/node_v8.cc index 9f3c721680fabf..0b6f5aa258c004 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -35,6 +35,7 @@ using v8::Array; using v8::BigInt; using v8::CFunction; using v8::Context; +using v8::DictionaryTemplate; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; @@ -326,9 +327,57 @@ static void SetHeapStatistics(JSONWriter* writer, Isolate* isolate) { static MaybeLocal ConvertHeapStatsToJSObject( Isolate* isolate, const cppgc::HeapStatistics& stats) { Local context = isolate->GetCurrentContext(); + Environment* env = Environment::GetCurrent(isolate); // Space Statistics LocalVector space_statistics_array(isolate); space_statistics_array.reserve(stats.space_stats.size()); + + auto object_stats_template = env->object_stats_template(); + auto page_stats_tmpl = env->page_stats_template(); + auto free_list_statistics_template = env->free_list_statistics_template(); + auto space_stats_tmpl = env->space_stats_template(); + auto heap_stats_tmpl = env->v8_heap_statistics_template(); + if (object_stats_template.IsEmpty()) { + std::string_view object_stats_names[] = {"allocated_bytes", "object_count"}; + object_stats_template = + DictionaryTemplate::New(isolate, object_stats_names); + env->set_object_stats_template(object_stats_template); + } + if (page_stats_tmpl.IsEmpty()) { + std::string_view page_stats_names[] = {"committed_size_bytes", + "resident_size_bytes", + "used_size_bytes", + "object_statistics"}; + page_stats_tmpl = DictionaryTemplate::New(isolate, page_stats_names); + env->set_page_stats_template(page_stats_tmpl); + } + if (free_list_statistics_template.IsEmpty()) { + std::string_view free_list_statistics_names[] = { + "bucket_size", "free_count", "free_size"}; + free_list_statistics_template = + DictionaryTemplate::New(isolate, free_list_statistics_names); + env->set_free_list_statistics_template(free_list_statistics_template); + } + if (space_stats_tmpl.IsEmpty()) { + std::string_view space_stats_names[] = {"name", + "committed_size_bytes", + "resident_size_bytes", + "used_size_bytes", + "page_stats", + "free_list_stats"}; + space_stats_tmpl = DictionaryTemplate::New(isolate, space_stats_names); + env->set_space_stats_template(space_stats_tmpl); + } + if (heap_stats_tmpl.IsEmpty()) { + std::string_view heap_statistics_names[] = {"committed_size_bytes", + "resident_size_bytes", + "used_size_bytes", + "space_statistics", + "type_names"}; + heap_stats_tmpl = DictionaryTemplate::New(isolate, heap_statistics_names); + env->set_v8_heap_statistics_template(heap_stats_tmpl); + } + for (size_t i = 0; i < stats.space_stats.size(); i++) { const cppgc::HeapStatistics::SpaceStatistics& space_stats = stats.space_stats[i]; @@ -344,30 +393,18 @@ static MaybeLocal ConvertHeapStatsToJSObject( for (size_t k = 0; k < page_stats.object_statistics.size(); k++) { const cppgc::HeapStatistics::ObjectStatsEntry& object_stats = page_stats.object_statistics[k]; - Local object_stats_names[] = { - FIXED_ONE_BYTE_STRING(isolate, "allocated_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "object_count")}; - Local object_stats_values[] = { + MaybeLocal object_stats_values[] = { Uint32::NewFromUnsigned( isolate, static_cast(object_stats.allocated_bytes)), Uint32::NewFromUnsigned( isolate, static_cast(object_stats.object_count))}; Local object_stats_object = - Object::New(isolate, - Null(isolate), - object_stats_names, - object_stats_values, - arraysize(object_stats_names)); + object_stats_template->NewInstance(context, object_stats_values); object_statistics_array.emplace_back(object_stats_object); } // Set page statistics - Local page_stats_names[] = { - FIXED_ONE_BYTE_STRING(isolate, "committed_size_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "resident_size_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "used_size_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "object_statistics")}; - Local page_stats_values[] = { + MaybeLocal page_stats_values[] = { Uint32::NewFromUnsigned( isolate, static_cast(page_stats.committed_size_bytes)), Uint32::NewFromUnsigned( @@ -377,21 +414,12 @@ static MaybeLocal ConvertHeapStatsToJSObject( Array::New(isolate, object_statistics_array.data(), object_statistics_array.size())}; - Local page_stats_object = - Object::New(isolate, - Null(isolate), - page_stats_names, - page_stats_values, - arraysize(page_stats_names)); - page_statistics_array.emplace_back(page_stats_object); + page_statistics_array.emplace_back( + page_stats_tmpl->NewInstance(context, page_stats_values)); } // Free List Statistics - Local free_list_statistics_names[] = { - FIXED_ONE_BYTE_STRING(isolate, "bucket_size"), - FIXED_ONE_BYTE_STRING(isolate, "free_count"), - FIXED_ONE_BYTE_STRING(isolate, "free_size")}; - Local free_list_statistics_values[] = { + MaybeLocal free_list_statistics_values[] = { ToV8ValuePrimitiveArray( context, space_stats.free_list_stats.bucket_size, isolate), ToV8ValuePrimitiveArray( @@ -400,27 +428,16 @@ static MaybeLocal ConvertHeapStatsToJSObject( context, space_stats.free_list_stats.free_size, isolate)}; Local free_list_statistics_obj = - Object::New(isolate, - Null(isolate), - free_list_statistics_names, - free_list_statistics_values, - arraysize(free_list_statistics_names)); + free_list_statistics_template->NewInstance(context, + free_list_statistics_values); // Set Space Statistics - Local space_stats_names[] = { - FIXED_ONE_BYTE_STRING(isolate, "name"), - FIXED_ONE_BYTE_STRING(isolate, "committed_size_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "resident_size_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "used_size_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "page_stats"), - FIXED_ONE_BYTE_STRING(isolate, "free_list_stats")}; - Local name_value; if (!ToV8Value(context, stats.space_stats[i].name, isolate) .ToLocal(&name_value)) { return MaybeLocal(); } - Local space_stats_values[] = { + MaybeLocal space_stats_values[] = { name_value, Uint32::NewFromUnsigned( isolate, @@ -436,29 +453,16 @@ static MaybeLocal ConvertHeapStatsToJSObject( page_statistics_array.size()), free_list_statistics_obj, }; - Local space_stats_object = - Object::New(isolate, - Null(isolate), - space_stats_names, - space_stats_values, - arraysize(space_stats_names)); - space_statistics_array.emplace_back(space_stats_object); + space_statistics_array.emplace_back( + space_stats_tmpl->NewInstance(context, space_stats_values)); } - // Set heap statistics - Local heap_statistics_names[] = { - FIXED_ONE_BYTE_STRING(isolate, "committed_size_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "resident_size_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "used_size_bytes"), - FIXED_ONE_BYTE_STRING(isolate, "space_statistics"), - FIXED_ONE_BYTE_STRING(isolate, "type_names")}; - Local type_names_value; if (!ToV8Value(context, stats.type_names, isolate) .ToLocal(&type_names_value)) { return MaybeLocal(); } - Local heap_statistics_values[] = { + MaybeLocal heap_statistics_values[] = { Uint32::NewFromUnsigned( isolate, static_cast(stats.committed_size_bytes)), Uint32::NewFromUnsigned(isolate, @@ -470,14 +474,7 @@ static MaybeLocal ConvertHeapStatsToJSObject( space_statistics_array.size()), type_names_value}; - Local heap_statistics_object = - Object::New(isolate, - Null(isolate), - heap_statistics_names, - heap_statistics_values, - arraysize(heap_statistics_names)); - - return heap_statistics_object; + return heap_stats_tmpl->NewInstance(context, heap_statistics_values); } static void GetCppHeapStatistics(const FunctionCallbackInfo& args) { From 58009a7b69c1175fb515ac732c7884fbdee3235b Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 7 Sep 2025 20:44:59 -0700 Subject: [PATCH 4/7] src: update CallSite to use DictionaryTemplate --- src/env_properties.h | 3 ++- src/node_util.cc | 31 ++++++++++++++++++------------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/env_properties.h b/src/env_properties.h index a908f50d538b8b..aa018261f8dcc5 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -457,6 +457,7 @@ V(blob_constructor_template, v8::FunctionTemplate) \ V(blob_reader_constructor_template, v8::FunctionTemplate) \ V(blocklist_constructor_template, v8::FunctionTemplate) \ + V(callsite_template, v8::DictionaryTemplate) \ V(contextify_global_template, v8::ObjectTemplate) \ V(contextify_wrapper_template, v8::ObjectTemplate) \ V(cpu_usage_template, v8::DictionaryTemplate) \ @@ -485,7 +486,7 @@ V(message_port_constructor_template, v8::FunctionTemplate) \ V(module_wrap_constructor_template, v8::FunctionTemplate) \ V(microtask_queue_ctor_template, v8::FunctionTemplate) \ - V(object_stats_template, v8::DictionaryTemplate) \ + V(object_stats_template, v8::DictionaryTemplate) \ V(page_stats_template, v8::DictionaryTemplate) \ V(pipe_constructor_template, v8::FunctionTemplate) \ V(promise_wrap_template, v8::ObjectTemplate) \ diff --git a/src/node_util.cc b/src/node_util.cc index 1972d30b9b3899..5439034b2cac5b 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -15,6 +15,7 @@ using v8::BigInt; using v8::Boolean; using v8::CFunction; using v8::Context; +using v8::DictionaryTemplate; using v8::External; using v8::FunctionCallbackInfo; using v8::IndexFilter; @@ -23,6 +24,7 @@ using v8::Isolate; using v8::KeyCollectionMode; using v8::Local; using v8::LocalVector; +using v8::MaybeLocal; using v8::Name; using v8::Object; using v8::ObjectTemplate; @@ -263,6 +265,19 @@ static void GetCallSites(const FunctionCallbackInfo& args) { const int frame_count = stack->GetFrameCount(); LocalVector callsite_objects(isolate); + auto callsite_template = env->callsite_template(); + if (callsite_template.IsEmpty()) { + std::string_view names[] = {"functionName", + "scriptId", + "scriptName", + "lineNumber", + "columnNumber", + // TODO(legendecas): deprecate CallSite.column. + "column"}; + callsite_template = DictionaryTemplate::New(isolate, names); + env->set_callsite_template(callsite_template); + } + // Frame 0 is node:util. It should be skipped. for (int i = 1; i < frame_count; ++i) { Local stack_frame = stack->GetFrame(isolate, i); @@ -279,16 +294,7 @@ static void GetCallSites(const FunctionCallbackInfo& args) { std::string script_id = std::to_string(stack_frame->GetScriptId()); - Local names[] = { - env->function_name_string(), - env->script_id_string(), - env->script_name_string(), - env->line_number_string(), - env->column_number_string(), - // TODO(legendecas): deprecate CallSite.column. - env->column_string(), - }; - Local values[] = { + MaybeLocal values[] = { function_name, OneByteString(isolate, script_id), script_name, @@ -297,10 +303,9 @@ static void GetCallSites(const FunctionCallbackInfo& args) { // TODO(legendecas): deprecate CallSite.column. Integer::NewFromUnsigned(isolate, stack_frame->GetColumn()), }; - Local obj = Object::New( - isolate, v8::Null(isolate), names, values, arraysize(names)); - callsite_objects.push_back(obj); + callsite_objects.push_back( + callsite_template->NewInstance(env->context(), values)); } Local callsites = From 9f5d58f016a01ca3ef1ec26d4e29d8d792df4616 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 7 Sep 2025 21:23:21 -0700 Subject: [PATCH 5/7] src: update sqlite to use DictionaryTemplate --- src/env_properties.h | 2 + src/node_sqlite.cc | 123 ++++++++---------- .../test-sqlite-statement-sync-columns.js | 11 -- test/parallel/test-sqlite-statement-sync.js | 4 +- 4 files changed, 55 insertions(+), 85 deletions(-) diff --git a/src/env_properties.h b/src/env_properties.h index aa018261f8dcc5..969de0e05de65f 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -480,6 +480,7 @@ V(http2ping_constructor_template, v8::ObjectTemplate) \ V(i18n_converter_template, v8::ObjectTemplate) \ V(intervalhistogram_constructor_template, v8::FunctionTemplate) \ + V(iter_template, v8::DictionaryTemplate) \ V(js_transferable_constructor_template, v8::FunctionTemplate) \ V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \ V(lock_holder_constructor_template, v8::FunctionTemplate) \ @@ -496,6 +497,7 @@ V(shutdown_wrap_template, v8::ObjectTemplate) \ V(socketaddress_constructor_template, v8::FunctionTemplate) \ V(space_stats_template, v8::DictionaryTemplate) \ + V(sqlite_column_template, v8::DictionaryTemplate) \ V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \ V(sqlite_statement_sync_iterator_constructor_template, v8::FunctionTemplate) \ V(sqlite_session_constructor_template, v8::FunctionTemplate) \ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 54620e95d06019..9b21ec66ae0e40 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -24,6 +24,7 @@ using v8::BigInt; using v8::Boolean; using v8::ConstructorBehavior; using v8::Context; +using v8::DictionaryTemplate; using v8::DontDelete; using v8::Exception; using v8::Function; @@ -119,6 +120,18 @@ using v8::Value; } \ } while (0) +namespace { +Local getLazyIterTemplate(Environment* env) { + auto iter_template = env->iter_template(); + if (iter_template.IsEmpty()) { + std::string_view iter_keys[] = {"done", "value"}; + iter_template = DictionaryTemplate::New(env->isolate(), iter_keys); + env->set_iter_template(iter_template); + } + return iter_template; +} +} // namespace + inline MaybeLocal CreateSQLiteError(Isolate* isolate, const char* message) { Local js_msg; @@ -2239,58 +2252,31 @@ void StatementSync::Columns(const FunctionCallbackInfo& args) { int num_cols = sqlite3_column_count(stmt->statement_); Isolate* isolate = env->isolate(); LocalVector cols(isolate); - LocalVector col_keys(isolate, - {env->column_string(), - env->database_string(), - env->name_string(), - env->table_string(), - env->type_string()}); + auto sqlite_column_template = env->sqlite_column_template(); + if (sqlite_column_template.IsEmpty()) { + std::string_view col_keys[] = { + "column", "database", "name", "table", "type"}; + sqlite_column_template = DictionaryTemplate::New(isolate, col_keys); + env->set_sqlite_column_template(sqlite_column_template); + } Local value; cols.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { - LocalVector col_values(isolate); - col_values.reserve(col_keys.size()); - - if (!NullableSQLiteStringToValue( - isolate, sqlite3_column_origin_name(stmt->statement_, i)) - .ToLocal(&value)) { - return; - } - col_values.emplace_back(value); - - if (!NullableSQLiteStringToValue( - isolate, sqlite3_column_database_name(stmt->statement_, i)) - .ToLocal(&value)) { - return; - } - col_values.emplace_back(value); - - if (!stmt->ColumnNameToName(i).ToLocal(&value)) { - return; - } - col_values.emplace_back(value); - - if (!NullableSQLiteStringToValue( - isolate, sqlite3_column_table_name(stmt->statement_, i)) - .ToLocal(&value)) { - return; - } - col_values.emplace_back(value); - - if (!NullableSQLiteStringToValue( - isolate, sqlite3_column_decltype(stmt->statement_, i)) - .ToLocal(&value)) { - return; - } - col_values.emplace_back(value); - - Local column = Object::New(isolate, - Null(isolate), - col_keys.data(), - col_values.data(), - col_keys.size()); - cols.emplace_back(column); + MaybeLocal values[] = { + NullableSQLiteStringToValue( + isolate, sqlite3_column_origin_name(stmt->statement_, i)), + NullableSQLiteStringToValue( + isolate, sqlite3_column_database_name(stmt->statement_, i)), + stmt->ColumnNameToName(i), + NullableSQLiteStringToValue( + isolate, sqlite3_column_table_name(stmt->statement_, i)), + NullableSQLiteStringToValue( + isolate, sqlite3_column_decltype(stmt->statement_, i)), + }; + + cols.emplace_back( + sqlite_column_template->NewInstance(env->context(), values)); } args.GetReturnValue().Set(Array::New(isolate, cols.data(), cols.size())); @@ -2522,15 +2508,16 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { THROW_AND_RETURN_ON_BAD_STATE( env, iter->stmt_->IsFinalized(), "statement has been finalized"); Isolate* isolate = env->isolate(); - LocalVector keys(isolate, {env->done_string(), env->value_string()}); + + auto iter_template = getLazyIterTemplate(env); if (iter->done_) { - LocalVector values(isolate, - {Boolean::New(isolate, true), Null(isolate)}); - DCHECK_EQ(values.size(), keys.size()); - Local result = Object::New( - isolate, Null(isolate), keys.data(), values.data(), keys.size()); - args.GetReturnValue().Set(result); + MaybeLocal values[]{ + Boolean::New(isolate, true), + Null(isolate), + }; + args.GetReturnValue().Set( + iter_template->NewInstance(env->context(), values)); return; } @@ -2539,12 +2526,9 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { CHECK_ERROR_OR_THROW( env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void()); sqlite3_reset(iter->stmt_->statement_); - LocalVector values(isolate, - {Boolean::New(isolate, true), Null(isolate)}); - DCHECK_EQ(values.size(), keys.size()); - Local result = Object::New( - isolate, Null(isolate), keys.data(), values.data(), keys.size()); - args.GetReturnValue().Set(result); + MaybeLocal values[] = {Boolean::New(isolate, true), Null(isolate)}; + args.GetReturnValue().Set( + iter_template->NewInstance(env->context(), values)); return; } @@ -2572,11 +2556,8 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); } - LocalVector values(isolate, {Boolean::New(isolate, false), row_value}); - DCHECK_EQ(keys.size(), values.size()); - Local result = Object::New( - isolate, Null(isolate), keys.data(), values.data(), keys.size()); - args.GetReturnValue().Set(result); + MaybeLocal values[] = {Boolean::New(isolate, false), row_value}; + args.GetReturnValue().Set(iter_template->NewInstance(env->context(), values)); } void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { @@ -2589,13 +2570,11 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { sqlite3_reset(iter->stmt_->statement_); iter->done_ = true; - LocalVector keys(isolate, {env->done_string(), env->value_string()}); - LocalVector values(isolate, - {Boolean::New(isolate, true), Null(isolate)}); - DCHECK_EQ(keys.size(), values.size()); - Local result = Object::New( - isolate, Null(isolate), keys.data(), values.data(), keys.size()); + auto iter_template = getLazyIterTemplate(env); + MaybeLocal values[] = {Boolean::New(isolate, true), Null(isolate)}; + + Local result = iter_template->NewInstance(env->context(), values); args.GetReturnValue().Set(result); } diff --git a/test/parallel/test-sqlite-statement-sync-columns.js b/test/parallel/test-sqlite-statement-sync-columns.js index a0c3fbd74347de..8121464bd67d97 100644 --- a/test/parallel/test-sqlite-statement-sync-columns.js +++ b/test/parallel/test-sqlite-statement-sync-columns.js @@ -18,7 +18,6 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT col1, col2, col3, col4, col5 FROM test'); assert.deepStrictEqual(stmt.columns(), [ { - __proto__: null, column: 'col1', database: 'main', name: 'col1', @@ -26,7 +25,6 @@ suite('StatementSync.prototype.columns()', () => { type: 'INTEGER', }, { - __proto__: null, column: 'col2', database: 'main', name: 'col2', @@ -34,7 +32,6 @@ suite('StatementSync.prototype.columns()', () => { type: 'REAL', }, { - __proto__: null, column: 'col3', database: 'main', name: 'col3', @@ -42,7 +39,6 @@ suite('StatementSync.prototype.columns()', () => { type: 'TEXT', }, { - __proto__: null, column: 'col4', database: 'main', name: 'col4', @@ -50,7 +46,6 @@ suite('StatementSync.prototype.columns()', () => { type: 'BLOB', }, { - __proto__: null, column: 'col5', database: 'main', name: 'col5', @@ -69,7 +64,6 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT value1, value2 FROM test1, test2'); assert.deepStrictEqual(stmt.columns(), [ { - __proto__: null, column: 'value1', database: 'main', name: 'value1', @@ -77,7 +71,6 @@ suite('StatementSync.prototype.columns()', () => { type: 'INTEGER', }, { - __proto__: null, column: 'value2', database: 'main', name: 'value2', @@ -93,7 +86,6 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT value AS foo FROM test'); assert.deepStrictEqual(stmt.columns(), [ { - __proto__: null, column: 'value', database: 'main', name: 'foo', @@ -109,7 +101,6 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT value + 1, value FROM test'); assert.deepStrictEqual(stmt.columns(), [ { - __proto__: null, column: null, database: null, name: 'value + 1', @@ -117,7 +108,6 @@ suite('StatementSync.prototype.columns()', () => { type: null, }, { - __proto__: null, column: 'value', database: 'main', name: 'value', @@ -133,7 +123,6 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT * FROM (SELECT * FROM test)'); assert.deepStrictEqual(stmt.columns(), [ { - __proto__: null, column: 'value', database: 'main', name: 'value', diff --git a/test/parallel/test-sqlite-statement-sync.js b/test/parallel/test-sqlite-statement-sync.js index 04494a02c692a8..db6b2a3aa783d5 100644 --- a/test/parallel/test-sqlite-statement-sync.js +++ b/test/parallel/test-sqlite-statement-sync.js @@ -168,7 +168,7 @@ suite('StatementSync.prototype.iterate()', () => { ]); t.assert.deepStrictEqual( iterator.next(), - { __proto__: null, done: true, value: null }, + { done: true, value: null }, ); }); }); @@ -561,7 +561,7 @@ suite('StatementSync.prototype.iterate() with array output', () => { ]); t.assert.deepStrictEqual( iterator.next(), - { __proto__: null, done: true, value: null }, + { done: true, value: null }, ); }); }); From 83886886ab655ce595b3d135e6c770041b961903 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 7 Sep 2025 22:19:33 -0700 Subject: [PATCH 6/7] src: remove obsolete/unused env env_properties Had a bunch of stuff in env_properties that was no longer used that hadn't been cleaned up. --- src/env_properties.h | 49 +------------------------------------------- 1 file changed, 1 insertion(+), 48 deletions(-) diff --git a/src/env_properties.h b/src/env_properties.h index 969de0e05de65f..6236c565e9a8e0 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -83,7 +83,6 @@ V(base_string, "base") \ V(base_url_string, "baseURL") \ V(bits_string, "bits") \ - V(block_list_string, "blockList") \ V(buffer_string, "buffer") \ V(bytes_parsed_string, "bytesParsed") \ V(bytes_read_string, "bytesRead") \ @@ -92,11 +91,9 @@ V(cached_data_produced_string, "cachedDataProduced") \ V(cached_data_rejected_string, "cachedDataRejected") \ V(cached_data_string, "cachedData") \ - V(cache_key_string, "cacheKey") \ V(cert_usage_string, "certUsage") \ V(change_string, "change") \ V(changes_string, "changes") \ - V(channel_string, "channel") \ V(chunks_sent_since_last_write_string, "chunksSentSinceLastWrite") \ V(client_id_string, "clientId") \ V(clone_unsupported_type_str, "Cannot clone object of unsupported type.") \ @@ -105,9 +102,6 @@ "transferList") \ V(clone_untransferable_str, "Found invalid value in transferList.") \ V(code_string, "code") \ - V(column_number_string, "columnNumber") \ - V(column_string, "column") \ - V(commonjs_string, "commonjs") \ V(config_string, "config") \ V(constants_string, "constants") \ V(crypto_dh_string, "dh") \ @@ -139,7 +133,6 @@ V(crypto_rsa_pss_string, "rsa-pss") \ V(cwd_string, "cwd") \ V(data_string, "data") \ - V(database_string, "database") \ V(default_is_true_string, "defaultIsTrue") \ V(deserialize_info_string, "deserializeInfo") \ V(dest_string, "dest") \ @@ -166,10 +159,8 @@ V(ecdh_string, "ECDH") \ V(emit_string, "emit") \ V(emit_warning_string, "emitWarning") \ - V(empty_object_string, "{}") \ V(encoding_string, "encoding") \ V(entries_string, "entries") \ - V(entry_type_string, "entryType") \ V(env_pairs_string, "envPairs") \ V(env_var_settings_string, "envVarSettings") \ V(err_sqlite_error_string, "ERR_SQLITE_ERROR") \ @@ -197,10 +188,8 @@ V(fingerprint_string, "fingerprint") \ V(flags_string, "flags") \ V(flowlabel_string, "flowlabel") \ - V(fragment_string, "fragment") \ V(frames_received_string, "framesReceived") \ V(frames_sent_string, "framesSent") \ - V(function_name_string, "functionName") \ V(function_string, "function") \ V(get_string, "get") \ V(get_data_clone_error_string, "_getDataCloneError") \ @@ -228,14 +217,10 @@ V(infoaccess_string, "infoAccess") \ V(inherit_string, "inherit") \ V(input_string, "input") \ - V(inputs_string, "inputs") \ - V(internal_binding_string, "internalBinding") \ - V(internal_string, "internal") \ V(inverse_string, "inverse") \ V(ipv4_string, "IPv4") \ V(ipv6_string, "IPv6") \ V(isclosing_string, "isClosing") \ - V(isfinished_string, "isFinished") \ V(issuer_string, "issuer") \ V(issuercert_string, "issuerCertificate") \ V(iterator_string, "Iterator") \ @@ -267,7 +252,6 @@ V(last_insert_rowid_string, "lastInsertRowid") \ V(length_string, "length") \ V(library_string, "library") \ - V(line_number_string, "lineNumber") \ V(loop_count, "loopCount") \ V(mac_string, "mac") \ V(match_string, "match") \ @@ -285,12 +269,10 @@ V(modulus_length_string, "modulusLength") \ V(name_string, "name") \ V(named_curve_string, "namedCurve") \ - V(netmask_string, "netmask") \ V(next_string, "next") \ V(nistcurve_string, "nistCurve") \ V(node_string, "node") \ V(nsname_string, "nsname") \ - V(num_cols_string, "num_cols") \ V(object_string, "Object") \ V(ocsp_request_string, "OCSPRequest") \ V(oncertcb_string, "oncertcb") \ @@ -348,7 +330,6 @@ V(psk_string, "psk") \ V(pubkey_string, "pubkey") \ V(public_exponent_string, "publicExponent") \ - V(query_string, "query") \ V(rate_string, "rate") \ V(raw_string, "raw") \ V(read_host_object_string, "_readHostObject") \ @@ -366,17 +347,11 @@ "export * from 'original'; export { default } from 'original'; export " \ "const __esModule = true;") \ V(require_string, "require") \ - V(resolve_string, "resolve") \ V(resource_string, "resource") \ V(result_string, "result") \ V(retry_string, "retry") \ V(return_arrays_string, "returnArrays") \ - V(return_string, "return") \ V(salt_length_string, "saltLength") \ - V(scheme_string, "scheme") \ - V(scopeid_string, "scopeid") \ - V(script_id_string, "scriptId") \ - V(script_name_string, "scriptName") \ V(search_string, "search") \ V(selector_string, "selector") \ V(serial_number_string, "serialNumber") \ @@ -398,9 +373,7 @@ V(stack_string, "stack") \ V(standard_name_string, "standardName") \ V(start_string, "start") \ - V(start_time_string, "startTime") \ V(state_string, "state") \ - V(statement_string, "statement") \ V(stats_string, "stats") \ V(status_string, "status") \ V(stdio_string, "stdio") \ @@ -428,12 +401,6 @@ V(type_string, "type") \ V(uid_string, "uid") \ V(unknown_string, "") \ - V(url_special_ftp_string, "ftp:") \ - V(url_special_file_string, "file:") \ - V(url_special_http_string, "http:") \ - V(url_special_https_string, "https:") \ - V(url_special_ws_string, "ws:") \ - V(url_special_wss_string, "wss:") \ V(url_string, "url") \ V(username_string, "username") \ V(valid_from_string, "valid_from") \ @@ -447,12 +414,10 @@ V(wrap_string, "wrap") \ V(writable_string, "writable") \ V(write_host_object_string, "_writeHostObject") \ - V(write_queue_size_string, "writeQueueSize") \ - V(x_forwarded_string, "x-forwarded-for") + V(write_queue_size_string, "writeQueueSize") #define PER_ISOLATE_TEMPLATE_PROPERTIES(V) \ V(async_wrap_ctor_template, v8::FunctionTemplate) \ - V(async_wrap_object_ctor_template, v8::FunctionTemplate) \ V(binding_data_default_template, v8::ObjectTemplate) \ V(blob_constructor_template, v8::FunctionTemplate) \ V(blob_reader_constructor_template, v8::FunctionTemplate) \ @@ -467,7 +432,6 @@ V(dir_instance_template, v8::ObjectTemplate) \ V(fd_constructor_template, v8::ObjectTemplate) \ V(fdclose_constructor_template, v8::ObjectTemplate) \ - V(fdentry_constructor_template, v8::FunctionTemplate) \ V(filehandlereadwrap_template, v8::ObjectTemplate) \ V(free_list_statistics_template, v8::DictionaryTemplate) \ V(fsreqpromise_constructor_template, v8::ObjectTemplate) \ @@ -486,12 +450,9 @@ V(lock_holder_constructor_template, v8::FunctionTemplate) \ V(message_port_constructor_template, v8::FunctionTemplate) \ V(module_wrap_constructor_template, v8::FunctionTemplate) \ - V(microtask_queue_ctor_template, v8::FunctionTemplate) \ V(object_stats_template, v8::DictionaryTemplate) \ V(page_stats_template, v8::DictionaryTemplate) \ V(pipe_constructor_template, v8::FunctionTemplate) \ - V(promise_wrap_template, v8::ObjectTemplate) \ - V(sab_lifetimepartner_constructor_template, v8::FunctionTemplate) \ V(script_context_constructor_template, v8::FunctionTemplate) \ V(secure_context_constructor_template, v8::FunctionTemplate) \ V(shutdown_wrap_template, v8::ObjectTemplate) \ @@ -501,11 +462,7 @@ V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \ V(sqlite_statement_sync_iterator_constructor_template, v8::FunctionTemplate) \ V(sqlite_session_constructor_template, v8::FunctionTemplate) \ - V(streambaseentry_ctor_template, v8::FunctionTemplate) \ V(streambaseoutputstream_constructor_template, v8::ObjectTemplate) \ - V(streamentry_ctor_template, v8::FunctionTemplate) \ - V(streamentry_opaque_ctor_template, v8::FunctionTemplate) \ - V(qlogoutputstream_constructor_template, v8::ObjectTemplate) \ V(tcp_constructor_template, v8::FunctionTemplate) \ V(tty_constructor_template, v8::FunctionTemplate) \ V(urlpatternresult_template, v8::DictionaryTemplate) \ @@ -525,11 +482,9 @@ V(async_hooks_init_function, v8::Function) \ V(async_hooks_promise_resolve_function, v8::Function) \ V(buffer_prototype_object, v8::Object) \ - V(crypto_key_object_constructor, v8::Function) \ V(crypto_key_object_private_constructor, v8::Function) \ V(crypto_key_object_public_constructor, v8::Function) \ V(crypto_key_object_secret_constructor, v8::Function) \ - V(domexception_function, v8::Function) \ V(enhance_fatal_stack_after_inspector, v8::Function) \ V(enhance_fatal_stack_before_inspector, v8::Function) \ V(get_source_map_error_source, v8::Function) \ @@ -566,7 +521,6 @@ V(primordials_safe_set_prototype_object, v8::Object) \ V(primordials_safe_weak_map_prototype_object, v8::Object) \ V(primordials_safe_weak_set_prototype_object, v8::Object) \ - V(promise_hook_handler, v8::Function) \ V(promise_reject_callback, v8::Function) \ V(snapshot_serialize_callback, v8::Function) \ V(snapshot_deserialize_callback, v8::Function) \ @@ -577,7 +531,6 @@ V(tls_wrap_constructor_function, v8::Function) \ V(trace_category_state_function, v8::Function) \ V(udp_constructor_function, v8::Function) \ - V(url_constructor_function, v8::Function) \ V(wasm_streaming_compilation_impl, v8::Function) \ V(wasm_streaming_object_constructor, v8::Function) From 384fad86a7940befb6314eb98c97733473ef1555 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 7 Sep 2025 22:26:07 -0700 Subject: [PATCH 7/7] src: fixup lint issues after dictionary template change --- src/node_sqlite.cc | 41 +++++++--- src/node_url_pattern.cc | 12 +-- src/node_util.cc | 24 +++--- src/node_v8.cc | 76 ++++++++++++------- src/node_worker.cc | 24 +++--- src/util-inl.h | 25 ++++++ src/util.h | 14 ++++ .../test-sqlite-statement-sync-columns.js | 11 +++ test/parallel/test-sqlite-statement-sync.js | 4 +- 9 files changed, 164 insertions(+), 67 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 9b21ec66ae0e40..924ed5789ba364 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -124,7 +124,7 @@ namespace { Local getLazyIterTemplate(Environment* env) { auto iter_template = env->iter_template(); if (iter_template.IsEmpty()) { - std::string_view iter_keys[] = {"done", "value"}; + static constexpr std::string_view iter_keys[] = {"done", "value"}; iter_template = DictionaryTemplate::New(env->isolate(), iter_keys); env->set_iter_template(iter_template); } @@ -2254,12 +2254,11 @@ void StatementSync::Columns(const FunctionCallbackInfo& args) { LocalVector cols(isolate); auto sqlite_column_template = env->sqlite_column_template(); if (sqlite_column_template.IsEmpty()) { - std::string_view col_keys[] = { + static constexpr std::string_view col_keys[] = { "column", "database", "name", "table", "type"}; sqlite_column_template = DictionaryTemplate::New(isolate, col_keys); env->set_sqlite_column_template(sqlite_column_template); } - Local value; cols.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { @@ -2275,8 +2274,13 @@ void StatementSync::Columns(const FunctionCallbackInfo& args) { isolate, sqlite3_column_decltype(stmt->statement_, i)), }; - cols.emplace_back( - sqlite_column_template->NewInstance(env->context(), values)); + Local col; + if (!NewDictionaryInstanceNullProto( + env->context(), sqlite_column_template, values) + .ToLocal(&col)) { + return; + } + cols.emplace_back(col); } args.GetReturnValue().Set(Array::New(isolate, cols.data(), cols.size())); @@ -2516,8 +2520,11 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { Boolean::New(isolate, true), Null(isolate), }; - args.GetReturnValue().Set( - iter_template->NewInstance(env->context(), values)); + Local result; + if (NewDictionaryInstanceNullProto(env->context(), iter_template, values) + .ToLocal(&result)) { + args.GetReturnValue().Set(result); + } return; } @@ -2527,8 +2534,11 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void()); sqlite3_reset(iter->stmt_->statement_); MaybeLocal values[] = {Boolean::New(isolate, true), Null(isolate)}; - args.GetReturnValue().Set( - iter_template->NewInstance(env->context(), values)); + Local result; + if (NewDictionaryInstanceNullProto(env->context(), iter_template, values) + .ToLocal(&result)) { + args.GetReturnValue().Set(result); + } return; } @@ -2557,7 +2567,11 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { } MaybeLocal values[] = {Boolean::New(isolate, false), row_value}; - args.GetReturnValue().Set(iter_template->NewInstance(env->context(), values)); + Local result; + if (NewDictionaryInstanceNullProto(env->context(), iter_template, values) + .ToLocal(&result)) { + args.GetReturnValue().Set(result); + } } void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { @@ -2574,8 +2588,11 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { auto iter_template = getLazyIterTemplate(env); MaybeLocal values[] = {Boolean::New(isolate, true), Null(isolate)}; - Local result = iter_template->NewInstance(env->context(), values); - args.GetReturnValue().Set(result); + Local result; + if (NewDictionaryInstanceNullProto(env->context(), iter_template, values) + .ToLocal(&result)) { + args.GetReturnValue().Set(result); + } } Session::Session(Environment* env, diff --git a/src/node_url_pattern.cc b/src/node_url_pattern.cc index 551989c814df04..f1bddaeab0260e 100644 --- a/src/node_url_pattern.cc +++ b/src/node_url_pattern.cc @@ -61,7 +61,6 @@ using v8::FunctionTemplate; using v8::Global; using v8::Isolate; using v8::Local; -using v8::LocalVector; using v8::MaybeLocal; using v8::Name; using v8::NewStringType; @@ -400,7 +399,7 @@ MaybeLocal URLPattern::URLPatternResult::ToJSValue( auto tmpl = env->urlpatternresult_template(); if (tmpl.IsEmpty()) { - std::string_view namesVec[] = { + static constexpr std::string_view namesVec[] = { "inputs", "protocol", "username", @@ -416,10 +415,8 @@ MaybeLocal URLPattern::URLPatternResult::ToJSValue( } size_t index = 0; - auto context = isolate->GetCurrentContext(); - MaybeLocal vals[] = { - Array::New(context, + Array::New(env->context(), result.inputs.size(), [&index, &inputs = result.inputs, env]() { auto& input = inputs[index++]; @@ -440,9 +437,8 @@ MaybeLocal URLPattern::URLPatternResult::ToJSValue( URLPatternComponentResult::ToJSObject(env, result.port), URLPatternComponentResult::ToJSObject(env, result.pathname), URLPatternComponentResult::ToJSObject(env, result.search), - URLPatternComponentResult::ToJSObject(env, result.hash), - }; - return tmpl->NewInstance(env->context(), vals); + URLPatternComponentResult::ToJSObject(env, result.hash)}; + return NewDictionaryInstanceNullProto(env->context(), tmpl, vals); } std::optional diff --git a/src/node_util.cc b/src/node_util.cc index 5439034b2cac5b..36bd7c0028153a 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -267,13 +267,14 @@ static void GetCallSites(const FunctionCallbackInfo& args) { auto callsite_template = env->callsite_template(); if (callsite_template.IsEmpty()) { - std::string_view names[] = {"functionName", - "scriptId", - "scriptName", - "lineNumber", - "columnNumber", - // TODO(legendecas): deprecate CallSite.column. - "column"}; + static constexpr std::string_view names[] = { + "functionName", + "scriptId", + "scriptName", + "lineNumber", + "columnNumber", + // TODO(legendecas): deprecate CallSite.column. + "column"}; callsite_template = DictionaryTemplate::New(isolate, names); env->set_callsite_template(callsite_template); } @@ -304,8 +305,13 @@ static void GetCallSites(const FunctionCallbackInfo& args) { Integer::NewFromUnsigned(isolate, stack_frame->GetColumn()), }; - callsite_objects.push_back( - callsite_template->NewInstance(env->context(), values)); + Local callsite; + if (!NewDictionaryInstanceNullProto( + env->context(), callsite_template, values) + .ToLocal(&callsite)) { + return; + } + callsite_objects.push_back(callsite); } Local callsites = diff --git a/src/node_v8.cc b/src/node_v8.cc index 0b6f5aa258c004..4d2c86e2da7429 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -47,7 +47,6 @@ using v8::Isolate; using v8::Local; using v8::LocalVector; using v8::MaybeLocal; -using v8::Name; using v8::Object; using v8::ScriptCompiler; using v8::String; @@ -338,16 +337,18 @@ static MaybeLocal ConvertHeapStatsToJSObject( auto space_stats_tmpl = env->space_stats_template(); auto heap_stats_tmpl = env->v8_heap_statistics_template(); if (object_stats_template.IsEmpty()) { - std::string_view object_stats_names[] = {"allocated_bytes", "object_count"}; + static constexpr std::string_view object_stats_names[] = {"allocated_bytes", + "object_count"}; object_stats_template = DictionaryTemplate::New(isolate, object_stats_names); env->set_object_stats_template(object_stats_template); } if (page_stats_tmpl.IsEmpty()) { - std::string_view page_stats_names[] = {"committed_size_bytes", - "resident_size_bytes", - "used_size_bytes", - "object_statistics"}; + static constexpr std::string_view page_stats_names[] = { + "committed_size_bytes", + "resident_size_bytes", + "used_size_bytes", + "object_statistics"}; page_stats_tmpl = DictionaryTemplate::New(isolate, page_stats_names); env->set_page_stats_template(page_stats_tmpl); } @@ -359,21 +360,23 @@ static MaybeLocal ConvertHeapStatsToJSObject( env->set_free_list_statistics_template(free_list_statistics_template); } if (space_stats_tmpl.IsEmpty()) { - std::string_view space_stats_names[] = {"name", - "committed_size_bytes", - "resident_size_bytes", - "used_size_bytes", - "page_stats", - "free_list_stats"}; + static constexpr std::string_view space_stats_names[] = { + "name", + "committed_size_bytes", + "resident_size_bytes", + "used_size_bytes", + "page_stats", + "free_list_stats"}; space_stats_tmpl = DictionaryTemplate::New(isolate, space_stats_names); env->set_space_stats_template(space_stats_tmpl); } if (heap_stats_tmpl.IsEmpty()) { - std::string_view heap_statistics_names[] = {"committed_size_bytes", - "resident_size_bytes", - "used_size_bytes", - "space_statistics", - "type_names"}; + static constexpr std::string_view heap_statistics_names[] = { + "committed_size_bytes", + "resident_size_bytes", + "used_size_bytes", + "space_statistics", + "type_names"}; heap_stats_tmpl = DictionaryTemplate::New(isolate, heap_statistics_names); env->set_v8_heap_statistics_template(heap_stats_tmpl); } @@ -398,8 +401,12 @@ static MaybeLocal ConvertHeapStatsToJSObject( isolate, static_cast(object_stats.allocated_bytes)), Uint32::NewFromUnsigned( isolate, static_cast(object_stats.object_count))}; - Local object_stats_object = - object_stats_template->NewInstance(context, object_stats_values); + Local object_stats_object; + if (!NewDictionaryInstanceNullProto( + context, object_stats_template, object_stats_values) + .ToLocal(&object_stats_object)) { + return MaybeLocal(); + } object_statistics_array.emplace_back(object_stats_object); } @@ -414,8 +421,13 @@ static MaybeLocal ConvertHeapStatsToJSObject( Array::New(isolate, object_statistics_array.data(), object_statistics_array.size())}; - page_statistics_array.emplace_back( - page_stats_tmpl->NewInstance(context, page_stats_values)); + Local page_stats_object; + if (!NewDictionaryInstanceNullProto( + context, page_stats_tmpl, page_stats_values) + .ToLocal(&page_stats_object)) { + return MaybeLocal(); + } + page_statistics_array.emplace_back(page_stats_object); } // Free List Statistics @@ -427,9 +439,13 @@ static MaybeLocal ConvertHeapStatsToJSObject( ToV8ValuePrimitiveArray( context, space_stats.free_list_stats.free_size, isolate)}; - Local free_list_statistics_obj = - free_list_statistics_template->NewInstance(context, - free_list_statistics_values); + Local free_list_statistics_obj; + if (!NewDictionaryInstanceNullProto(context, + free_list_statistics_template, + free_list_statistics_values) + .ToLocal(&free_list_statistics_obj)) { + return MaybeLocal(); + } // Set Space Statistics Local name_value; @@ -453,8 +469,13 @@ static MaybeLocal ConvertHeapStatsToJSObject( page_statistics_array.size()), free_list_statistics_obj, }; - space_statistics_array.emplace_back( - space_stats_tmpl->NewInstance(context, space_stats_values)); + Local space_stats_object; + if (!NewDictionaryInstanceNullProto( + context, space_stats_tmpl, space_stats_values) + .ToLocal(&space_stats_object)) { + return MaybeLocal(); + } + space_statistics_array.emplace_back(space_stats_object); } Local type_names_value; @@ -474,7 +495,8 @@ static MaybeLocal ConvertHeapStatsToJSObject( space_statistics_array.size()), type_names_value}; - return heap_stats_tmpl->NewInstance(context, heap_statistics_values); + return NewDictionaryInstanceNullProto( + context, heap_stats_tmpl, heap_statistics_values); } static void GetCppHeapStatistics(const FunctionCallbackInfo& args) { diff --git a/src/node_worker.cc b/src/node_worker.cc index 03fba36a098128..9518ab9d812f21 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -32,13 +32,13 @@ using v8::Float64Array; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::HandleScope; +using v8::HeapStatistics; using v8::Integer; using v8::Isolate; using v8::Local; using v8::Locker; using v8::Maybe; using v8::MaybeLocal; -using v8::Name; using v8::NewStringType; using v8::Null; using v8::Number; @@ -878,7 +878,7 @@ void Worker::CpuUsage(const FunctionCallbackInfo& args) { } else { auto tmpl = env->cpu_usage_template(); if (tmpl.IsEmpty()) { - std::string_view names[] = { + static constexpr std::string_view names[] = { "user", "system", }; @@ -894,7 +894,10 @@ void Worker::CpuUsage(const FunctionCallbackInfo& args) { 1e6 * cpu_usage_stats->ru_stime.tv_sec + cpu_usage_stats->ru_stime.tv_usec), }; - argv[1] = tmpl->NewInstance(env->context(), values); + if (!NewDictionaryInstanceNullProto(env->context(), tmpl, values) + .ToLocal(&argv[1])) { + return; + } } taker->MakeCallback(env->ondone_string(), arraysize(argv), argv); @@ -1063,7 +1066,7 @@ void Worker::GetHeapStatistics(const FunctionCallbackInfo& args) { env](Environment* worker_env) mutable { // We create a unique pointer to HeapStatistics so that the actual object // it's not copied in the lambda, but only the pointer is. - auto heap_stats = std::make_unique(); + auto heap_stats = std::make_unique(); worker_env->isolate()->GetHeapStatistics(heap_stats.get()); // Here, the worker thread temporarily owns the WorkerHeapStatisticsTaker @@ -1096,7 +1099,7 @@ void Worker::GetHeapStatistics(const FunctionCallbackInfo& args) { "used_global_handles_size", "external_memory", }; - tmpl = v8::DictionaryTemplate::New(isolate, heap_stats_names); + tmpl = DictionaryTemplate::New(isolate, heap_stats_names); env->set_heap_statistics_template(tmpl); } @@ -1117,10 +1120,13 @@ void Worker::GetHeapStatistics(const FunctionCallbackInfo& args) { Number::New(isolate, heap_stats->used_global_handles_size()), Number::New(isolate, heap_stats->external_memory())}; - DCHECK_EQ(arraysize(heap_stats_names), arraysize(heap_stats_values)); - - Local args[] = { - tmpl->NewInstance(env->context(), heap_stats_values)}; + Local obj; + if (!NewDictionaryInstanceNullProto( + env->context(), tmpl, heap_stats_values) + .ToLocal(&obj)) { + return; + } + Local args[] = {obj}; taker->get()->MakeCallback( env->ondone_string(), arraysize(args), args); // implicitly delete `taker` diff --git a/src/util-inl.h b/src/util-inl.h index 778cc57537a966..fbce06d7cef9c2 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -704,6 +704,31 @@ inline std::wstring ConvertToWideString(const std::string& str, } #endif // _WIN32 +inline v8::MaybeLocal NewDictionaryInstance( + v8::Local context, + v8::Local tmpl, + v8::MemorySpan> property_values) { + for (auto& value : property_values) { + if (value.IsEmpty()) return v8::MaybeLocal(); + } + return tmpl->NewInstance(context, property_values); +} + +inline v8::MaybeLocal NewDictionaryInstanceNullProto( + v8::Local context, + v8::Local tmpl, + v8::MemorySpan> property_values) { + for (auto& value : property_values) { + if (value.IsEmpty()) return v8::MaybeLocal(); + } + v8::Local obj = tmpl->NewInstance(context, property_values); + if (obj->SetPrototypeV2(context, v8::Null(context->GetIsolate())) + .IsNothing()) { + return v8::MaybeLocal(); + } + return obj; +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/util.h b/src/util.h index e7f35b678f85af..b6f49bcf8e7eab 100644 --- a/src/util.h +++ b/src/util.h @@ -1040,6 +1040,20 @@ inline bool IsWindowsBatchFile(const char* filename); inline std::wstring ConvertToWideString(const std::string& str, UINT code_page); #endif // _WIN32 +// A helper to create a new instance of the dictionary template. +// Unlike v8::DictionaryTemplate::NewInstance, this method will +// check that all properties have been set (are not empty MaybeLocals) +// or will return early with an empty MaybeLocal under the assumption +// that an error has been thrown. +inline v8::MaybeLocal NewDictionaryInstance( + v8::Local context, + v8::Local tmpl, + v8::MemorySpan> property_values); +inline v8::MaybeLocal NewDictionaryInstanceNullProto( + v8::Local context, + v8::Local tmpl, + v8::MemorySpan> property_values); + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/test/parallel/test-sqlite-statement-sync-columns.js b/test/parallel/test-sqlite-statement-sync-columns.js index 8121464bd67d97..a0c3fbd74347de 100644 --- a/test/parallel/test-sqlite-statement-sync-columns.js +++ b/test/parallel/test-sqlite-statement-sync-columns.js @@ -18,6 +18,7 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT col1, col2, col3, col4, col5 FROM test'); assert.deepStrictEqual(stmt.columns(), [ { + __proto__: null, column: 'col1', database: 'main', name: 'col1', @@ -25,6 +26,7 @@ suite('StatementSync.prototype.columns()', () => { type: 'INTEGER', }, { + __proto__: null, column: 'col2', database: 'main', name: 'col2', @@ -32,6 +34,7 @@ suite('StatementSync.prototype.columns()', () => { type: 'REAL', }, { + __proto__: null, column: 'col3', database: 'main', name: 'col3', @@ -39,6 +42,7 @@ suite('StatementSync.prototype.columns()', () => { type: 'TEXT', }, { + __proto__: null, column: 'col4', database: 'main', name: 'col4', @@ -46,6 +50,7 @@ suite('StatementSync.prototype.columns()', () => { type: 'BLOB', }, { + __proto__: null, column: 'col5', database: 'main', name: 'col5', @@ -64,6 +69,7 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT value1, value2 FROM test1, test2'); assert.deepStrictEqual(stmt.columns(), [ { + __proto__: null, column: 'value1', database: 'main', name: 'value1', @@ -71,6 +77,7 @@ suite('StatementSync.prototype.columns()', () => { type: 'INTEGER', }, { + __proto__: null, column: 'value2', database: 'main', name: 'value2', @@ -86,6 +93,7 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT value AS foo FROM test'); assert.deepStrictEqual(stmt.columns(), [ { + __proto__: null, column: 'value', database: 'main', name: 'foo', @@ -101,6 +109,7 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT value + 1, value FROM test'); assert.deepStrictEqual(stmt.columns(), [ { + __proto__: null, column: null, database: null, name: 'value + 1', @@ -108,6 +117,7 @@ suite('StatementSync.prototype.columns()', () => { type: null, }, { + __proto__: null, column: 'value', database: 'main', name: 'value', @@ -123,6 +133,7 @@ suite('StatementSync.prototype.columns()', () => { const stmt = db.prepare('SELECT * FROM (SELECT * FROM test)'); assert.deepStrictEqual(stmt.columns(), [ { + __proto__: null, column: 'value', database: 'main', name: 'value', diff --git a/test/parallel/test-sqlite-statement-sync.js b/test/parallel/test-sqlite-statement-sync.js index db6b2a3aa783d5..04494a02c692a8 100644 --- a/test/parallel/test-sqlite-statement-sync.js +++ b/test/parallel/test-sqlite-statement-sync.js @@ -168,7 +168,7 @@ suite('StatementSync.prototype.iterate()', () => { ]); t.assert.deepStrictEqual( iterator.next(), - { done: true, value: null }, + { __proto__: null, done: true, value: null }, ); }); }); @@ -561,7 +561,7 @@ suite('StatementSync.prototype.iterate() with array output', () => { ]); t.assert.deepStrictEqual( iterator.next(), - { done: true, value: null }, + { __proto__: null, done: true, value: null }, ); }); });