Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bootstrap: include code cache in the embedded snapshot #43023

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,8 @@

/benchmark/misc/startup.js @nodejs/startup
/src/node.cc @nodejs/startup
/src/node_code_cache_stub.cc @nodejs/startup
/src/node_native_module* @nodejs/startup
/lib/internal/bootstrap/* @nodejs/startup
/tools/code_cache/* @nodejs/startup
/tools/snapshot/* @nodejs/startup

# V8
Expand Down
2 changes: 1 addition & 1 deletion configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -1249,7 +1249,7 @@ def configure_node(o):
o['variables']['node_use_node_snapshot'] = b(
not cross_compiling and not options.shared)

if options.without_node_code_cache or options.node_builtin_modules_path:
if options.without_node_code_cache or options.without_node_snapshot or options.node_builtin_modules_path:
o['variables']['node_use_node_code_cache'] = 'false'
else:
# TODO(refack): fix this when implementing embedded code-cache when cross-compiling.
Expand Down
5 changes: 2 additions & 3 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,8 @@ const features = {
tls_sni: hasOpenSSL,
tls_ocsp: hasOpenSSL,
tls: hasOpenSSL,
// This needs to be dynamic because snapshot is built without code cache.
// TODO(joyeecheung): https://github.com/nodejs/node/issues/31074
// Make it possible to build snapshot with code cache
// This needs to be dynamic because --no-node-snapshot disables the
// code cache even if the bianry is built with embedded code cache.
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
get cached_builtins() {
return nativeModule.hasCachedBuiltins();
}
Expand Down
106 changes: 11 additions & 95 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
'deps/undici/undici.js',
],
'node_mksnapshot_exec': '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)node_mksnapshot<(EXECUTABLE_SUFFIX)',
'mkcodecache_exec': '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)mkcodecache<(EXECUTABLE_SUFFIX)',
'conditions': [
['GENERATOR == "ninja"', {
'node_text_start_object_path': 'src/large_pages/node_text_start.node_text_start.o'
Expand Down Expand Up @@ -304,32 +303,7 @@
},
},
}],
['node_use_node_code_cache=="true"', {
'dependencies': [
'mkcodecache',
],
'actions': [
{
'action_name': 'run_mkcodecache',
'process_outputs_as_sources': 1,
'inputs': [
'<(mkcodecache_exec)',
],
'outputs': [
'<(SHARED_INTERMEDIATE_DIR)/node_code_cache.cc',
],
'action': [
'<@(_inputs)',
'<@(_outputs)',
],
},
],
}, {
'sources': [
'src/node_code_cache_stub.cc'
],
}],
['node_use_node_snapshot=="true"', {
['node_use_node_snapshot=="true"', {
'dependencies': [
'node_mksnapshot',
],
Expand Down Expand Up @@ -739,7 +713,6 @@
[ 'node_shared=="true"', {
'sources': [
'src/node_snapshot_stub.cc',
'src/node_code_cache_stub.cc',
]
}],
[ 'node_shared=="true" and node_module_version!="" and OS!="win"', {
Expand All @@ -749,6 +722,11 @@
'@rpath/lib<(node_core_target_name).<(shlib_suffix)'
},
}],
[ 'node_use_node_code_cache=="true"', {
'defines': [
'NODE_USE_NODE_CODE_CACHE=1',
],
}],
['node_shared=="true" and OS=="aix"', {
'product_name': 'node_base',
}],
Expand Down Expand Up @@ -1149,7 +1127,6 @@
],
'sources': [
'src/node_snapshot_stub.cc',
'src/node_code_cache_stub.cc',
'test/fuzzers/fuzz_url.cc',
],
'conditions': [
Expand Down Expand Up @@ -1192,7 +1169,6 @@
],
'sources': [
'src/node_snapshot_stub.cc',
'src/node_code_cache_stub.cc',
'test/fuzzers/fuzz_env.cc',
],
'conditions': [
Expand Down Expand Up @@ -1242,7 +1218,6 @@

'sources': [
'src/node_snapshot_stub.cc',
'src/node_code_cache_stub.cc',
'test/cctest/node_test_fixture.cc',
'test/cctest/node_test_fixture.h',
'test/cctest/test_aliased_buffer.cc',
Expand Down Expand Up @@ -1335,7 +1310,6 @@

'sources': [
'src/node_snapshot_stub.cc',
'src/node_code_cache_stub.cc',
'test/embedding/embedtest.cc',
],

Expand Down Expand Up @@ -1379,68 +1353,6 @@
}],
]
}, # overlapped-checker

# TODO(joyeecheung): do not depend on node_lib,
# instead create a smaller static library node_lib_base that does
# just enough for node_native_module.cc and the cache builder to
# compile without compiling the generated code cache C++ file.
# So generate_code_cache -> mkcodecache -> node_lib_base,
# node_lib -> node_lib_base & generate_code_cache
{
'target_name': 'mkcodecache',
'type': 'executable',

'dependencies': [
'<(node_lib_target_name)',
'deps/histogram/histogram.gyp:histogram',
'deps/uvwasi/uvwasi.gyp:uvwasi',
],

'includes': [
'node.gypi'
],

'include_dirs': [
'src',
'tools/msvs/genfiles',
'deps/v8/include',
'deps/cares/include',
'deps/uv/include',
'deps/uvwasi/include',
],

'defines': [
'NODE_WANT_INTERNALS=1'
],
'sources': [
'src/node_snapshot_stub.cc',
'src/node_code_cache_stub.cc',
'tools/code_cache/mkcodecache.cc',
'tools/code_cache/cache_builder.cc',
'tools/code_cache/cache_builder.h',
],

'conditions': [
[ 'node_use_openssl=="true"', {
'defines': [
'HAVE_OPENSSL=1',
],
}],
['v8_enable_inspector==1', {
'defines': [
'HAVE_INSPECTOR=1',
],
}],
['OS=="win"', {
'libraries': [
'dbghelp.lib',
'PsApi.lib',
'winmm.lib',
'Ws2_32.lib',
],
}],
],
}, # mkcodecache
{
'target_name': 'node_mksnapshot',
'type': 'executable',
Expand Down Expand Up @@ -1468,7 +1380,6 @@

'sources': [
'src/node_snapshot_stub.cc',
'src/node_code_cache_stub.cc',
'tools/snapshot/node_mksnapshot.cc',
],

Expand All @@ -1478,6 +1389,11 @@
'HAVE_OPENSSL=1',
],
}],
[ 'node_use_node_code_cache=="true"', {
'defines': [
'NODE_USE_NODE_CODE_CACHE=1',
],
}],
['v8_enable_inspector==1', {
'defines': [
'HAVE_INSPECTOR=1',
Expand Down
16 changes: 14 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include "handle_wrap.h"
#include "node.h"
#include "node_binding.h"
#include "node_external_reference.h"
#include "node_main_instance.h"
#include "node_native_module.h"
#include "node_options.h"
Expand Down Expand Up @@ -973,9 +972,22 @@ struct EnvSerializeInfo {
};

struct SnapshotData {
v8::StartupData blob;
// The result of v8::SnapshotCreator::CreateBlob() during the snapshot
// building process.
v8::StartupData v8_snapshot_blob_data;

static const size_t kNodeBaseContextIndex = 0;
static const size_t kNodeMainContextIndex = kNodeBaseContextIndex + 1;

std::vector<size_t> isolate_data_indices;
// TODO(joyeecheung): there should be a vector of env_info once we snapshot
// the worker environments.
EnvSerializeInfo env_info;
// A vector of built-in ids and v8::ScriptCompiler::CachedData, this can be
// shared across Node.js instances because they are supposed to share the
// read only space. We use the vector because v8::ScriptCompiler::CachedData
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
// is not copyable.
std::vector<native_module::CodeCacheInfo> code_cache;
};

class Environment : public MemoryRetainer {
Expand Down
6 changes: 4 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -980,8 +980,6 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,

#endif // defined(NODE_HAVE_I18N_SUPPORT)

NativeModuleEnv::InitializeCodeCache();

// We should set node_is_initialized here instead of in node::Start,
// otherwise embedders using node::Init to initialize everything will not be
// able to set it and native modules will not load for them.
Expand Down Expand Up @@ -1175,6 +1173,10 @@ int Start(int argc, char** argv) {
: nullptr;
uv_loop_configure(uv_default_loop(), UV_METRICS_IDLE_TIME);

if (snapshot_data != nullptr) {
native_module::NativeModuleEnv::RefreshCodeCache(
snapshot_data->code_cache);
}
NodeMainInstance main_instance(snapshot_data,
uv_default_loop(),
per_process::v8_platform.Platform(),
Expand Down
22 changes: 0 additions & 22 deletions src/node_code_cache_stub.cc

This file was deleted.

4 changes: 3 additions & 1 deletion src/node_main_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "debug_utils-inl.h"
#include "node_external_reference.h"
#include "node_internals.h"
#include "node_native_module_env.h"
#include "node_options-inl.h"
#include "node_snapshot_builder.h"
#include "node_snapshotable.h"
Expand Down Expand Up @@ -183,12 +184,13 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code) {
EnvironmentFlags::kDefaultFlags,
{}));
context = Context::FromSnapshot(isolate_,
SnapshotBuilder::kNodeMainContextIndex,
SnapshotData::kNodeMainContextIndex,
{DeserializeNodeInternalFields, env.get()})
.ToLocalChecked();

CHECK(!context.IsEmpty());
Context::Scope context_scope(context);

CHECK(InitializeContextRuntime(context).IsJust());
SetIsolateErrorHandlers(isolate_, {});
env->InitializeMainContext(context, &(snapshot_data_->env_info));
Expand Down
33 changes: 28 additions & 5 deletions src/node_native_module.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "node_native_module.h"
#include "util-inl.h"
#include "debug_utils-inl.h"
#include "node_internals.h"
#include "util-inl.h"

namespace node {
namespace native_module {
Expand Down Expand Up @@ -277,6 +278,11 @@ MaybeLocal<Function> NativeModuleLoader::LookupAndCompile(
: ScriptCompiler::kEagerCompile;
ScriptCompiler::Source script_source(source, origin, cached_data);

per_process::Debug(DebugCategory::CODE_CACHE,
"Compiling %s %s code cache\n",
id,
has_cache ? "with" : "without");

MaybeLocal<Function> maybe_fun =
ScriptCompiler::CompileFunctionInContext(context,
&script_source,
Expand Down Expand Up @@ -304,17 +310,34 @@ MaybeLocal<Function> NativeModuleLoader::LookupAndCompile(
*result = (has_cache && !script_source.GetCachedData()->rejected)
? Result::kWithCache
: Result::kWithoutCache;

if (has_cache) {
per_process::Debug(DebugCategory::CODE_CACHE,
"Code cache of %s (%s) %s\n",
id,
script_source.GetCachedData()->buffer_policy ==
ScriptCompiler::CachedData::BufferNotOwned
? "BufferNotOwned"
: "BufferOwned",
script_source.GetCachedData()->rejected ? "is rejected"
: "is accepted");
}

// Generate new cache for next compilation
std::unique_ptr<ScriptCompiler::CachedData> new_cached_data(
ScriptCompiler::CreateCodeCacheForFunction(fun));
CHECK_NOT_NULL(new_cached_data);

{
Mutex::ScopedLock lock(code_cache_mutex_);
// The old entry should've been erased by now so we can just emplace.
// If another thread did the same thing in the meantime, that should not
// be an issue.
code_cache_.emplace(id, std::move(new_cached_data));
const auto it = code_cache_.find(id);
// TODO(joyeecheung): it's safer for each thread to have its own
// copy of the code cache map.
if (it == code_cache_.end()) {
code_cache_.emplace(id, std::move(new_cached_data));
} else {
it->second.reset(new_cached_data.release());
}
}

return scope.Escape(fun);
Expand Down
Loading