[modules] Maintain array of cells for imports and local exports.

This makes use of the newly introduced cell indices to speed up variable
accesses. Imports and local exports are now directly stored in (separate)
arrays. In the future, we may merge the two arrays into a single one, or
even into the module context.

This CL also replaces the LoadImport and LoadExport runtime functions with
a single LoadVariable taking a variable index as argument (rather than a
name).

BUG=v8:1569

Review-Url: https://codereview.chromium.org/2465283004
Cr-Commit-Position: refs/heads/master@{#40808}

src/ast/modules.cc

@@ -212,6 +212,13 @@ void ModuleDescriptor::MakeIndirectExportsExplicit(Zone* zone) {
   }
 }
 
+ModuleDescriptor::CellIndexKind ModuleDescriptor::GetCellIndexKind(
+    int cell_index) {
+  if (cell_index > 0) return kExport;
+  if (cell_index < 0) return kImport;
+  return kInvalid;
+}
+
 void ModuleDescriptor::AssignCellIndices() {
   int export_index = 1;
   for (auto it = regular_exports_.begin(); it != regular_exports_.end();) {

src/ast/modules.h

@@ -86,10 +86,12 @@ class ModuleDescriptor : public ZoneObject {
     int module_request;
 
     // Import/export entries that are associated with a MODULE-allocated
-    // variable use the cell_index value to encode the location of their cell.
-    // Negative values are used for imports and positive values for exports.
-    // For entries that are not associated with a MODULE-allocated variable,
-    // cell_index is 0.
+    // variable (i.e. regular_imports and regular_exports after Validate) use
+    // the cell_index value to encode the location of their cell.  During
+    // variable allocation, this will be be copied into the variable's index
+    // field.
+    // Entries that are not associated with a MODULE-allocated variable have
+    // GetCellIndexKind(cell_index) == kInvalid.
     int cell_index;
 
     // TODO(neis): Remove local_name component?
@@ -109,6 +111,9 @@ class ModuleDescriptor : public ZoneObject {
                               Handle<ModuleInfoEntry> entry);
   };
 
+  enum CellIndexKind { kInvalid, kExport, kImport };
+  static CellIndexKind GetCellIndexKind(int cell_index);
+
   // Module requests.
   const ZoneMap<const AstRawString*, int>& module_requests() const {
     return module_requests_;

src/debug/debug-scopes.cc

@@ -794,29 +794,14 @@ void ScopeIterator::CopyModuleVarsToScopeObject(Handle<ScopeInfo> scope_info,
           ->value();
   for (int i = 0; i < module_variable_count; ++i) {
     Handle<String> local_name;
-    bool is_export;
+    Handle<Object> value;
     {
       String* name;
       int index;
       scope_info->ModuleVariable(i, &name, &index);
       CHECK(!ScopeInfo::VariableIsSynthetic(name));
       local_name = handle(name, isolate);
-      DCHECK_NE(index, 0);
-      is_export = index > 0;
-    }
-
-    Handle<Object> value;
-    if (is_export) {
-      value =
-          Module::LoadExport(handle(context->module(), isolate), local_name);
-    } else {
-      Handle<ModuleInfo> module_info(scope_info->ModuleDescriptorInfo(),
-                                     isolate);
-      Handle<ModuleInfoEntry> entry =
-          ModuleInfo::LookupRegularImport(module_info, local_name);
-      Handle<String> import_name(String::cast(entry->import_name()), isolate);
-      value = Module::LoadImport(handle(context->module(), isolate),
-                                 import_name, entry->module_request());
+      value = Module::LoadVariable(handle(context->module(), isolate), index);
     }
 
     // Reflect variables under TDZ as undefined in scope object.

src/factory.cc

@@ -1817,6 +1817,10 @@ Handle<Module> Factory::NewModule(Handle<SharedFunctionInfo> code) {
                                  isolate());
   Handle<ObjectHashTable> exports =
       ObjectHashTable::New(isolate(), module_info->RegularExportCount());
+  Handle<FixedArray> regular_exports =
+      NewFixedArray(module_info->RegularExportCount());
+  Handle<FixedArray> regular_imports =
+      NewFixedArray(module_info->regular_imports()->length());
   int requested_modules_length = module_info->module_requests()->length();
   Handle<FixedArray> requested_modules =
       requested_modules_length > 0 ? NewFixedArray(requested_modules_length)
@@ -1825,6 +1829,8 @@ Handle<Module> Factory::NewModule(Handle<SharedFunctionInfo> code) {
   Handle<Module> module = Handle<Module>::cast(NewStruct(MODULE_TYPE));
   module->set_code(*code);
   module->set_exports(*exports);
+  module->set_regular_exports(*regular_exports);
+  module->set_regular_imports(*regular_imports);
   module->set_hash(isolate()->GenerateIdentityHash(Smi::kMaxValue));
   module->set_module_namespace(isolate()->heap()->undefined_value());
   module->set_requested_modules(*requested_modules);

src/interpreter/bytecode-generator.cc

@@ -1865,26 +1865,11 @@ void BytecodeGenerator::BuildVariableLoad(Variable* variable,
       break;
     }
     case VariableLocation::MODULE: {
-      ModuleDescriptor* descriptor = scope()->GetModuleScope()->module();
-      if (variable->IsExport()) {
-        auto it = descriptor->regular_exports().find(variable->raw_name());
-        DCHECK(it != descriptor->regular_exports().end());
-        Register export_name = register_allocator()->NewRegister();
-        builder()
-            ->LoadLiteral(it->second->export_name->string())
-            .StoreAccumulatorInRegister(export_name)
-            .CallRuntime(Runtime::kLoadModuleExport, export_name);
-      } else {
-        auto it = descriptor->regular_imports().find(variable->raw_name());
-        DCHECK(it != descriptor->regular_imports().end());
-        RegisterList args = register_allocator()->NewRegisterList(2);
-        builder()
-            ->LoadLiteral(it->second->import_name->string())
-            .StoreAccumulatorInRegister(args[0])
-            .LoadLiteral(Smi::FromInt(it->second->module_request))
-            .StoreAccumulatorInRegister(args[1])
-            .CallRuntime(Runtime::kLoadModuleImport, args);
-      }
+      Register index = register_allocator()->NewRegister();
+      builder()
+          ->LoadLiteral(Smi::FromInt(variable->index()))
+          .StoreAccumulatorInRegister(index)
+          .CallRuntime(Runtime::kLoadModuleVariable, index);
       if (hole_check_mode == HoleCheckMode::kRequired) {
         BuildThrowIfHole(variable->name());
       }
@@ -2052,23 +2037,17 @@ void BytecodeGenerator::BuildVariableAssignment(Variable* variable,
       // assignments for them.
       DCHECK(variable->IsExport());
 
-      ModuleDescriptor* mod = scope()->GetModuleScope()->module();
-      // There may be several export names for this local name, but it doesn't
-      // matter which one we pick, as they all map to the same cell.
-      auto it = mod->regular_exports().find(variable->raw_name());
-      DCHECK(it != mod->regular_exports().end());
-
       RegisterList args = register_allocator()->NewRegisterList(2);
       builder()
           ->StoreAccumulatorInRegister(args[1])
-          .LoadLiteral(it->second->export_name->string())
+          .LoadLiteral(Smi::FromInt(variable->index()))
           .StoreAccumulatorInRegister(args[0]);
       if (hole_check_mode == HoleCheckMode::kRequired) {
-        builder()->CallRuntime(Runtime::kLoadModuleExport, args[0]);
+        builder()->CallRuntime(Runtime::kLoadModuleVariable, args[0]);
         BuildHoleCheckForVariableAssignment(variable, op);
       }
       builder()
-          ->CallRuntime(Runtime::kStoreModuleExport, args)
+          ->CallRuntime(Runtime::kStoreModuleVariable, args)
           .LoadAccumulatorWithRegister(args[1]);
       break;
     }

src/objects-inl.h

@@ -5781,6 +5781,8 @@ ACCESSORS(JSFixedArrayIterator, initial_next, JSFunction, kNextOffset)
 
 ACCESSORS(Module, code, Object, kCodeOffset)
 ACCESSORS(Module, exports, ObjectHashTable, kExportsOffset)
+ACCESSORS(Module, regular_exports, FixedArray, kRegularExportsOffset)
+ACCESSORS(Module, regular_imports, FixedArray, kRegularImportsOffset)
 ACCESSORS(Module, module_namespace, HeapObject, kModuleNamespaceOffset)
 ACCESSORS(Module, requested_modules, FixedArray, kRequestedModulesOffset)
 SMI_ACCESSORS(Module, hash, kHashOffset)

src/objects.cc

@@ -19826,6 +19826,22 @@ class Module::ResolveSet
   Zone* zone_;
 };
 
+namespace {
+
+int ExportIndex(int cell_index) {
+  DCHECK_EQ(ModuleDescriptor::GetCellIndexKind(cell_index),
+            ModuleDescriptor::kExport);
+  return cell_index - 1;
+}
+
+int ImportIndex(int cell_index) {
+  DCHECK_EQ(ModuleDescriptor::GetCellIndexKind(cell_index),
+            ModuleDescriptor::kImport);
+  return -cell_index - 1;
+}
+
+}  // anonymous namespace
+
 void Module::CreateIndirectExport(Handle<Module> module, Handle<String> name,
                                   Handle<ModuleInfoEntry> entry) {
   Isolate* isolate = module->GetIsolate();
@@ -19835,11 +19851,15 @@ void Module::CreateIndirectExport(Handle<Module> module, Handle<String> name,
   module->set_exports(*exports);
 }
 
-void Module::CreateExport(Handle<Module> module, Handle<FixedArray> names) {
+void Module::CreateExport(Handle<Module> module, int cell_index,
+                          Handle<FixedArray> names) {
   DCHECK_LT(0, names->length());
   Isolate* isolate = module->GetIsolate();
+
   Handle<Cell> cell =
       isolate->factory()->NewCell(isolate->factory()->undefined_value());
+  module->regular_exports()->set(ExportIndex(cell_index), *cell);
+
   Handle<ObjectHashTable> exports(module->exports(), isolate);
   for (int i = 0, n = names->length(); i < n; ++i) {
     Handle<String> name(String::cast(names->get(i)), isolate);
@@ -19849,24 +19869,33 @@ void Module::CreateExport(Handle<Module> module, Handle<FixedArray> names) {
   module->set_exports(*exports);
 }
 
-void Module::StoreExport(Handle<Module> module, Handle<String> name,
-                         Handle<Object> value) {
-  Handle<Cell> cell(Cell::cast(module->exports()->Lookup(name)));
-  cell->set_value(*value);
-}
-
-Handle<Object> Module::LoadExport(Handle<Module> module, Handle<String> name) {
+Handle<Object> Module::LoadVariable(Handle<Module> module, int cell_index) {
   Isolate* isolate = module->GetIsolate();
-  Handle<Object> object(module->exports()->Lookup(name), isolate);
+  Handle<Object> object;
+  switch (ModuleDescriptor::GetCellIndexKind(cell_index)) {
+    case ModuleDescriptor::kImport:
+      object = handle(module->regular_imports()->get(ImportIndex(cell_index)),
+                      isolate);
+      break;
+    case ModuleDescriptor::kExport:
+      object = handle(module->regular_exports()->get(ExportIndex(cell_index)),
+                      isolate);
+      break;
+    case ModuleDescriptor::kInvalid:
+      UNREACHABLE();
+      break;
+  }
   return handle(Handle<Cell>::cast(object)->value(), isolate);
 }
 
-Handle<Object> Module::LoadImport(Handle<Module> module, Handle<String> name,
-                                  int module_request) {
+void Module::StoreVariable(Handle<Module> module, int cell_index,
+                           Handle<Object> value) {
   Isolate* isolate = module->GetIsolate();
-  Handle<Module> requested_module(
-      Module::cast(module->requested_modules()->get(module_request)), isolate);
-  return Module::LoadExport(requested_module, name);
+  DCHECK_EQ(ModuleDescriptor::GetCellIndexKind(cell_index),
+            ModuleDescriptor::kExport);
+  Handle<Object> object(module->regular_exports()->get(ExportIndex(cell_index)),
+                        isolate);
+  Handle<Cell>::cast(object)->set_value(*value);
 }
 
 MaybeHandle<Cell> Module::ResolveImport(Handle<Module> module,
@@ -20021,10 +20050,12 @@ bool Module::Instantiate(Handle<Module> module, v8::Local<v8::Context> context,
                                  isolate);
 
   // Set up local exports.
+  // TODO(neis): Create regular_exports array here instead of in factory method?
   for (int i = 0, n = module_info->RegularExportCount(); i < n; ++i) {
+    int cell_index = module_info->RegularExportCellIndex(i);
     Handle<FixedArray> export_names(module_info->RegularExportExportNames(i),
                                     isolate);
-    CreateExport(module, export_names);
+    CreateExport(module, cell_index, export_names);
   }
 
   // Partially set up indirect exports.
@@ -20076,11 +20107,13 @@ bool Module::Instantiate(Handle<Module> module, v8::Local<v8::Context> context,
         isolate);
     MessageLocation loc(script, entry->beg_pos(), entry->end_pos());
     ResolveSet resolve_set(&zone);
-    if (ResolveImport(module, name, entry->module_request(), loc, true,
-                      &resolve_set)
-            .is_null()) {
+    Handle<Cell> cell;
+    if (!ResolveImport(module, name, entry->module_request(), loc, true,
+                       &resolve_set)
+             .ToHandle(&cell)) {
       return false;
     }
+    module->regular_imports()->set(ImportIndex(entry->cell_index()), *cell);
   }
 
   // Resolve indirect exports.

src/objects.h

@@ -8179,7 +8179,15 @@ class Module : public Struct {
   // Module::ModuleVerify() for the precise invariant.
   DECL_ACCESSORS(code, Object)
 
-  // The export table.
+  // Arrays of cells corresponding to regular exports and regular imports.
+  // A cell's position in the array is determined by the cell index of the
+  // associated module entry (which coincides with the variable index of the
+  // associated variable).
+  DECL_ACCESSORS(regular_exports, FixedArray)
+  DECL_ACCESSORS(regular_imports, FixedArray)
+
+  // The complete export table, mapping an export name to its cell.
+  // TODO(neis): We may want to remove the regular exports from the table.
   DECL_ACCESSORS(exports, ObjectHashTable)
 
   // Hash for this object (a random non-zero Smi).
@@ -8210,12 +8218,9 @@ class Module : public Struct {
   // Implementation of spec operation ModuleEvaluation.
   static MUST_USE_RESULT MaybeHandle<Object> Evaluate(Handle<Module> module);
 
-  static Handle<Object> LoadExport(Handle<Module> module, Handle<String> name);
-  static void StoreExport(Handle<Module> module, Handle<String> name,
-                          Handle<Object> value);
-
-  static Handle<Object> LoadImport(Handle<Module> module, Handle<String> name,
-                                   int module_request);
+  static Handle<Object> LoadVariable(Handle<Module> module, int cell_index);
+  static void StoreVariable(Handle<Module> module, int cell_index,
+                            Handle<Object> value);
 
   // Get the namespace object for [module_request] of [module].  If it doesn't
   // exist yet, it is created.
@@ -8224,7 +8229,9 @@ class Module : public Struct {
 
   static const int kCodeOffset = HeapObject::kHeaderSize;
   static const int kExportsOffset = kCodeOffset + kPointerSize;
-  static const int kHashOffset = kExportsOffset + kPointerSize;
+  static const int kRegularExportsOffset = kExportsOffset + kPointerSize;
+  static const int kRegularImportsOffset = kRegularExportsOffset + kPointerSize;
+  static const int kHashOffset = kRegularImportsOffset + kPointerSize;
   static const int kModuleNamespaceOffset = kHashOffset + kPointerSize;
   static const int kRequestedModulesOffset =
       kModuleNamespaceOffset + kPointerSize;
@@ -8233,7 +8240,8 @@ class Module : public Struct {
  private:
   enum { kEvaluatedBit };
 
-  static void CreateExport(Handle<Module> module, Handle<FixedArray> names);
+  static void CreateExport(Handle<Module> module, int cell_index,
+                           Handle<FixedArray> names);
   static void CreateIndirectExport(Handle<Module> module, Handle<String> name,
                                    Handle<ModuleInfoEntry> entry);
 

src/runtime/runtime-module.cc

@@ -17,30 +17,21 @@ RUNTIME_FUNCTION(Runtime_GetModuleNamespace) {
   return *Module::GetModuleNamespace(module, module_request);
 }
 
-RUNTIME_FUNCTION(Runtime_LoadModuleExport) {
+RUNTIME_FUNCTION(Runtime_LoadModuleVariable) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 1);
-  CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
+  CONVERT_SMI_ARG_CHECKED(index, 0);
   Handle<Module> module(isolate->context()->module());
-  return *Module::LoadExport(module, name);
+  return *Module::LoadVariable(module, index);
 }
 
-RUNTIME_FUNCTION(Runtime_LoadModuleImport) {
+RUNTIME_FUNCTION(Runtime_StoreModuleVariable) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 2);
-  CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
-  CONVERT_SMI_ARG_CHECKED(module_request, 1);
-  Handle<Module> module(isolate->context()->module());
-  return *Module::LoadImport(module, name, module_request);
-}
-
-RUNTIME_FUNCTION(Runtime_StoreModuleExport) {
-  HandleScope scope(isolate);
-  DCHECK(args.length() == 2);
-  CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
+  CONVERT_SMI_ARG_CHECKED(index, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, value, 1);
   Handle<Module> module(isolate->context()->module());
-  Module::StoreExport(module, name, value);
+  Module::StoreVariable(module, index, value);
   return isolate->heap()->undefined_value();
 }
 

src/runtime/runtime.h

@@ -359,9 +359,8 @@ namespace internal {
 
 #define FOR_EACH_INTRINSIC_MODULE(F) \
   F(GetModuleNamespace, 1, 1)        \
-  F(LoadModuleExport, 1, 1)          \
-  F(LoadModuleImport, 2, 1)          \
-  F(StoreModuleExport, 2, 1)
+  F(LoadModuleVariable, 1, 1)        \
+  F(StoreModuleVariable, 2, 1)
 
 #define FOR_EACH_INTRINSIC_NUMBERS(F)  \
   F(IsValidSmi, 1, 1)                  \