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.
  • Loading branch information
joyeecheung committed Apr 21, 2020
1 parent 40ae03a commit cae01f6
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 42 deletions.
37 changes: 25 additions & 12 deletions lib/internal/bootstrap/node.js
Expand Up @@ -40,6 +40,7 @@ setupPrepareStackTrace();

const {
JSONParse,
ObjectFreeze,
ObjectDefineProperty,
ObjectGetPrototypeOf,
ObjectSetPrototypeOf,
Expand All @@ -57,7 +58,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);

// Bootstrappers for all threads, including worker threads and main thread
const perThreadSetup = require('internal/process/per_thread');
Expand Down Expand Up @@ -184,21 +186,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 Expand Up @@ -358,3 +367,7 @@ function defineOperation(target, name, method) {
value: method
});
}

// Make sure this is done at the end of bootstrap included in snapshot.
nativeModule.snapshottedModules = [...(process.moduleLoadList)];
ObjectFreeze(nativeModule);
42 changes: 21 additions & 21 deletions src/node.cc
Expand Up @@ -308,27 +308,6 @@ MaybeLocal<Value> Environment::BootstrapNode() {
return scope.EscapeMaybe(result);
}

auto thread_switch_id =
is_main_thread() ? "internal/bootstrap/switches/is_main_thread"
: "internal/bootstrap/switches/is_not_main_thread";
result =
ExecuteBootstrapper(this, thread_switch_id, &node_params, &node_args);

if (result.IsEmpty()) {
return scope.EscapeMaybe(result);
}

auto process_state_switch_id =
owns_process_state()
? "internal/bootstrap/switches/does_own_process_state"
: "internal/bootstrap/switches/does_not_own_process_state";
result = ExecuteBootstrapper(
this, process_state_switch_id, &node_params, &node_args);

if (result.IsEmpty()) {
return scope.EscapeMaybe(result);
}

Local<String> env_string = FIXED_ONE_BYTE_STRING(isolate_, "env");
Local<Object> env_var_proxy;
if (!CreateEnvVarProxy(context(), isolate_, current_callback_data())
Expand Down Expand Up @@ -393,6 +372,27 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
->GetFunction(env->context())
.ToLocalChecked()};

MaybeLocal<Value> result;
auto thread_switch_id =
env->is_main_thread() ? "internal/bootstrap/switches/is_main_thread"
: "internal/bootstrap/switches/is_not_main_thread";
result = ExecuteBootstrapper(env, thread_switch_id, &parameters, &arguments);

if (result.IsEmpty()) {
return scope.EscapeMaybe(result);
}

auto process_state_switch_id =
env->owns_process_state()
? "internal/bootstrap/switches/does_own_process_state"
: "internal/bootstrap/switches/does_not_own_process_state";
result = ExecuteBootstrapper(
env, process_state_switch_id, &parameters, &arguments);

if (result.IsEmpty()) {
return scope.EscapeMaybe(result);
}

return scope.EscapeMaybe(
ExecuteBootstrapper(env, main_script_id, &parameters, &arguments));
}
Expand Down
3 changes: 0 additions & 3 deletions src/node_config.cc
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
9 changes: 6 additions & 3 deletions src/node_native_module_env.cc
Expand Up @@ -155,6 +155,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,10 +203,8 @@ void NativeModuleEnv::Initialize(Local<Object> target,

env->SetMethod(target, "getCacheUsage", NativeModuleEnv::GetCacheUsage);
env->SetMethod(target, "compileFunction", NativeModuleEnv::CompileFunction);
// internalBinding('native_module') should be frozen
target->SetIntegrityLevel(context, IntegrityLevel::kFrozen).FromJust();
env->SetMethod(target, "hasCachedBuiltins", HasCachedBuiltins);
}

} // namespace native_module
} // namespace node

Expand Down
13 changes: 10 additions & 3 deletions test/parallel/test-code-cache.js
Expand Up @@ -11,6 +11,7 @@ const {
internalBinding
} = require('internal/test/binding');
const {
snapshottedModules,
getCacheUsage,
moduleCategories: { canBeRequired, cannotBeRequired }
} = internalBinding('native_module');
Expand All @@ -25,9 +26,13 @@ const {
compiledWithCache
} = 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);
const inSnapshot = new Set(extractModules(snapshottedModules));

// 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 +67,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) &&
!inSnapshot.has(key)) {
wrong.push(`"${key}" should've been compiled **with** code cache`);
}
}
Expand Down

0 comments on commit cae01f6

Please sign in to comment.