Skip to content

Commit ab57c8a

Browse files
joyeecheungjoyeecheung
committed
deps: V8: backport 46d7bc7f5fdc
Original commit message: [api] Add index-based module resolution in InstantiateModule() Add new InstantiateModule() overload that allows embedders to identify modules requests by index in the module requests array rather than using specifier and import attributes. When embedders want to fetch all the modules using information from module->GetModuleRequests() before calling InstantiateModule() instead of having to do the fetching inside the InstantiateModule() callback, they could just maintain a simple array of modules indexed by module request positions and look up the fetched the module by index in the new callback. Previously this has to be done by mapping from specifier and import attributes to module objects cached on the embedder side, leading to an overhead to hash the specifier and import attributes for each module request. Refs: nodejs#59117 Bug: 435317398 Change-Id: Ie017d2f3ccc605e0f58aa423504b5fa5fdbcc633 Refs: v8/v8@46d7bc7 Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 5a7ba89 commit ab57c8a

File tree

8 files changed

+388
-39
lines changed

8 files changed

+388
-39
lines changed

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838

3939
# Reset this number to 0 on major V8 upgrades.
4040
# Increment by one for each non-official patch applied to deps/v8.
41-
'v8_embedder_string': '-node.18',
41+
'v8_embedder_string': '-node.19',
4242

4343
##### V8 defaults for Node.js #####
4444

‎deps/v8/include/v8-script.h‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,13 @@ class V8_EXPORT Module : public Data {
220220
Local<Context> context, Local<String> specifier,
221221
Local<FixedArray> import_attributes, Local<Module> referrer);
222222

223+
using ResolveModuleByIndexCallback = MaybeLocal<Module> (*)(
224+
Local<Context> context, size_t module_request_index,
225+
Local<Module> referrer);
226+
using ResolveSourceByIndexCallback = MaybeLocal<Object> (*)(
227+
Local<Context> context, size_t module_request_index,
228+
Local<Module> referrer);
229+
223230
/**
224231
* Instantiates the module and its dependencies.
225232
*
@@ -231,6 +238,16 @@ class V8_EXPORT Module : public Data {
231238
Local<Context> context, ResolveModuleCallback module_callback,
232239
ResolveSourceCallback source_callback = nullptr);
233240

241+
/**
242+
* Similar to the variant that takes ResolveModuleCallback and
243+
* ResolveSourceCallback, but uses the index into the array that is returned
244+
* by GetModuleRequests() instead of the specifier and import attributes to
245+
* identify the requests.
246+
*/
247+
V8_WARN_UNUSED_RESULT Maybe<bool> InstantiateModule(
248+
Local<Context> context, ResolveModuleByIndexCallback module_callback,
249+
ResolveSourceByIndexCallback source_callback = nullptr);
250+
234251
/**
235252
* Evaluates the module and its dependencies.
236253
*

‎deps/v8/src/api/api.cc‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2245,14 +2245,32 @@ int Module::GetIdentityHash() const {
22452245
return self->hash();
22462246
}
22472247

2248+
Maybe<bool> Module::InstantiateModule(
2249+
Local<Context> context, ResolveModuleByIndexCallback module_callback,
2250+
ResolveSourceByIndexCallback source_callback) {
2251+
auto i_isolate = i::Isolate::Current();
2252+
ENTER_V8(i_isolate, context, Module, InstantiateModule, i::HandleScope);
2253+
i::Module::UserResolveCallbacks callbacks;
2254+
callbacks.module_callback_by_index = module_callback;
2255+
callbacks.source_callback_by_index = source_callback;
2256+
has_exception =
2257+
!i::Module::Instantiate(i_isolate, Utils::OpenHandle(this), context,
2258+
callbacks);
2259+
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
2260+
return Just(true);
2261+
}
2262+
22482263
Maybe<bool> Module::InstantiateModule(Local<Context> context,
22492264
ResolveModuleCallback module_callback,
22502265
ResolveSourceCallback source_callback) {
2251-
auto i_isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
2266+
auto i_isolate = i::Isolate::Current();
22522267
ENTER_V8(i_isolate, context, Module, InstantiateModule, i::HandleScope);
2268+
i::Module::UserResolveCallbacks callbacks;
2269+
callbacks.module_callback = module_callback;
2270+
callbacks.source_callback = source_callback;
22532271
has_exception =
22542272
!i::Module::Instantiate(i_isolate, Utils::OpenHandle(this), context,
2255-
module_callback, source_callback);
2273+
callbacks);
22562274
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
22572275
return Just(true);
22582276
}

‎deps/v8/src/objects/module.cc‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -205,14 +205,12 @@ MaybeHandle<Cell> Module::ResolveExport(Isolate* isolate, Handle<Module> module,
205205

206206
bool Module::Instantiate(Isolate* isolate, Handle<Module> module,
207207
v8::Local<v8::Context> context,
208-
v8::Module::ResolveModuleCallback module_callback,
209-
v8::Module::ResolveSourceCallback source_callback) {
208+
Module::UserResolveCallbacks callbacks) {
210209
#ifdef DEBUG
211210
PrintStatusMessage(*module, "Instantiating module ");
212211
#endif // DEBUG
213212

214-
if (!PrepareInstantiate(isolate, module, context, module_callback,
215-
source_callback)) {
213+
if (!PrepareInstantiate(isolate, module, context, callbacks)) {
216214
ResetGraph(isolate, module);
217215
DCHECK_EQ(module->status(), kUnlinked);
218216
return false;
@@ -231,11 +229,9 @@ bool Module::Instantiate(Isolate* isolate, Handle<Module> module,
231229
return true;
232230
}
233231

234-
bool Module::PrepareInstantiate(
235-
Isolate* isolate, DirectHandle<Module> module,
236-
v8::Local<v8::Context> context,
237-
v8::Module::ResolveModuleCallback module_callback,
238-
v8::Module::ResolveSourceCallback source_callback) {
232+
bool Module::PrepareInstantiate(Isolate* isolate, DirectHandle<Module> module,
233+
v8::Local<v8::Context> context,
234+
UserResolveCallbacks callbacks) {
239235
DCHECK_NE(module->status(), kEvaluating);
240236
DCHECK_NE(module->status(), kLinking);
241237
if (module->status() >= kPreLinking) return true;
@@ -244,8 +240,7 @@ bool Module::PrepareInstantiate(
244240

245241
if (IsSourceTextModule(*module)) {
246242
return SourceTextModule::PrepareInstantiate(
247-
isolate, Cast<SourceTextModule>(module), context, module_callback,
248-
source_callback);
243+
isolate, Cast<SourceTextModule>(module), context, callbacks);
249244
} else {
250245
return SyntheticModule::PrepareInstantiate(
251246
isolate, Cast<SyntheticModule>(module), context);

‎deps/v8/src/objects/module.h‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,21 @@ class Module : public TorqueGeneratedModule<Module, HeapObject> {
5858
// i.e. has a top-level await.
5959
V8_WARN_UNUSED_RESULT bool IsGraphAsync(Isolate* isolate) const;
6060

61+
struct UserResolveCallbacks {
62+
v8::Module::ResolveModuleCallback module_callback = nullptr;
63+
v8::Module::ResolveSourceCallback source_callback = nullptr;
64+
v8::Module::ResolveModuleByIndexCallback module_callback_by_index = nullptr;
65+
v8::Module::ResolveSourceByIndexCallback source_callback_by_index = nullptr;
66+
};
67+
6168
// Implementation of spec operation ModuleDeclarationInstantiation.
6269
// Returns false if an exception occurred during instantiation, true
6370
// otherwise. (In the case where the callback throws an exception, that
6471
// exception is propagated.)
65-
static V8_WARN_UNUSED_RESULT bool Instantiate(
66-
Isolate* isolate, Handle<Module> module, v8::Local<v8::Context> context,
67-
v8::Module::ResolveModuleCallback module_callback,
68-
v8::Module::ResolveSourceCallback source_callback);
72+
static V8_WARN_UNUSED_RESULT bool Instantiate(Isolate* isolate,
73+
Handle<Module> module,
74+
v8::Local<v8::Context> context,
75+
UserResolveCallbacks callbacks);
6976

7077
// Implementation of spec operation ModuleEvaluation.
7178
static V8_WARN_UNUSED_RESULT MaybeDirectHandle<Object> Evaluate(
@@ -100,9 +107,7 @@ class Module : public TorqueGeneratedModule<Module, HeapObject> {
100107

101108
static V8_WARN_UNUSED_RESULT bool PrepareInstantiate(
102109
Isolate* isolate, DirectHandle<Module> module,
103-
v8::Local<v8::Context> context,
104-
v8::Module::ResolveModuleCallback module_callback,
105-
v8::Module::ResolveSourceCallback source_callback);
110+
v8::Local<v8::Context> context, UserResolveCallbacks callbacks);
106111
static V8_WARN_UNUSED_RESULT bool FinishInstantiate(
107112
Isolate* isolate, Handle<Module> module,
108113
ZoneForwardList<Handle<SourceTextModule>>* stack, unsigned* dfs_index,

‎deps/v8/src/objects/source-text-module.cc‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -345,10 +345,10 @@ MaybeHandle<Cell> SourceTextModule::ResolveExportUsingStarExports(
345345

346346
bool SourceTextModule::PrepareInstantiate(
347347
Isolate* isolate, DirectHandle<SourceTextModule> module,
348-
v8::Local<v8::Context> context,
349-
v8::Module::ResolveModuleCallback module_callback,
350-
v8::Module::ResolveSourceCallback source_callback) {
351-
DCHECK_NE(module_callback, nullptr);
348+
v8::Local<v8::Context> context, Module::UserResolveCallbacks callbacks) {
349+
// One of the callbacks must be set, otherwise we cannot resolve.
350+
DCHECK_IMPLIES(callbacks.module_callback == nullptr,
351+
callbacks.module_callback_by_index != nullptr);
352352
// Obtain requested modules.
353353
DirectHandle<SourceTextModuleInfo> module_info(module->info(), isolate);
354354
DirectHandle<FixedArray> module_requests(module_info->module_requests(),
@@ -364,11 +364,23 @@ bool SourceTextModule::PrepareInstantiate(
364364
switch (module_request->phase()) {
365365
case ModuleImportPhase::kEvaluation: {
366366
v8::Local<v8::Module> api_requested_module;
367-
if (!module_callback(context, v8::Utils::ToLocal(specifier),
368-
v8::Utils::FixedArrayToLocal(import_attributes),
369-
v8::Utils::ToLocal(Cast<Module>(module)))
370-
.ToLocal(&api_requested_module)) {
371-
return false;
367+
if (callbacks.module_callback != nullptr) {
368+
if (!callbacks
369+
.module_callback(
370+
context, v8::Utils::ToLocal(specifier),
371+
v8::Utils::FixedArrayToLocal(import_attributes),
372+
v8::Utils::ToLocal(Cast<Module>(module)))
373+
.ToLocal(&api_requested_module)) {
374+
return false;
375+
}
376+
} else {
377+
DCHECK_NOT_NULL(callbacks.module_callback_by_index);
378+
if (!callbacks
379+
.module_callback_by_index(
380+
context, i, v8::Utils::ToLocal(Cast<Module>(module)))
381+
.ToLocal(&api_requested_module)) {
382+
return false;
383+
}
372384
}
373385
DirectHandle<Module> requested_module =
374386
Utils::OpenDirectHandle(*api_requested_module);
@@ -378,11 +390,23 @@ bool SourceTextModule::PrepareInstantiate(
378390
case ModuleImportPhase::kSource: {
379391
DCHECK(v8_flags.js_source_phase_imports);
380392
v8::Local<v8::Object> api_requested_module_source;
381-
if (!source_callback(context, v8::Utils::ToLocal(specifier),
382-
v8::Utils::FixedArrayToLocal(import_attributes),
383-
v8::Utils::ToLocal(Cast<Module>(module)))
384-
.ToLocal(&api_requested_module_source)) {
385-
return false;
393+
if (callbacks.source_callback != nullptr) {
394+
if (!callbacks
395+
.source_callback(
396+
context, v8::Utils::ToLocal(specifier),
397+
v8::Utils::FixedArrayToLocal(import_attributes),
398+
v8::Utils::ToLocal(Cast<Module>(module)))
399+
.ToLocal(&api_requested_module_source)) {
400+
return false;
401+
}
402+
} else {
403+
DCHECK_NOT_NULL(callbacks.source_callback_by_index);
404+
if (!callbacks
405+
.source_callback_by_index(
406+
context, i, v8::Utils::ToLocal(Cast<Module>(module)))
407+
.ToLocal(&api_requested_module_source)) {
408+
return false;
409+
}
386410
}
387411
DirectHandle<JSReceiver> requested_module_source =
388412
Utils::OpenDirectHandle(*api_requested_module_source);
@@ -404,7 +428,7 @@ bool SourceTextModule::PrepareInstantiate(
404428
DirectHandle<Module> requested_module(
405429
Cast<Module>(requested_modules->get(i)), isolate);
406430
if (!Module::PrepareInstantiate(isolate, requested_module, context,
407-
module_callback, source_callback)) {
431+
callbacks)) {
408432
return false;
409433
}
410434
}

‎deps/v8/src/objects/source-text-module.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,7 @@ class SourceTextModule
172172

173173
static V8_WARN_UNUSED_RESULT bool PrepareInstantiate(
174174
Isolate* isolate, DirectHandle<SourceTextModule> module,
175-
v8::Local<v8::Context> context,
176-
v8::Module::ResolveModuleCallback module_callback,
177-
v8::Module::ResolveSourceCallback source_callback);
175+
v8::Local<v8::Context> context, Module::UserResolveCallbacks callbacks);
178176
static V8_WARN_UNUSED_RESULT bool FinishInstantiate(
179177
Isolate* isolate, Handle<SourceTextModule> module,
180178
ZoneForwardList<Handle<SourceTextModule>>* stack, unsigned* dfs_index,

0 commit comments

Comments
 (0)