From ccc76bbfd76a305f2cea12c80356295163813f45 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 23 Feb 2024 02:50:27 +0100 Subject: [PATCH] src: simplify embedder entry point execution Previously we wrapped the embedder entry point callback into a binding and then invoke the binding from JS land which was a bit convoluted. Now we just call it directly from C++. The main scripts that needed to tail call the embedder callback now return the arguments in an array so that the C++ land can extract the arguments and pass them to the callback. We also set `PauseOnNextJavascriptStatement()` for --inspect-brk and mark the bootstrap complete milestone directly in C++ for these execution modes. PR-URL: https://github.com/nodejs/node/pull/51557 Reviewed-By: Daniel Lemire Reviewed-By: Chengzhong Wu --- lib/internal/main/embedding.js | 5 +-- lib/internal/main/mksnapshot.js | 15 ++------- src/env-inl.h | 8 ----- src/env.h | 4 --- src/node.cc | 60 ++++++++++++++++++++++++++++----- src/node_snapshotable.cc | 22 ------------ 6 files changed, 54 insertions(+), 60 deletions(-) diff --git a/lib/internal/main/embedding.js b/lib/internal/main/embedding.js index a1076ccb82a46b..cc7cb0eee9d837 100644 --- a/lib/internal/main/embedding.js +++ b/lib/internal/main/embedding.js @@ -1,18 +1,15 @@ 'use strict'; const { prepareMainThreadExecution, - markBootstrapComplete, } = require('internal/process/pre_execution'); const { isExperimentalSeaWarningNeeded } = internalBinding('sea'); const { emitExperimentalWarning } = require('internal/util'); const { embedderRequire, embedderRunCjs } = require('internal/util/embedding'); -const { runEmbedderEntryPoint } = internalBinding('mksnapshot'); prepareMainThreadExecution(false, true); -markBootstrapComplete(); if (isExperimentalSeaWarningNeeded()) { emitExperimentalWarning('Single executable application'); } -return runEmbedderEntryPoint(process, embedderRequire, embedderRunCjs); +return [process, embedderRequire, embedderRunCjs]; diff --git a/lib/internal/main/mksnapshot.js b/lib/internal/main/mksnapshot.js index f00bf886153e6e..0d00acd6a168ba 100644 --- a/lib/internal/main/mksnapshot.js +++ b/lib/internal/main/mksnapshot.js @@ -11,7 +11,6 @@ const { const { BuiltinModule: { normalizeRequirableId } } = require('internal/bootstrap/realm'); const { - runEmbedderEntryPoint, compileSerializeMain, anonymousMainPath, } = internalBinding('mksnapshot'); @@ -20,9 +19,6 @@ const { isExperimentalSeaWarningNeeded } = internalBinding('sea'); const { emitExperimentalWarning } = require('internal/util'); const { emitWarningSync } = require('internal/process/warning'); -const { - getOptionValue, -} = require('internal/options'); const { initializeCallbacks, @@ -179,18 +175,11 @@ function main() { return fn(requireForUserSnapshot, filename, dirname); } - const serializeMainArgs = [process, requireForUserSnapshot, minimalRunCjs]; - if (isExperimentalSeaWarningNeeded()) { emitExperimentalWarning('Single executable application'); } - if (getOptionValue('--inspect-brk')) { - internalBinding('inspector').callAndPauseOnStart( - runEmbedderEntryPoint, undefined, ...serializeMainArgs); - } else { - runEmbedderEntryPoint(...serializeMainArgs); - } + return [process, requireForUserSnapshot, minimalRunCjs]; } -main(); +return main(); diff --git a/src/env-inl.h b/src/env-inl.h index d5124e73e7c4c6..022f1507ce6a72 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -430,14 +430,6 @@ inline builtins::BuiltinLoader* Environment::builtin_loader() { return &builtin_loader_; } -inline const StartExecutionCallback& Environment::embedder_entry_point() const { - return embedder_entry_point_; -} - -inline void Environment::set_embedder_entry_point(StartExecutionCallback&& fn) { - embedder_entry_point_ = std::move(fn); -} - inline double Environment::new_async_id() { async_hooks()->async_id_fields()[AsyncHooks::kAsyncIdCounter] += 1; return async_hooks()->async_id_fields()[AsyncHooks::kAsyncIdCounter]; diff --git a/src/env.h b/src/env.h index 20671217050e92..6e9f5a466ac63e 100644 --- a/src/env.h +++ b/src/env.h @@ -999,9 +999,6 @@ class Environment : public MemoryRetainer { #endif // HAVE_INSPECTOR - inline const StartExecutionCallback& embedder_entry_point() const; - inline void set_embedder_entry_point(StartExecutionCallback&& fn); - inline void set_process_exit_handler( std::function&& handler); @@ -1212,7 +1209,6 @@ class Environment : public MemoryRetainer { std::unique_ptr principal_realm_ = nullptr; builtins::BuiltinLoader builtin_loader_; - StartExecutionCallback embedder_entry_point_; // Used by allocate_managed_buffer() and release_managed_buffer() to keep // track of the BackingStore for a given pointer. diff --git a/src/node.cc b/src/node.cc index 117b4d8e71439d..95bd05f9d06bb5 100644 --- a/src/node.cc +++ b/src/node.cc @@ -131,7 +131,10 @@ namespace node { +using v8::Array; +using v8::Context; using v8::EscapableHandleScope; +using v8::Function; using v8::Isolate; using v8::Local; using v8::MaybeLocal; @@ -278,6 +281,29 @@ MaybeLocal StartExecution(Environment* env, const char* main_script_id) { return scope.EscapeMaybe(realm->ExecuteBootstrapper(main_script_id)); } +// Convert the result returned by an intermediate main script into +// StartExecutionCallbackInfo. Currently the result is an array containing +// [process, requireFunction, cjsRunner] +std::optional CallbackInfoFromArray( + Local context, Local result) { + CHECK(result->IsArray()); + Local args = result.As(); + CHECK_EQ(args->Length(), 3); + Local process_obj, require_fn, runcjs_fn; + if (!args->Get(context, 0).ToLocal(&process_obj) || + !args->Get(context, 1).ToLocal(&require_fn) || + !args->Get(context, 2).ToLocal(&runcjs_fn)) { + return std::nullopt; + } + CHECK(process_obj->IsObject()); + CHECK(require_fn->IsFunction()); + CHECK(runcjs_fn->IsFunction()); + node::StartExecutionCallbackInfo info{process_obj.As(), + require_fn.As(), + runcjs_fn.As()}; + return info; +} + MaybeLocal StartExecution(Environment* env, StartExecutionCallback cb) { InternalCallbackScope callback_scope( env, @@ -285,19 +311,35 @@ MaybeLocal StartExecution(Environment* env, StartExecutionCallback cb) { { 1, 0 }, InternalCallbackScope::kSkipAsyncHooks); + // Only snapshot builder or embedder applications set the + // callback. if (cb != nullptr) { EscapableHandleScope scope(env->isolate()); - // TODO(addaleax): pass the callback to the main script more directly, - // e.g. by making StartExecution(env, builtin) parametrizable - env->set_embedder_entry_point(std::move(cb)); - auto reset_entry_point = - OnScopeLeave([&]() { env->set_embedder_entry_point({}); }); - const char* entry = env->isolate_data()->is_building_snapshot() - ? "internal/main/mksnapshot" - : "internal/main/embedding"; + Local result; + if (env->isolate_data()->is_building_snapshot()) { + if (!StartExecution(env, "internal/main/mksnapshot").ToLocal(&result)) { + return MaybeLocal(); + } + } else { + if (!StartExecution(env, "internal/main/embedding").ToLocal(&result)) { + return MaybeLocal(); + } + } + + auto info = CallbackInfoFromArray(env->context(), result); + if (!info.has_value()) { + MaybeLocal(); + } +#if HAVE_INSPECTOR + if (env->options()->debug_options().break_first_line) { + env->inspector_agent()->PauseOnNextJavascriptStatement("Break on start"); + } +#endif - return scope.EscapeMaybe(StartExecution(env, entry)); + env->performance_state()->Mark( + performance::NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE); + return scope.EscapeMaybe(cb(info.value())); } CHECK(!env->isolate_data()->is_building_snapshot()); diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index dce529fc5d245f..7e44fef542eaf5 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -41,7 +41,6 @@ using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Isolate; using v8::Local; -using v8::MaybeLocal; using v8::Object; using v8::ObjectTemplate; using v8::ScriptCompiler; @@ -1411,25 +1410,6 @@ void SerializeSnapshotableObjects(Realm* realm, }); } -static void RunEmbedderEntryPoint(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Local process_obj = args[0]; - Local require_fn = args[1]; - Local runcjs_fn = args[2]; - CHECK(process_obj->IsObject()); - CHECK(require_fn->IsFunction()); - CHECK(runcjs_fn->IsFunction()); - - const node::StartExecutionCallback& callback = env->embedder_entry_point(); - node::StartExecutionCallbackInfo info{process_obj.As(), - require_fn.As(), - runcjs_fn.As()}; - MaybeLocal retval = callback(info); - if (!retval.IsEmpty()) { - args.GetReturnValue().Set(retval.ToLocalChecked()); - } -} - void CompileSerializeMain(const FunctionCallbackInfo& args) { CHECK(args[0]->IsString()); Local filename = args[0].As(); @@ -1553,7 +1533,6 @@ void CreatePerContextProperties(Local target, void CreatePerIsolateProperties(IsolateData* isolate_data, Local target) { Isolate* isolate = isolate_data->isolate(); - SetMethod(isolate, target, "runEmbedderEntryPoint", RunEmbedderEntryPoint); SetMethod(isolate, target, "compileSerializeMain", CompileSerializeMain); SetMethod(isolate, target, "setSerializeCallback", SetSerializeCallback); SetMethod(isolate, target, "setDeserializeCallback", SetDeserializeCallback); @@ -1566,7 +1545,6 @@ void CreatePerIsolateProperties(IsolateData* isolate_data, } void RegisterExternalReferences(ExternalReferenceRegistry* registry) { - registry->Register(RunEmbedderEntryPoint); registry->Register(CompileSerializeMain); registry->Register(SetSerializeCallback); registry->Register(SetDeserializeCallback);