From ae00bc68b53a98f23f3a0ab237581a7d5af1995b Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 22 Apr 2020 04:10:10 +0800 Subject: [PATCH] src: make code cache test work with snapshots Keep track of snapshotted modules in JS land, and move bootstrap switches into StartExecution() so that they are not included into part of the environment-independent bootstrap process. PR-URL: https://github.com/nodejs/node/pull/32984 Reviewed-By: Anna Henningsen Reviewed-By: Daniel Bevenius --- lib/internal/bootstrap/node.js | 32 ++++++++++++++++++++------------ src/env.cc | 21 ++++++++++++++++++++- src/env.h | 4 ++++ src/node.cc | 1 + src/node_config.cc | 3 --- src/node_native_module_env.cc | 14 ++++++++++++++ test/parallel/test-code-cache.js | 14 ++++++++++---- 7 files changed, 69 insertions(+), 20 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 92dc2a6cc7d2d6..b39bfe078f450d 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -57,7 +57,8 @@ process.domain = null; process._exiting = false; // process.config is serialized config.gypi -process.config = JSONParse(internalBinding('native_module').config); +const nativeModule = internalBinding('native_module'); +process.config = JSONParse(nativeModule.config); require('internal/worker/js_transferable').setup(); // Bootstrappers for all threads, including worker threads and main thread @@ -192,21 +193,28 @@ process.assert = deprecate( // TODO(joyeecheung): this property has not been well-maintained, should we // deprecate it in favor of a better API? const { isDebugBuild, hasOpenSSL, hasInspector } = config; +const features = { + inspector: hasInspector, + debug: isDebugBuild, + uv: true, + ipv6: true, // TODO(bnoordhuis) ping libuv + tls_alpn: hasOpenSSL, + 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 + get cached_builtins() { + return nativeModule.hasCachedBuiltins(); + } +}; + ObjectDefineProperty(process, 'features', { enumerable: true, writable: false, configurable: false, - value: { - inspector: hasInspector, - debug: isDebugBuild, - uv: true, - ipv6: true, // TODO(bnoordhuis) ping libuv - tls_alpn: hasOpenSSL, - tls_sni: hasOpenSSL, - tls_ocsp: hasOpenSSL, - tls: hasOpenSSL, - cached_builtins: config.hasCachedBuiltins, - } + value: features }); { diff --git a/src/env.cc b/src/env.cc index e30e7d6e1dae6a..1cf37d4fa91c8c 100644 --- a/src/env.cc +++ b/src/env.cc @@ -1208,6 +1208,11 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) { EnvSerializeInfo info; Local ctx = context(); + // Currently all modules are compiled without cache in builtin snapshot + // builder. + info.native_modules = std::vector( + native_modules_without_cache.begin(), native_modules_without_cache.end()); + info.async_hooks = async_hooks_.Serialize(ctx, creator); info.immediate_info = immediate_info_.Serialize(ctx, creator); info.tick_info = tick_info_.Serialize(ctx, creator); @@ -1257,11 +1262,24 @@ std::ostream& operator<<(std::ostream& output, return output; } +std::ostream& operator<<(std::ostream& output, + const std::vector& vec) { + output << "{\n"; + for (const auto& info : vec) { + output << " \"" << info << "\",\n"; + } + output << "}"; + return output; +} + std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) { output << "{\n" + << "// -- native_modules begins --\n" + << i.native_modules << ",\n" + << "// -- native_modules ends --\n" << "// -- async_hooks begins --\n" << i.async_hooks << ",\n" - << "// -- async_hooks begins --\n" + << "// -- async_hooks ends --\n" << i.tick_info << ", // tick_info\n" << i.immediate_info << ", // immediate_info\n" << "// -- performance_state begins --\n" @@ -1284,6 +1302,7 @@ std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) { void Environment::DeserializeProperties(const EnvSerializeInfo* info) { Local ctx = context(); + native_modules_in_snapshot = info->native_modules; async_hooks_.Deserialize(ctx); immediate_info_.Deserialize(ctx); tick_info_.Deserialize(ctx); diff --git a/src/env.h b/src/env.h index c6f6ac481544f5..d475fc78fb9484 100644 --- a/src/env.h +++ b/src/env.h @@ -899,6 +899,7 @@ struct PropInfo { }; struct EnvSerializeInfo { + std::vector native_modules; AsyncHooks::SerializeInfo async_hooks; TickInfo::SerializeInfo tick_info; ImmediateInfo::SerializeInfo immediate_info; @@ -1086,6 +1087,9 @@ class Environment : public MemoryRetainer { std::set native_modules_with_cache; std::set native_modules_without_cache; + // This is only filled during deserialization. We use a vector since + // it's only used for tests. + std::vector native_modules_in_snapshot; std::unordered_multimap hash_to_module_map; std::unordered_map id_to_module_map; diff --git a/src/node.cc b/src/node.cc index ce2834448e9b69..f14ff44be80d74 100644 --- a/src/node.cc +++ b/src/node.cc @@ -350,6 +350,7 @@ MaybeLocal Environment::BootstrapNode() { return scope.EscapeMaybe(result); } + // TODO(joyeecheung): skip these in the snapshot building for workers. auto thread_switch_id = is_main_thread() ? "internal/bootstrap/switches/is_main_thread" : "internal/bootstrap/switches/is_not_main_thread"; diff --git a/src/node_config.cc b/src/node_config.cc index 6ee3164a134fe8..2d8ad25bbe9c02 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -84,9 +84,6 @@ static void Initialize(Local target, #if defined HAVE_DTRACE || defined HAVE_ETW READONLY_TRUE_PROPERTY(target, "hasDtrace"); #endif - - READONLY_PROPERTY(target, "hasCachedBuiltins", - v8::Boolean::New(isolate, native_module::has_code_cache)); } // InitConfig } // namespace node diff --git a/src/node_native_module_env.cc b/src/node_native_module_env.cc index a3a42a3ec4b9a6..e38529eefae9c4 100644 --- a/src/node_native_module_env.cc +++ b/src/node_native_module_env.cc @@ -94,6 +94,14 @@ void NativeModuleEnv::GetCacheUsage(const FunctionCallbackInfo& args) { OneByteString(isolate, "compiledWithoutCache"), ToJsSet(context, env->native_modules_without_cache)) .FromJust(); + + result + ->Set(env->context(), + OneByteString(isolate, "compiledInSnapshot"), + ToV8Value(env->context(), env->native_modules_in_snapshot) + .ToLocalChecked()) + .FromJust(); + args.GetReturnValue().Set(result); } @@ -155,6 +163,11 @@ MaybeLocal NativeModuleEnv::LookupAndCompile( return maybe; } +void HasCachedBuiltins(const FunctionCallbackInfo& args) { + args.GetReturnValue().Set( + v8::Boolean::New(args.GetIsolate(), has_code_cache)); +} + // TODO(joyeecheung): It is somewhat confusing that Class::Initialize // is used to initialize to the binding, but it is the current convention. // Rename this across the code base to something that makes more sense. @@ -198,6 +211,7 @@ void NativeModuleEnv::Initialize(Local target, env->SetMethod(target, "getCacheUsage", NativeModuleEnv::GetCacheUsage); env->SetMethod(target, "compileFunction", NativeModuleEnv::CompileFunction); + env->SetMethod(target, "hasCachedBuiltins", HasCachedBuiltins); // internalBinding('native_module') should be frozen target->SetIntegrityLevel(context, IntegrityLevel::kFrozen).FromJust(); } diff --git a/test/parallel/test-code-cache.js b/test/parallel/test-code-cache.js index 3c4488c557d524..1b151e269dcfaf 100644 --- a/test/parallel/test-code-cache.js +++ b/test/parallel/test-code-cache.js @@ -22,12 +22,16 @@ for (const key of canBeRequired) { // The computation has to be delayed until we have done loading modules const { compiledWithoutCache, - compiledWithCache + compiledWithCache, + compiledInSnapshot } = getCacheUsage(); -const loadedModules = process.moduleLoadList - .filter((m) => m.startsWith('NativeModule')) +function extractModules(list) { + return list.filter((m) => m.startsWith('NativeModule')) .map((m) => m.replace('NativeModule ', '')); +} + +const loadedModules = extractModules(process.moduleLoadList); // Cross-compiled binaries do not have code cache, verifies that the builtins // are all compiled without cache and we are doing the bookkeeping right. @@ -62,7 +66,9 @@ if (!process.features.cached_builtins) { if (cannotBeRequired.has(key) && !compiledWithoutCache.has(key)) { wrong.push(`"${key}" should've been compiled **without** code cache`); } - if (canBeRequired.has(key) && !compiledWithCache.has(key)) { + if (canBeRequired.has(key) && + !compiledWithCache.has(key) && + compiledInSnapshot.indexOf(key) === -1) { wrong.push(`"${key}" should've been compiled **with** code cache`); } }