Skip to content

Commit 5ebd539

Browse files
joyeecheungjoyeecheung
committed
esm: use index-based resolution callbacks
This makes use of a new module resolution V8 API that passes in an index to the module request array to identify the module request, which eliminates the need of hashing on the module request for lookup on our end.
1 parent ab57c8a commit 5ebd539

File tree

2 files changed

+32
-91
lines changed

2 files changed

+32
-91
lines changed

‎src/module_wrap.cc‎

Lines changed: 27 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -72,49 +72,6 @@ void ModuleCacheKey::MemoryInfo(MemoryTracker* tracker) const {
7272
tracker->TrackField("import_attributes", import_attributes);
7373
}
7474

75-
template <int elements_per_attribute>
76-
ModuleCacheKey ModuleCacheKey::From(Local<Context> context,
77-
Local<String> specifier,
78-
Local<FixedArray> import_attributes) {
79-
CHECK_EQ(import_attributes->Length() % elements_per_attribute, 0);
80-
Isolate* isolate = context->GetIsolate();
81-
std::size_t h1 = specifier->GetIdentityHash();
82-
size_t num_attributes = import_attributes->Length() / elements_per_attribute;
83-
ImportAttributeVector attributes;
84-
attributes.reserve(num_attributes);
85-
86-
std::size_t h2 = 0;
87-
88-
for (int i = 0; i < import_attributes->Length();
89-
i += elements_per_attribute) {
90-
DCHECK(DataIsString(import_attributes->Get(context, i)));
91-
DCHECK(DataIsString(import_attributes->Get(context, i + 1)));
92-
93-
Local<String> v8_key = import_attributes->Get(context, i).As<String>();
94-
Local<String> v8_value =
95-
import_attributes->Get(context, i + 1).As<String>();
96-
Utf8Value key_utf8(isolate, v8_key);
97-
Utf8Value value_utf8(isolate, v8_value);
98-
99-
attributes.emplace_back(key_utf8.ToString(), value_utf8.ToString());
100-
h2 ^= v8_key->GetIdentityHash();
101-
h2 ^= v8_value->GetIdentityHash();
102-
}
103-
104-
// Combine the hashes using a simple XOR and bit shift to reduce
105-
// collisions. Note that the hash does not guarantee uniqueness.
106-
std::size_t hash = h1 ^ (h2 << 1);
107-
108-
Utf8Value utf8_specifier(isolate, specifier);
109-
return ModuleCacheKey{utf8_specifier.ToString(), attributes, hash};
110-
}
111-
112-
ModuleCacheKey ModuleCacheKey::From(Local<Context> context,
113-
Local<ModuleRequest> v8_request) {
114-
return From(
115-
context, v8_request->GetSpecifier(), v8_request->GetImportAttributes());
116-
}
117-
11875
ModuleWrap::ModuleWrap(Realm* realm,
11976
Local<Object> object,
12077
Local<Module> module,
@@ -141,15 +98,6 @@ ModuleWrap::ModuleWrap(Realm* realm,
14198
}
14299
MakeWeak();
143100
module_.SetWeak();
144-
145-
HandleScope scope(realm->isolate());
146-
Local<Context> context = realm->context();
147-
Local<FixedArray> requests = module->GetModuleRequests();
148-
for (int i = 0; i < requests->Length(); i++) {
149-
ModuleCacheKey module_cache_key = ModuleCacheKey::From(
150-
context, requests->Get(context, i).As<ModuleRequest>());
151-
resolve_cache_[module_cache_key] = i;
152-
}
153101
}
154102

155103
ModuleWrap::~ModuleWrap() {
@@ -991,11 +939,10 @@ void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
991939
// static
992940
MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
993941
Local<Context> context,
994-
Local<String> specifier,
995-
Local<FixedArray> import_attributes,
942+
size_t module_request_index,
996943
Local<Module> referrer) {
997944
ModuleWrap* resolved_module;
998-
if (!ResolveModule(context, specifier, import_attributes, referrer)
945+
if (!ResolveModule(context, module_request_index, referrer)
999946
.To(&resolved_module)) {
1000947
return {};
1001948
}
@@ -1006,11 +953,10 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
1006953
// static
1007954
MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
1008955
Local<Context> context,
1009-
Local<String> specifier,
1010-
Local<FixedArray> import_attributes,
956+
size_t module_request_index,
1011957
Local<Module> referrer) {
1012958
ModuleWrap* resolved_module;
1013-
if (!ResolveModule(context, specifier, import_attributes, referrer)
959+
if (!ResolveModule(context, module_request_index, referrer)
1014960
.To(&resolved_module)) {
1015961
return {};
1016962
}
@@ -1031,12 +977,22 @@ MaybeLocal<Object> ModuleWrap::ResolveSourceCallback(
1031977
return module_source_object.As<Object>();
1032978
}
1033979

980+
static std::string GetSpecifierFromModuleRequest(Local<Context> context,
981+
Local<Module> referrer,
982+
size_t module_request_index) {
983+
Local<ModuleRequest> raw_request =
984+
referrer->GetModuleRequests()
985+
->Get(context, static_cast<int>(module_request_index))
986+
.As<ModuleRequest>();
987+
Local<String> specifier = raw_request->GetSpecifier();
988+
Utf8Value specifier_utf8(context->GetIsolate(), specifier);
989+
return specifier_utf8.ToString();
990+
}
991+
1034992
// static
1035-
Maybe<ModuleWrap*> ModuleWrap::ResolveModule(
1036-
Local<Context> context,
1037-
Local<String> specifier,
1038-
Local<FixedArray> import_attributes,
1039-
Local<Module> referrer) {
993+
Maybe<ModuleWrap*> ModuleWrap::ResolveModule(Local<Context> context,
994+
size_t module_request_index,
995+
Local<Module> referrer) {
1040996
Isolate* isolate = context->GetIsolate();
1041997
Environment* env = Environment::GetCurrent(context);
1042998
if (env == nullptr) {
@@ -1046,31 +1002,24 @@ Maybe<ModuleWrap*> ModuleWrap::ResolveModule(
10461002
// Check that the referrer is not yet been instantiated.
10471003
DCHECK(referrer->GetStatus() <= Module::kInstantiated);
10481004

1049-
ModuleCacheKey cache_key =
1050-
ModuleCacheKey::From(context, specifier, import_attributes);
1051-
10521005
ModuleWrap* dependent = ModuleWrap::GetFromModule(env, referrer);
10531006
if (dependent == nullptr) {
1007+
std::string specifier =
1008+
GetSpecifierFromModuleRequest(context, referrer, module_request_index);
10541009
THROW_ERR_VM_MODULE_LINK_FAILURE(
1055-
env, "request for '%s' is from invalid module", cache_key.specifier);
1010+
env, "request for '%s' is from invalid module", specifier);
10561011
return Nothing<ModuleWrap*>();
10571012
}
10581013
if (!dependent->IsLinked()) {
1014+
std::string specifier =
1015+
GetSpecifierFromModuleRequest(context, referrer, module_request_index);
10591016
THROW_ERR_VM_MODULE_LINK_FAILURE(
1060-
env,
1061-
"request for '%s' is from a module not been linked",
1062-
cache_key.specifier);
1063-
return Nothing<ModuleWrap*>();
1064-
}
1065-
1066-
auto it = dependent->resolve_cache_.find(cache_key);
1067-
if (it == dependent->resolve_cache_.end()) {
1068-
THROW_ERR_VM_MODULE_LINK_FAILURE(
1069-
env, "request for '%s' is not in cache", cache_key.specifier);
1017+
env, "request for '%s' is from a module not been linked", specifier);
10701018
return Nothing<ModuleWrap*>();
10711019
}
10721020

1073-
ModuleWrap* module_wrap = dependent->GetLinkedRequest(it->second);
1021+
ModuleWrap* module_wrap =
1022+
dependent->GetLinkedRequest(static_cast<uint32_t>(module_request_index));
10741023
CHECK_NOT_NULL(module_wrap);
10751024
return Just(module_wrap);
10761025
}

‎src/module_wrap.h‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,6 @@ struct ModuleCacheKey : public MemoryRetainer {
9090
};
9191

9292
class ModuleWrap : public BaseObject {
93-
using ResolveCache =
94-
std::unordered_map<ModuleCacheKey, uint32_t, ModuleCacheKey::Hash>;
95-
9693
public:
9794
enum InternalFields {
9895
kModuleSlot = BaseObject::kInternalFieldCount,
@@ -195,26 +192,21 @@ class ModuleWrap : public BaseObject {
195192

196193
static v8::MaybeLocal<v8::Module> ResolveModuleCallback(
197194
v8::Local<v8::Context> context,
198-
v8::Local<v8::String> specifier,
199-
v8::Local<v8::FixedArray> import_attributes,
195+
size_t module_request_index,
200196
v8::Local<v8::Module> referrer);
201197
static v8::MaybeLocal<v8::Object> ResolveSourceCallback(
202198
v8::Local<v8::Context> context,
203-
v8::Local<v8::String> specifier,
204-
v8::Local<v8::FixedArray> import_attributes,
199+
size_t module_request_index,
205200
v8::Local<v8::Module> referrer);
206201
static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);
207202

208203
// This method may throw a JavaScript exception, so the return type is
209204
// wrapped in a Maybe.
210-
static v8::Maybe<ModuleWrap*> ResolveModule(
211-
v8::Local<v8::Context> context,
212-
v8::Local<v8::String> specifier,
213-
v8::Local<v8::FixedArray> import_attributes,
214-
v8::Local<v8::Module> referrer);
205+
static v8::Maybe<ModuleWrap*> ResolveModule(v8::Local<v8::Context> context,
206+
size_t module_request_index,
207+
v8::Local<v8::Module> referrer);
215208

216209
v8::Global<v8::Module> module_;
217-
ResolveCache resolve_cache_;
218210
contextify::ContextifyContext* contextify_context_ = nullptr;
219211
bool synthetic_ = false;
220212
bool linked_ = false;

0 commit comments

Comments
 (0)