Skip to content

Commit 77b0efe

Browse files
committed
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.
1 parent 7d0932e commit 77b0efe

File tree

7 files changed

+57
-19
lines changed

7 files changed

+57
-19
lines changed

lib/internal/bootstrap/node.js

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ process.domain = null;
5757
process._exiting = false;
5858

5959
// process.config is serialized config.gypi
60-
process.config = JSONParse(internalBinding('native_module').config);
60+
const nativeModule = internalBinding('native_module');
61+
process.config = JSONParse(nativeModule.config);
6162

6263
// Bootstrappers for all threads, including worker threads and main thread
6364
const perThreadSetup = require('internal/process/per_thread');
@@ -184,21 +185,28 @@ process.assert = deprecate(
184185
// TODO(joyeecheung): this property has not been well-maintained, should we
185186
// deprecate it in favor of a better API?
186187
const { isDebugBuild, hasOpenSSL, hasInspector } = config;
188+
const features = {
189+
inspector: hasInspector,
190+
debug: isDebugBuild,
191+
uv: true,
192+
ipv6: true, // TODO(bnoordhuis) ping libuv
193+
tls_alpn: hasOpenSSL,
194+
tls_sni: hasOpenSSL,
195+
tls_ocsp: hasOpenSSL,
196+
tls: hasOpenSSL,
197+
// This needs to be dynamic because snapshot is built without code cache.
198+
// TODO(joyeecheung): https://github.com/nodejs/node/issues/31074
199+
// Make it possible to build snapshot with code cache
200+
get cached_builtins() {
201+
return nativeModule.hasCachedBuiltins();
202+
}
203+
};
204+
187205
ObjectDefineProperty(process, 'features', {
188206
enumerable: true,
189207
writable: false,
190208
configurable: false,
191-
value: {
192-
inspector: hasInspector,
193-
debug: isDebugBuild,
194-
uv: true,
195-
ipv6: true, // TODO(bnoordhuis) ping libuv
196-
tls_alpn: hasOpenSSL,
197-
tls_sni: hasOpenSSL,
198-
tls_ocsp: hasOpenSSL,
199-
tls: hasOpenSSL,
200-
cached_builtins: config.hasCachedBuiltins,
201-
}
209+
value: features
202210
});
203211

204212
{

src/env.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,6 +1256,12 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
12561256
EnvSerializeInfo info;
12571257
Local<Context> ctx = context();
12581258

1259+
// Currently all modules are compiled without cache in snapshot builder
1260+
std::vector<std::string> modules_in_snapshot(
1261+
native_modules_without_cache.begin(), native_modules_without_cache.end());
1262+
Local<Value> arr = ToV8Value(ctx, modules_in_snapshot).ToLocalChecked();
1263+
set_native_module_in_snapshot(arr.As<v8::Array>());
1264+
12591265
NoBindingData* data =
12601266
BindingDataBase::Unwrap<NoBindingData>(ctx, default_callback_data());
12611267
info.default_callback_data = creator->AddData(ctx, data->object());

src/env.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,7 @@ constexpr size_t kFsStatsBufferLength =
456456
V(immediate_callback_function, v8::Function) \
457457
V(inspector_console_extension_installer, v8::Function) \
458458
V(message_port, v8::Object) \
459+
V(native_module_in_snapshot, v8::Array) \
459460
V(native_module_require, v8::Function) \
460461
V(performance_entry_callback, v8::Function) \
461462
V(performance_entry_template, v8::Function) \

src/node.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ MaybeLocal<Value> Environment::BootstrapNode() {
308308
return scope.EscapeMaybe(result);
309309
}
310310

311+
// TODO(joyeecheung): skip these in the snapshot building for workers.
311312
auto thread_switch_id =
312313
is_main_thread() ? "internal/bootstrap/switches/is_main_thread"
313314
: "internal/bootstrap/switches/is_not_main_thread";

src/node_config.cc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,6 @@ static void Initialize(Local<Object> target,
8484
#if defined HAVE_DTRACE || defined HAVE_ETW
8585
READONLY_TRUE_PROPERTY(target, "hasDtrace");
8686
#endif
87-
88-
READONLY_PROPERTY(target, "hasCachedBuiltins",
89-
v8::Boolean::New(isolate, native_module::has_code_cache));
9087
} // InitConfig
9188

9289
} // namespace node

src/node_native_module_env.cc

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,19 @@ void NativeModuleEnv::GetCacheUsage(const FunctionCallbackInfo<Value>& args) {
9595
OneByteString(isolate, "compiledWithoutCache"),
9696
ToJsSet(context, env->native_modules_without_cache))
9797
.FromJust();
98+
99+
Local<Value> native_module_in_snapshot;
100+
if (env->native_module_in_snapshot().IsEmpty()) {
101+
native_module_in_snapshot = v8::Array::New(isolate);
102+
} else {
103+
native_module_in_snapshot = env->native_module_in_snapshot();
104+
}
105+
result
106+
->Set(env->context(),
107+
OneByteString(isolate, "compiledInSnapshot"),
108+
native_module_in_snapshot)
109+
.FromJust();
110+
98111
args.GetReturnValue().Set(result);
99112
}
100113

@@ -155,6 +168,11 @@ MaybeLocal<Function> NativeModuleEnv::LookupAndCompile(
155168
return maybe;
156169
}
157170

171+
void HasCachedBuiltins(const FunctionCallbackInfo<Value>& args) {
172+
args.GetReturnValue().Set(
173+
v8::Boolean::New(args.GetIsolate(), has_code_cache));
174+
}
175+
158176
// TODO(joyeecheung): It is somewhat confusing that Class::Initialize
159177
// is used to initialize to the binding, but it is the current convention.
160178
// Rename this across the code base to something that makes more sense.
@@ -198,6 +216,7 @@ void NativeModuleEnv::Initialize(Local<Object> target,
198216

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

test/parallel/test-code-cache.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,16 @@ for (const key of canBeRequired) {
2222
// The computation has to be delayed until we have done loading modules
2323
const {
2424
compiledWithoutCache,
25-
compiledWithCache
25+
compiledWithCache,
26+
compiledInSnapshot
2627
} = getCacheUsage();
2728

28-
const loadedModules = process.moduleLoadList
29-
.filter((m) => m.startsWith('NativeModule'))
29+
function extractModules(list) {
30+
return list.filter((m) => m.startsWith('NativeModule'))
3031
.map((m) => m.replace('NativeModule ', ''));
32+
}
33+
34+
const loadedModules = extractModules(process.moduleLoadList);
3135

3236
// Cross-compiled binaries do not have code cache, verifies that the builtins
3337
// are all compiled without cache and we are doing the bookkeeping right.
@@ -62,7 +66,9 @@ if (!process.features.cached_builtins) {
6266
if (cannotBeRequired.has(key) && !compiledWithoutCache.has(key)) {
6367
wrong.push(`"${key}" should've been compiled **without** code cache`);
6468
}
65-
if (canBeRequired.has(key) && !compiledWithCache.has(key)) {
69+
if (canBeRequired.has(key) &&
70+
!compiledWithCache.has(key) &&
71+
compiledInSnapshot.indexOf(key) === -1) {
6672
wrong.push(`"${key}" should've been compiled **with** code cache`);
6773
}
6874
}

0 commit comments

Comments
 (0)