Skip to content

Commit cae01f6

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 40ae03a commit cae01f6

5 files changed

Lines changed: 62 additions & 42 deletions

File tree

lib/internal/bootstrap/node.js

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ setupPrepareStackTrace();
4040

4141
const {
4242
JSONParse,
43+
ObjectFreeze,
4344
ObjectDefineProperty,
4445
ObjectGetPrototypeOf,
4546
ObjectSetPrototypeOf,
@@ -57,7 +58,8 @@ process.domain = null;
5758
process._exiting = false;
5859

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

6264
// Bootstrappers for all threads, including worker threads and main thread
6365
const perThreadSetup = require('internal/process/per_thread');
@@ -184,21 +186,28 @@ process.assert = deprecate(
184186
// TODO(joyeecheung): this property has not been well-maintained, should we
185187
// deprecate it in favor of a better API?
186188
const { isDebugBuild, hasOpenSSL, hasInspector } = config;
189+
const features = {
190+
inspector: hasInspector,
191+
debug: isDebugBuild,
192+
uv: true,
193+
ipv6: true, // TODO(bnoordhuis) ping libuv
194+
tls_alpn: hasOpenSSL,
195+
tls_sni: hasOpenSSL,
196+
tls_ocsp: hasOpenSSL,
197+
tls: hasOpenSSL,
198+
// This needs to be dynamic because snapshot is built without code cache.
199+
// TODO(joyeecheung): https://github.com/nodejs/node/issues/31074
200+
// Make it possible to build snapshot with code cache
201+
get cached_builtins() {
202+
return nativeModule.hasCachedBuiltins();
203+
}
204+
};
205+
187206
ObjectDefineProperty(process, 'features', {
188207
enumerable: true,
189208
writable: false,
190209
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-
}
210+
value: features
202211
});
203212

204213
{
@@ -358,3 +367,7 @@ function defineOperation(target, name, method) {
358367
value: method
359368
});
360369
}
370+
371+
// Make sure this is done at the end of bootstrap included in snapshot.
372+
nativeModule.snapshottedModules = [...(process.moduleLoadList)];
373+
ObjectFreeze(nativeModule);

src/node.cc

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

311-
auto thread_switch_id =
312-
is_main_thread() ? "internal/bootstrap/switches/is_main_thread"
313-
: "internal/bootstrap/switches/is_not_main_thread";
314-
result =
315-
ExecuteBootstrapper(this, thread_switch_id, &node_params, &node_args);
316-
317-
if (result.IsEmpty()) {
318-
return scope.EscapeMaybe(result);
319-
}
320-
321-
auto process_state_switch_id =
322-
owns_process_state()
323-
? "internal/bootstrap/switches/does_own_process_state"
324-
: "internal/bootstrap/switches/does_not_own_process_state";
325-
result = ExecuteBootstrapper(
326-
this, process_state_switch_id, &node_params, &node_args);
327-
328-
if (result.IsEmpty()) {
329-
return scope.EscapeMaybe(result);
330-
}
331-
332311
Local<String> env_string = FIXED_ONE_BYTE_STRING(isolate_, "env");
333312
Local<Object> env_var_proxy;
334313
if (!CreateEnvVarProxy(context(), isolate_, current_callback_data())
@@ -393,6 +372,27 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
393372
->GetFunction(env->context())
394373
.ToLocalChecked()};
395374

375+
MaybeLocal<Value> result;
376+
auto thread_switch_id =
377+
env->is_main_thread() ? "internal/bootstrap/switches/is_main_thread"
378+
: "internal/bootstrap/switches/is_not_main_thread";
379+
result = ExecuteBootstrapper(env, thread_switch_id, &parameters, &arguments);
380+
381+
if (result.IsEmpty()) {
382+
return scope.EscapeMaybe(result);
383+
}
384+
385+
auto process_state_switch_id =
386+
env->owns_process_state()
387+
? "internal/bootstrap/switches/does_own_process_state"
388+
: "internal/bootstrap/switches/does_not_own_process_state";
389+
result = ExecuteBootstrapper(
390+
env, process_state_switch_id, &parameters, &arguments);
391+
392+
if (result.IsEmpty()) {
393+
return scope.EscapeMaybe(result);
394+
}
395+
396396
return scope.EscapeMaybe(
397397
ExecuteBootstrapper(env, main_script_id, &parameters, &arguments));
398398
}

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: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,11 @@ MaybeLocal<Function> NativeModuleEnv::LookupAndCompile(
155155
return maybe;
156156
}
157157

158+
void HasCachedBuiltins(const FunctionCallbackInfo<Value>& args) {
159+
args.GetReturnValue().Set(
160+
v8::Boolean::New(args.GetIsolate(), has_code_cache));
161+
}
162+
158163
// TODO(joyeecheung): It is somewhat confusing that Class::Initialize
159164
// is used to initialize to the binding, but it is the current convention.
160165
// Rename this across the code base to something that makes more sense.
@@ -198,10 +203,8 @@ void NativeModuleEnv::Initialize(Local<Object> target,
198203

199204
env->SetMethod(target, "getCacheUsage", NativeModuleEnv::GetCacheUsage);
200205
env->SetMethod(target, "compileFunction", NativeModuleEnv::CompileFunction);
201-
// internalBinding('native_module') should be frozen
202-
target->SetIntegrityLevel(context, IntegrityLevel::kFrozen).FromJust();
206+
env->SetMethod(target, "hasCachedBuiltins", HasCachedBuiltins);
203207
}
204-
205208
} // namespace native_module
206209
} // namespace node
207210

test/parallel/test-code-cache.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
internalBinding
1212
} = require('internal/test/binding');
1313
const {
14+
snapshottedModules,
1415
getCacheUsage,
1516
moduleCategories: { canBeRequired, cannotBeRequired }
1617
} = internalBinding('native_module');
@@ -25,9 +26,13 @@ const {
2526
compiledWithCache
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);
35+
const inSnapshot = new Set(extractModules(snapshottedModules));
3136

3237
// Cross-compiled binaries do not have code cache, verifies that the builtins
3338
// are all compiled without cache and we are doing the bookkeeping right.
@@ -62,7 +67,9 @@ if (!process.features.cached_builtins) {
6267
if (cannotBeRequired.has(key) && !compiledWithoutCache.has(key)) {
6368
wrong.push(`"${key}" should've been compiled **without** code cache`);
6469
}
65-
if (canBeRequired.has(key) && !compiledWithCache.has(key)) {
70+
if (canBeRequired.has(key) &&
71+
!compiledWithCache.has(key) &&
72+
!inSnapshot.has(key)) {
6673
wrong.push(`"${key}" should've been compiled **with** code cache`);
6774
}
6875
}

0 commit comments

Comments
 (0)