Skip to content

Commit c708abb

Browse files
joyeecheungtargos
joyeecheung
authored and
targos
committed
src: use NativeModuleLoader to compile per_context.js
This patch introduces a NativeModuleLoader::CompileAndCall that can run a JS script under `lib/` as a function called with a null receiver and arguments specified from the C++ layer. Since all our bootstrappers are wrapped in functions in the source to avoid leaking variables into the global scope anyway, this allows us to remove that extra indentation in the JS source code. As a start we move the compilation and execution of per_context.js to NativeModuleLoader::CompileAndCall(). This patch also changes the return value of NativeModuleLoader::LookupAndCompile() to a MaybeLocal since the caller has to take care of the result being empty anyway. This patch reverts the previous design of having the NativeModuleLoader::Compile() method magically know about the parameters of the function - until we have tooling in-place to guess the parameter names in the source with some annotation, it's more readable to allow the caller to specify the parameters along with the arguments values. PR-URL: #24660 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
1 parent 83ab5f4 commit c708abb

File tree

4 files changed

+144
-100
lines changed

4 files changed

+144
-100
lines changed

‎lib/internal/per_context.js‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,44 @@
1+
// arguments: global
2+
13
'use strict';
24

35
// node::NewContext calls this script
46

5-
(function(global) {
6-
// https://github.com/nodejs/node/issues/14909
7-
if (global.Intl) delete global.Intl.v8BreakIterator;
7+
// https://github.com/nodejs/node/issues/14909
8+
if (global.Intl) delete global.Intl.v8BreakIterator;
89

9-
// https://github.com/nodejs/node/issues/21219
10-
// Adds Atomics.notify and warns on first usage of Atomics.wake
11-
// https://github.com/v8/v8/commit/c79206b363 adds Atomics.notify so
12-
// now we alias Atomics.wake to notify so that we can remove it
13-
// semver major without worrying about V8.
10+
// https://github.com/nodejs/node/issues/21219
11+
// Adds Atomics.notify and warns on first usage of Atomics.wake
12+
// https://github.com/v8/v8/commit/c79206b363 adds Atomics.notify so
13+
// now we alias Atomics.wake to notify so that we can remove it
14+
// semver major without worrying about V8.
1415

15-
const AtomicsNotify = global.Atomics.notify;
16-
const ReflectApply = global.Reflect.apply;
16+
const AtomicsNotify = global.Atomics.notify;
17+
const ReflectApply = global.Reflect.apply;
1718

18-
const warning = 'Atomics.wake will be removed in a future version, ' +
19-
'use Atomics.notify instead.';
19+
const warning = 'Atomics.wake will be removed in a future version, ' +
20+
'use Atomics.notify instead.';
2021

21-
let wakeWarned = false;
22-
function wake(typedArray, index, count) {
23-
if (!wakeWarned) {
24-
wakeWarned = true;
22+
let wakeWarned = false;
23+
function wake(typedArray, index, count) {
24+
if (!wakeWarned) {
25+
wakeWarned = true;
2526

26-
if (global.process !== undefined) {
27-
global.process.emitWarning(warning, 'Atomics');
28-
} else {
29-
global.console.error(`Atomics: ${warning}`);
30-
}
27+
if (global.process !== undefined) {
28+
global.process.emitWarning(warning, 'Atomics');
29+
} else {
30+
global.console.error(`Atomics: ${warning}`);
3131
}
32-
33-
return ReflectApply(AtomicsNotify, this, arguments);
3432
}
3533

36-
global.Object.defineProperties(global.Atomics, {
37-
wake: {
38-
value: wake,
39-
writable: true,
40-
enumerable: false,
41-
configurable: true,
42-
},
43-
});
44-
}(this));
34+
return ReflectApply(AtomicsNotify, this, arguments);
35+
}
36+
37+
global.Object.defineProperties(global.Atomics, {
38+
wake: {
39+
value: wake,
40+
writable: true,
41+
enumerable: false,
42+
configurable: true,
43+
},
44+
});

‎src/node.cc‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ using v8::ObjectTemplate;
162162
using v8::PropertyAttribute;
163163
using v8::ReadOnly;
164164
using v8::Script;
165-
using v8::ScriptCompiler;
166165
using v8::ScriptOrigin;
167166
using v8::SealHandleScope;
168167
using v8::SideEffectType;
@@ -2500,14 +2499,16 @@ Local<Context> NewContext(Isolate* isolate,
25002499
// Run lib/internal/per_context.js
25012500
Context::Scope context_scope(context);
25022501

2503-
// TODO(joyeecheung): use NativeModuleLoader::Compile
2504-
Local<String> per_context =
2505-
per_process_loader.GetSource(isolate, "internal/per_context");
2506-
ScriptCompiler::Source per_context_src(per_context, nullptr);
2507-
Local<Script> s = ScriptCompiler::Compile(
2508-
context,
2509-
&per_context_src).ToLocalChecked();
2510-
s->Run(context).ToLocalChecked();
2502+
std::vector<Local<String>> parameters = {
2503+
FIXED_ONE_BYTE_STRING(isolate, "global")};
2504+
std::vector<Local<Value>> arguments = {context->Global()};
2505+
MaybeLocal<Value> result = per_process_loader.CompileAndCall(
2506+
context, "internal/per_context", &parameters, &arguments, nullptr);
2507+
if (result.IsEmpty()) {
2508+
// Execution failed during context creation.
2509+
// TODO(joyeecheung): deprecate this signature and return a MaybeLocal.
2510+
return Local<Context>();
2511+
}
25112512
}
25122513

25132514
return context;

‎src/node_native_module.cc‎

Lines changed: 60 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -103,29 +103,54 @@ void NativeModuleLoader::CompileCodeCache(
103103

104104
// TODO(joyeecheung): allow compiling cache for bootstrapper by
105105
// switching on id
106-
Local<Value> result = CompileAsModule(env, *id, true);
106+
MaybeLocal<Value> result =
107+
CompileAsModule(env, *id, CompilationResultType::kCodeCache);
107108
if (!result.IsEmpty()) {
108-
args.GetReturnValue().Set(result);
109+
args.GetReturnValue().Set(result.ToLocalChecked());
109110
}
110111
}
111112

112113
void NativeModuleLoader::CompileFunction(
113114
const FunctionCallbackInfo<Value>& args) {
114115
Environment* env = Environment::GetCurrent(args);
115-
116116
CHECK(args[0]->IsString());
117117
node::Utf8Value id(env->isolate(), args[0].As<String>());
118-
Local<Value> result = CompileAsModule(env, *id, false);
118+
119+
MaybeLocal<Value> result =
120+
CompileAsModule(env, *id, CompilationResultType::kFunction);
119121
if (!result.IsEmpty()) {
120-
args.GetReturnValue().Set(result);
122+
args.GetReturnValue().Set(result.ToLocalChecked());
121123
}
122124
}
123125

124-
Local<Value> NativeModuleLoader::CompileAsModule(Environment* env,
125-
const char* id,
126-
bool produce_code_cache) {
126+
// TODO(joyeecheung): it should be possible to generate the argument names
127+
// from some special comments for the bootstrapper case.
128+
MaybeLocal<Value> NativeModuleLoader::CompileAndCall(
129+
Local<Context> context,
130+
const char* id,
131+
std::vector<Local<String>>* parameters,
132+
std::vector<Local<Value>>* arguments,
133+
Environment* optional_env) {
134+
Isolate* isolate = context->GetIsolate();
135+
MaybeLocal<Value> compiled = per_process_loader.LookupAndCompile(
136+
context, id, parameters, CompilationResultType::kFunction, nullptr);
137+
if (compiled.IsEmpty()) {
138+
return compiled;
139+
}
140+
Local<Function> fn = compiled.ToLocalChecked().As<Function>();
141+
return fn->Call(
142+
context, v8::Null(isolate), arguments->size(), arguments->data());
143+
}
144+
145+
MaybeLocal<Value> NativeModuleLoader::CompileAsModule(
146+
Environment* env, const char* id, CompilationResultType result) {
147+
std::vector<Local<String>> parameters = {env->exports_string(),
148+
env->require_string(),
149+
env->module_string(),
150+
env->process_string(),
151+
env->internal_binding_string()};
127152
return per_process_loader.LookupAndCompile(
128-
env->context(), id, produce_code_cache, env);
153+
env->context(), id, &parameters, result, env);
129154
}
130155

131156
// Currently V8 only checks that the length of the source code is the
@@ -183,15 +208,18 @@ ScriptCompiler::CachedData* NativeModuleLoader::GetCachedData(
183208
return new ScriptCompiler::CachedData(code_cache_value, code_cache_length);
184209
}
185210

186-
// Returns Local<Function> of the compiled module if produce_code_cache
211+
// Returns Local<Function> of the compiled module if return_code_cache
187212
// is false (we are only compiling the function).
188213
// Otherwise return a Local<Object> containing the cache.
189-
Local<Value> NativeModuleLoader::LookupAndCompile(Local<Context> context,
190-
const char* id,
191-
bool produce_code_cache,
192-
Environment* optional_env) {
214+
MaybeLocal<Value> NativeModuleLoader::LookupAndCompile(
215+
Local<Context> context,
216+
const char* id,
217+
std::vector<Local<String>>* parameters,
218+
CompilationResultType result_type,
219+
Environment* optional_env) {
193220
Isolate* isolate = context->GetIsolate();
194221
EscapableHandleScope scope(isolate);
222+
Local<Value> ret; // Used to convert to MaybeLocal before return
195223

196224
Local<String> source = GetSource(isolate, id);
197225

@@ -209,7 +237,7 @@ Local<Value> NativeModuleLoader::LookupAndCompile(Local<Context> context,
209237
// built with them.
210238
// 2. If we are generating code cache for tools/general_code_cache.js, we
211239
// are not going to use any cache ourselves.
212-
if (has_code_cache_ && !produce_code_cache) {
240+
if (has_code_cache_ && result_type == CompilationResultType::kFunction) {
213241
cached_data = GetCachedData(id);
214242
if (cached_data != nullptr) {
215243
use_cache = true;
@@ -219,50 +247,33 @@ Local<Value> NativeModuleLoader::LookupAndCompile(Local<Context> context,
219247
ScriptCompiler::Source script_source(source, origin, cached_data);
220248

221249
ScriptCompiler::CompileOptions options;
222-
if (produce_code_cache) {
250+
if (result_type == CompilationResultType::kCodeCache) {
223251
options = ScriptCompiler::kEagerCompile;
224252
} else if (use_cache) {
225253
options = ScriptCompiler::kConsumeCodeCache;
226254
} else {
227255
options = ScriptCompiler::kNoCompileOptions;
228256
}
229257

230-
MaybeLocal<Function> maybe_fun;
231-
// Currently we assume if Environment is ready, then we must be compiling
232-
// native modules instead of bootstrappers.
233-
if (optional_env != nullptr) {
234-
Local<String> parameters[] = {optional_env->exports_string(),
235-
optional_env->require_string(),
236-
optional_env->module_string(),
237-
optional_env->process_string(),
238-
optional_env->internal_binding_string()};
239-
maybe_fun = ScriptCompiler::CompileFunctionInContext(context,
240-
&script_source,
241-
arraysize(parameters),
242-
parameters,
243-
0,
244-
nullptr,
245-
options);
246-
} else {
247-
// Until we migrate bootstrappers compilations here this is unreachable
248-
// TODO(joyeecheung): it should be possible to generate the argument names
249-
// from some special comments for the bootstrapper case.
250-
// Note that for bootstrappers we may not be able to get the argument
251-
// names as env->some_string() because we might be compiling before
252-
// those strings are initialized.
253-
UNREACHABLE();
254-
}
258+
MaybeLocal<Function> maybe_fun =
259+
ScriptCompiler::CompileFunctionInContext(context,
260+
&script_source,
261+
parameters->size(),
262+
parameters->data(),
263+
0,
264+
nullptr,
265+
options);
255266

256-
Local<Function> fun;
257267
// This could fail when there are early errors in the native modules,
258268
// e.g. the syntax errors
259-
if (maybe_fun.IsEmpty() || !maybe_fun.ToLocal(&fun)) {
269+
if (maybe_fun.IsEmpty()) {
260270
// In the case of early errors, v8 is already capable of
261271
// decorating the stack for us - note that we use CompileFunctionInContext
262272
// so there is no need to worry about wrappers.
263-
return scope.Escape(Local<Value>());
273+
return MaybeLocal<Value>();
264274
}
265275

276+
Local<Function> fun = maybe_fun.ToLocalChecked();
266277
if (use_cache) {
267278
if (optional_env != nullptr) {
268279
// This could happen when Node is run with any v8 flag, but
@@ -279,7 +290,7 @@ Local<Value> NativeModuleLoader::LookupAndCompile(Local<Context> context,
279290
}
280291
}
281292

282-
if (produce_code_cache) {
293+
if (result_type == CompilationResultType::kCodeCache) {
283294
std::unique_ptr<ScriptCompiler::CachedData> cached_data(
284295
ScriptCompiler::CreateCodeCacheForFunction(fun));
285296
CHECK_NE(cached_data, nullptr);
@@ -296,10 +307,12 @@ Local<Value> NativeModuleLoader::LookupAndCompile(Local<Context> context,
296307
copied.release(),
297308
cached_data_length,
298309
ArrayBufferCreationMode::kInternalized);
299-
return scope.Escape(Uint8Array::New(buf, 0, cached_data_length));
310+
ret = Uint8Array::New(buf, 0, cached_data_length);
300311
} else {
301-
return scope.Escape(fun);
312+
ret = fun;
302313
}
314+
315+
return scope.Escape(ret);
303316
}
304317

305318
void NativeModuleLoader::Initialize(Local<Object> target,

‎src/node_native_module.h‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,43 @@ namespace native_module {
1616
using NativeModuleRecordMap = std::map<std::string, UnionBytes>;
1717
using NativeModuleHashMap = std::map<std::string, std::string>;
1818

19-
// The native (C++) side of the native module compilation.
20-
// This class should not depend on Environment
19+
// The native (C++) side of the NativeModule in JS land, which
20+
// handles compilation and caching of builtin modules (NativeModule)
21+
// and bootstrappers, whose source are bundled into the binary
22+
// as static data.
23+
// This class should not depend on a particular isolate, context, or
24+
// environment. Rather it should take them as arguments when necessary.
25+
// The instances of this class are per-process.
2126
class NativeModuleLoader {
2227
public:
28+
// kCodeCache indicates that the compilation result should be returned
29+
// as a Uint8Array, whereas kFunction indicates that the result should
30+
// be returned as a Function.
31+
// TODO(joyeecheung): it's possible to always produce code cache
32+
// on the main thread and consume them in worker threads, or just
33+
// share the cache among all the threads, although
34+
// we need to decide whether to do that even when workers are not used.
35+
enum class CompilationResultType { kCodeCache, kFunction };
36+
2337
NativeModuleLoader();
2438
static void Initialize(v8::Local<v8::Object> target,
2539
v8::Local<v8::Value> unused,
2640
v8::Local<v8::Context> context);
2741
v8::Local<v8::Object> GetSourceObject(v8::Local<v8::Context> context) const;
2842
v8::Local<v8::String> GetSource(v8::Isolate* isolate, const char* id) const;
2943

44+
// Run a script with JS source bundled inside the binary as if it's wrapped
45+
// in a function called with a null receiver and arguments specified in C++.
46+
// The returned value is empty if an exception is encountered.
47+
// JS code run with this method can assume that their top-level
48+
// declarations won't affect the global scope.
49+
v8::MaybeLocal<v8::Value> CompileAndCall(
50+
v8::Local<v8::Context> context,
51+
const char* id,
52+
std::vector<v8::Local<v8::String>>* parameters,
53+
std::vector<v8::Local<v8::Value>>* arguments,
54+
Environment* optional_env);
55+
3056
private:
3157
static void GetCacheUsage(const v8::FunctionCallbackInfo<v8::Value>& args);
3258
// For legacy process.binding('natives') which is mutable, and for
@@ -48,17 +74,21 @@ class NativeModuleLoader {
4874
void LoadCodeCacheHash(); // Loads data into code_cache_hash_
4975

5076
v8::ScriptCompiler::CachedData* GetCachedData(const char* id) const;
51-
static v8::Local<v8::Value> CompileAsModule(Environment* env,
52-
const char* id,
53-
bool produce_code_cache);
54-
// TODO(joyeecheung): make this public and reuse it to compile bootstrappers.
77+
78+
// Compile a script as a NativeModule that can be loaded via
79+
// NativeModule.p.require in JS land.
80+
static v8::MaybeLocal<v8::Value> CompileAsModule(
81+
Environment* env, const char* id, CompilationResultType result_type);
82+
5583
// For bootstrappers optional_env may be a nullptr.
56-
// This method magically knows what parameter it should pass to
57-
// the function to be compiled.
58-
v8::Local<v8::Value> LookupAndCompile(v8::Local<v8::Context> context,
59-
const char* id,
60-
bool produce_code_cache,
61-
Environment* optional_env);
84+
// If an exception is encountered (e.g. source code contains
85+
// syntax error), the returned value is empty.
86+
v8::MaybeLocal<v8::Value> LookupAndCompile(
87+
v8::Local<v8::Context> context,
88+
const char* id,
89+
std::vector<v8::Local<v8::String>>* parameters,
90+
CompilationResultType result_type,
91+
Environment* optional_env);
6292

6393
bool has_code_cache_ = false;
6494
NativeModuleRecordMap source_;

0 commit comments

Comments
 (0)