[modules] Store Module metadata in per-Context EmbedderData

src/d8.cc

Index: src/d8.cc
diff --git a/src/d8.cc b/src/d8.cc
index 01801f80f6d30a158e944a992e0c823b1741f04e..d1f11a7189f0a9aa0f8f612028219d661ead176b 100644
--- a/src/d8.cc
+++ b/src/d8.cc
@@ -9,7 +9,7 @@
 
 #include <algorithm>
 #include <fstream>
-#include <map>
+#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -558,29 +558,68 @@ std::string EnsureAbsolutePath(const std::string& path,
   return IsAbsolutePath(path) ? path : dir_name + '/' + path;
 }
 
+// Per-context Module data, allowing sharing of module maps
+// across top-level module loads.
+class ModuleEmbedderData {
+ private:
+  class ModuleGlobalHash {
+   public:
+    explicit ModuleGlobalHash(Isolate* isolate) : isolate_(isolate) {}
+    size_t operator()(const Global<Module>& module) const {
+      return module.Get(isolate_)->GetIdentityHash();
+    }
+
+   private:
+    Isolate* isolate_;
+  };
+
+ public:
+  explicit ModuleEmbedderData(Isolate* isolate)
+      : module_to_directory_map(10, ModuleGlobalHash(isolate)) {}
+
+  // Map from normalized module specifiers to Modules.
+  std::unordered_map<std::string, Global<Module>> specifier_to_module_map;
+  // Map from Module to the directory that Module was loaded from.

[neis, 2016/10/06 09:44:46]

Nit: keep the comments both in singular or both in plural.

[adamk, 2016/10/06 20:47:01]

Thanks for keeping me honest. Went with singular.

+  std::unordered_map<Global<Module>, std::string, ModuleGlobalHash>
+      module_to_directory_map;
+};
+
+enum D8EmbedderDataFields {
+  kDebugIdIndex = Context::kDebugIdIndex,

[neis, 2016/10/06 09:44:46]

What is this for?

[adamk, 2016/10/06 20:47:01]

Added a comment (and removed the type name). Basically this is a fancy attempt
at making a future-proof kModuleEmbedderDataIndex = 1 :)

+  kModuleEmbedderDataIndex
+};
+
+void InitializeModuleEmbedderData(Local<Context> context) {
+  context->SetAlignedPointerInEmbedderData(
+      kModuleEmbedderDataIndex, new ModuleEmbedderData(context->GetIsolate()));
+}
+
+ModuleEmbedderData* GetModuleDataFromContext(Local<Context> context) {
+  return static_cast<ModuleEmbedderData*>(
+      context->GetAlignedPointerFromEmbedderData(kModuleEmbedderDataIndex));
+}
+
 MaybeLocal<Module> ResolveModuleCallback(Local<Context> context,
                                          Local<String> specifier,
-                                         Local<Module> referrer,
-                                         Local<Value> data) {
+                                         Local<Module> referrer) {
   Isolate* isolate = context->GetIsolate();
-  auto module_map = static_cast<std::map<std::string, Global<Module>>*>(
-      External::Cast(*data)->Value());
-  Local<String> dir_name = Local<String>::Cast(referrer->GetEmbedderData());
+  ModuleEmbedderData* d = GetModuleDataFromContext(context);
+  auto dir_name_it =
+      d->module_to_directory_map.find(Global<Module>(isolate, referrer));
+  CHECK(dir_name_it != d->module_to_directory_map.end());
   std::string absolute_path =
-      EnsureAbsolutePath(ToSTLString(specifier), ToSTLString(dir_name));
-  auto it = module_map->find(absolute_path);
-  if (it != module_map->end()) {
-    return it->second.Get(isolate);
-  }
-  return MaybeLocal<Module>();
+      EnsureAbsolutePath(ToSTLString(specifier), dir_name_it->second);
+  auto module_it = d->specifier_to_module_map.find(absolute_path);
+  CHECK(module_it != d->specifier_to_module_map.end());
+  return module_it->second.Get(isolate);
 }
 
 }  // anonymous namespace
 
-MaybeLocal<Module> Shell::FetchModuleTree(
-    Isolate* isolate, const std::string& file_name,
-    std::map<std::string, Global<Module>>* module_map) {
+MaybeLocal<Module> Shell::FetchModuleTree(Local<Context> context,
+                                          const std::string& file_name) {
   DCHECK(IsAbsolutePath(file_name));
+  Isolate* isolate = context->GetIsolate();
   TryCatch try_catch(isolate);
   try_catch.SetVerbose(true);
   Local<String> source_text = ReadFile(isolate, file_name.c_str());
@@ -597,19 +636,20 @@ MaybeLocal<Module> Shell::FetchModuleTree(
     ReportException(isolate, &try_catch);
     return MaybeLocal<Module>();
   }
-  module_map->insert(
+
+  ModuleEmbedderData* d = GetModuleDataFromContext(context);
+  d->specifier_to_module_map.insert(
       std::make_pair(file_name, Global<Module>(isolate, module)));
 
   std::string dir_name = DirName(file_name);
-  module->SetEmbedderData(
-      String::NewFromUtf8(isolate, dir_name.c_str(), NewStringType::kNormal)
-          .ToLocalChecked());
+  d->module_to_directory_map.insert(
+      std::make_pair(Global<Module>(isolate, module), dir_name));

[neis, 2016/10/06 09:44:46]

Can you add checks here that these insertions succeeded?

[adamk, 2016/10/06 20:47:01]

Done.

 
   for (int i = 0, length = module->GetModuleRequestsLength(); i < length; ++i) {
     Local<String> name = module->GetModuleRequest(i);
     std::string absolute_path = EnsureAbsolutePath(ToSTLString(name), dir_name);
-    if (!module_map->count(absolute_path)) {
-      if (FetchModuleTree(isolate, absolute_path, module_map).IsEmpty()) {
+    if (!d->specifier_to_module_map.count(absolute_path)) {
+      if (FetchModuleTree(context, absolute_path).IsEmpty()) {
         return MaybeLocal<Module>();
       }
     }
@@ -621,14 +661,16 @@ MaybeLocal<Module> Shell::FetchModuleTree(
 bool Shell::ExecuteModule(Isolate* isolate, const char* file_name) {
   HandleScope handle_scope(isolate);
 
+  PerIsolateData* data = PerIsolateData::Get(isolate);
+  Local<Context> realm = data->realms_[data->realm_current_].Get(isolate);
+  Context::Scope context_scope(realm);
+
   std::string absolute_path =
       EnsureAbsolutePath(file_name, GetWorkingDirectory());
   std::replace(absolute_path.begin(), absolute_path.end(), '\\', '/');
 
   Local<Module> root_module;
-  std::map<std::string, Global<Module>> module_map;
-  if (!FetchModuleTree(isolate, absolute_path, &module_map)
-           .ToLocal(&root_module)) {
+  if (!FetchModuleTree(realm, absolute_path).ToLocal(&root_module)) {
     return false;
   }
 
@@ -636,16 +678,9 @@ bool Shell::ExecuteModule(Isolate* isolate, const char* file_name) {
   try_catch.SetVerbose(true);
 
   MaybeLocal<Value> maybe_result;
-  {
-    PerIsolateData* data = PerIsolateData::Get(isolate);
-    Local<Context> realm = data->realms_[data->realm_current_].Get(isolate);
-    Context::Scope context_scope(realm);
-
-    if (root_module->Instantiate(realm, ResolveModuleCallback,
-                                 External::New(isolate, &module_map))) {
-      maybe_result = root_module->Evaluate(realm);
-      EmptyMessageQueues(isolate);
-    }
+  if (root_module->Instantiate(realm, ResolveModuleCallback)) {
+    maybe_result = root_module->Evaluate(realm);
+    EmptyMessageQueues(isolate);
   }
   Local<Value> result;
   if (!maybe_result.ToLocal(&result)) {
@@ -775,6 +810,7 @@ MaybeLocal<Context> Shell::CreateRealm(
     try_catch.ReThrow();
     return MaybeLocal<Context>();
   }
+  InitializeModuleEmbedderData(context);
   data->realms_[index].Reset(isolate, context);
   args.GetReturnValue().Set(index);
   return context;
@@ -1476,6 +1512,7 @@ Local<Context> Shell::CreateEvaluationContext(Isolate* isolate) {
   EscapableHandleScope handle_scope(isolate);
   Local<Context> context = Context::New(isolate, NULL, global_template);
   DCHECK(!context.IsEmpty());
+  InitializeModuleEmbedderData(context);
   Context::Scope scope(context);
 
   i::Factory* factory = reinterpret_cast<i::Isolate*>(isolate)->factory();