Skip to content

Commit

Permalink
src: make code cache test work with snapshots
Browse files Browse the repository at this point in the history
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: #32984
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
  • Loading branch information
joyeecheung committed Jul 18, 2020
1 parent 1faf6f4 commit 7ecb285
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 20 deletions.
32 changes: 20 additions & 12 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
});

{
Expand Down
21 changes: 20 additions & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1208,6 +1208,11 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
EnvSerializeInfo info;
Local<Context> ctx = context();

// Currently all modules are compiled without cache in builtin snapshot
// builder.
info.native_modules = std::vector<std::string>(
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);
Expand Down Expand Up @@ -1257,11 +1262,24 @@ std::ostream& operator<<(std::ostream& output,
return output;
}

std::ostream& operator<<(std::ostream& output,
const std::vector<std::string>& 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"
Expand All @@ -1284,6 +1302,7 @@ std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
void Environment::DeserializeProperties(const EnvSerializeInfo* info) {
Local<Context> ctx = context();

native_modules_in_snapshot = info->native_modules;
async_hooks_.Deserialize(ctx);
immediate_info_.Deserialize(ctx);
tick_info_.Deserialize(ctx);
Expand Down
4 changes: 4 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,7 @@ struct PropInfo {
};

struct EnvSerializeInfo {
std::vector<std::string> native_modules;
AsyncHooks::SerializeInfo async_hooks;
TickInfo::SerializeInfo tick_info;
ImmediateInfo::SerializeInfo immediate_info;
Expand Down Expand Up @@ -1086,6 +1087,9 @@ class Environment : public MemoryRetainer {

std::set<std::string> native_modules_with_cache;
std::set<std::string> native_modules_without_cache;
// This is only filled during deserialization. We use a vector since
// it's only used for tests.
std::vector<std::string> native_modules_in_snapshot;

std::unordered_multimap<int, loader::ModuleWrap*> hash_to_module_map;
std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
Expand Down
1 change: 1 addition & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ MaybeLocal<Value> 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";
Expand Down
3 changes: 0 additions & 3 deletions src/node_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ static void Initialize(Local<Object> 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
Expand Down
14 changes: 14 additions & 0 deletions src/node_native_module_env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ void NativeModuleEnv::GetCacheUsage(const FunctionCallbackInfo<Value>& 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);
}

Expand Down Expand Up @@ -155,6 +163,11 @@ MaybeLocal<Function> NativeModuleEnv::LookupAndCompile(
return maybe;
}

void HasCachedBuiltins(const FunctionCallbackInfo<Value>& 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.
Expand Down Expand Up @@ -198,6 +211,7 @@ void NativeModuleEnv::Initialize(Local<Object> 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();
}
Expand Down
14 changes: 10 additions & 4 deletions test/parallel/test-code-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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`);
}
}
Expand Down

0 comments on commit 7ecb285

Please sign in to comment.