Bug 1420412 - Use a single slot to store the module environment record r=anba
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 24 Nov 2017 15:52:22 +0000
changeset 393610 ebe3e6a61f6772d6fce2ad5df87e3c8329cec919
parent 393609 ea8096f3bd04c16068708b1afaee77edef2dd3c1
child 393611 6ecf891cc6ce23f8b2a2635f171903220343baf4
push id32967
push useraciure@mozilla.com
push dateFri, 24 Nov 2017 22:04:51 +0000
treeherdermozilla-central@4ce67352b2a9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1420412
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1420412 - Use a single slot to store the module environment record r=anba
js/src/builtin/Module.js
js/src/builtin/ModuleObject.cpp
js/src/builtin/ModuleObject.h
js/src/builtin/SelfHostingDefines.h
js/src/builtin/TestingFunctions.cpp
js/src/vm/SelfHosting.cpp
--- a/js/src/builtin/Module.js
+++ b/js/src/builtin/Module.js
@@ -237,18 +237,21 @@ function GetModuleEnvironment(module)
 {
     assert(IsModule(module), "Non-module passed to GetModuleEnvironment");
 
     // Check for a previous failed attempt to instantiate this module. This can
     // only happen due to a bug in the module loader.
     if (module.status === MODULE_STATUS_ERRORED)
         ThrowInternalError(JSMSG_MODULE_INSTANTIATE_FAILED, module.status);
 
+    assert(module.status >= MODULE_STATUS_INSTANTIATING,
+           "Attempt to access module environement before instantation");
+
     let env = UnsafeGetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT);
-    assert(env === undefined || IsModuleEnvironment(env),
+    assert(IsModuleEnvironment(env),
            "Module environment slot contains unexpected value");
 
     return env;
 }
 
 function RecordModuleError(module, error)
 {
     // Set the module's status to 'errored' to indicate a failed module
@@ -438,17 +441,17 @@ function ModuleDeclarationEnvironmentSet
                "Unexpected failure to resolve export in ModuleDeclarationEnvironmentSetup");
         if (!resolution.resolved) {
             return ResolutionError(resolution, "indirectExport", e.exportName,
                                    e.lineNumber, e.columnNumber);
         }
     }
 
     // Steps 5-6
-    CreateModuleEnvironment(module);
+    // Note that we have already created the environment by this point.
     let env = GetModuleEnvironment(module);
 
     // Step 8
     let importEntries = module.importEntries;
     for (let i = 0; i < importEntries.length; i++) {
         let imp = importEntries[i];
         let importedModule = CallModuleResolveHook(module, imp.moduleRequest,
                                                    MODULE_STATUS_INSTANTIATING);
--- a/js/src/builtin/ModuleObject.cpp
+++ b/js/src/builtin/ModuleObject.cpp
@@ -773,26 +773,34 @@ ModuleObject::finalize(js::FreeOp* fop, 
     if (self->hasImportBindings())
         fop->delete_(&self->importBindings());
     if (IndirectBindingMap* bindings = self->namespaceBindings())
         fop->delete_(bindings);
     if (FunctionDeclarationVector* funDecls = self->functionDeclarations())
         fop->delete_(funDecls);
 }
 
+ModuleEnvironmentObject&
+ModuleObject::initialEnvironment() const
+{
+    Value value = getReservedSlot(EnvironmentSlot);
+    return value.toObject().as<ModuleEnvironmentObject>();
+}
+
 ModuleEnvironmentObject*
 ModuleObject::environment() const
 {
     MOZ_ASSERT(status() != MODULE_STATUS_ERRORED);
 
-    Value value = getReservedSlot(EnvironmentSlot);
-    if (value.isUndefined())
+    // According to the spec the environment record is created during
+    // instantiation, but we create it earlier than that.
+    if (status() < MODULE_STATUS_INSTANTIATED)
         return nullptr;
 
-    return &value.toObject().as<ModuleEnvironmentObject>();
+    return &initialEnvironment();
 }
 
 bool
 ModuleObject::hasImportBindings() const
 {
     // Import bindings may not be present if we hit OOM in initialization.
     return !getReservedSlot(ImportBindingsSlot).isUndefined();
 }
@@ -847,17 +855,17 @@ ModuleObject::init(HandleScript script)
 {
     initReservedSlot(ScriptSlot, PrivateValue(script));
     initReservedSlot(StatusSlot, Int32Value(MODULE_STATUS_ERRORED));
 }
 
 void
 ModuleObject::setInitialEnvironment(HandleModuleEnvironmentObject initialEnvironment)
 {
-    initReservedSlot(InitialEnvironmentSlot, ObjectValue(*initialEnvironment));
+    initReservedSlot(EnvironmentSlot, ObjectValue(*initialEnvironment));
 }
 
 void
 ModuleObject::initImportExportData(HandleArrayObject requestedModules,
                                    HandleArrayObject importEntries,
                                    HandleArrayObject localExportEntries,
                                    HandleArrayObject indirectExportEntries,
                                    HandleArrayObject starExportEntries)
@@ -987,22 +995,16 @@ ModuleObject::hostDefinedField() const
 }
 
 void
 ModuleObject::setHostDefinedField(const JS::Value& value)
 {
     setReservedSlot(HostDefinedSlot, value);
 }
 
-ModuleEnvironmentObject&
-ModuleObject::initialEnvironment() const
-{
-    return getReservedSlot(InitialEnvironmentSlot).toObject().as<ModuleEnvironmentObject>();
-}
-
 Scope*
 ModuleObject::enclosingScope() const
 {
     return script()->enclosingScope();
 }
 
 /* static */ void
 ModuleObject::trace(JSTracer* trc, JSObject* obj)
@@ -1018,42 +1020,33 @@ ModuleObject::trace(JSTracer* trc, JSObj
         module.importBindings().trace(trc);
     if (IndirectBindingMap* bindings = module.namespaceBindings())
         bindings->trace(trc);
 
     if (FunctionDeclarationVector* funDecls = module.functionDeclarations())
         funDecls->trace(trc);
 }
 
-void
-ModuleObject::createEnvironment()
-{
-    // The environment has already been created, we just neet to set it in the
-    // right slot.
-    MOZ_ASSERT(!getReservedSlot(InitialEnvironmentSlot).isUndefined());
-    MOZ_ASSERT(getReservedSlot(EnvironmentSlot).isUndefined());
-    setReservedSlot(EnvironmentSlot, getReservedSlot(InitialEnvironmentSlot));
-}
-
 bool
 ModuleObject::noteFunctionDeclaration(JSContext* cx, HandleAtom name, HandleFunction fun)
 {
     FunctionDeclarationVector* funDecls = functionDeclarations();
     if (!funDecls->emplaceBack(name, fun)) {
         ReportOutOfMemory(cx);
         return false;
     }
 
     return true;
 }
 
 /* static */ bool
 ModuleObject::instantiateFunctionDeclarations(JSContext* cx, HandleModuleObject self)
 {
 #ifdef DEBUG
+    MOZ_ASSERT(self->status() == MODULE_STATUS_INSTANTIATING);
     if (!AssertFrozen(cx, self))
         return false;
 #endif
 
     FunctionDeclarationVector* funDecls = self->functionDeclarations();
     if (!funDecls) {
         JS_ReportErrorASCII(cx, "Module function declarations have already been instantiated");
         return false;
@@ -1089,16 +1082,17 @@ ModuleObject::instantiateFunctionDeclara
     self->setReservedSlot(FunctionDeclarationsSlot, UndefinedValue());
     return true;
 }
 
 /* static */ bool
 ModuleObject::execute(JSContext* cx, HandleModuleObject self, MutableHandleValue rval)
 {
 #ifdef DEBUG
+    MOZ_ASSERT(self->status() == MODULE_STATUS_EVALUATING);
     if (!AssertFrozen(cx, self))
         return false;
 #endif
 
     RootedScript script(cx, self->script());
     RootedModuleEnvironmentObject scope(cx, self->environment());
     if (!scope) {
         JS_ReportErrorASCII(cx, "Module declarations have not yet been instantiated");
--- a/js/src/builtin/ModuleObject.h
+++ b/js/src/builtin/ModuleObject.h
@@ -234,17 +234,16 @@ using FunctionDeclarationVector = GCVect
 using ModuleStatus = int32_t;
 
 class ModuleObject : public NativeObject
 {
   public:
     enum ModuleSlot
     {
         ScriptSlot = 0,
-        InitialEnvironmentSlot,
         EnvironmentSlot,
         NamespaceSlot,
         StatusSlot,
         ErrorSlot,
         HostDefinedSlot,
         RequestedModulesSlot,
         ImportEntriesSlot,
         LocalExportEntriesSlot,
@@ -305,19 +304,16 @@ class ModuleObject : public NativeObject
     JSObject* namespaceExports();
     IndirectBindingMap* namespaceBindings();
 
     static bool Instantiate(JSContext* cx, HandleModuleObject self);
     static bool Evaluate(JSContext* cx, HandleModuleObject self);
 
     void setHostDefinedField(const JS::Value& value);
 
-    // For intrinsic_CreateModuleEnvironment.
-    void createEnvironment();
-
     // For BytecodeEmitter.
     bool noteFunctionDeclaration(JSContext* cx, HandleAtom name, HandleFunction fun);
 
     // For intrinsic_InstantiateModuleFunctionDeclarations.
     static bool instantiateFunctionDeclarations(JSContext* cx, HandleModuleObject self);
 
     // For intrinsic_ExecuteModule.
     static bool execute(JSContext* cx, HandleModuleObject self, MutableHandleValue rval);
--- a/js/src/builtin/SelfHostingDefines.h
+++ b/js/src/builtin/SelfHostingDefines.h
@@ -95,21 +95,21 @@
 #define REGEXP_FLAGS_SLOT 2
 
 #define REGEXP_IGNORECASE_FLAG  0x01
 #define REGEXP_GLOBAL_FLAG      0x02
 #define REGEXP_MULTILINE_FLAG   0x04
 #define REGEXP_STICKY_FLAG      0x08
 #define REGEXP_UNICODE_FLAG     0x10
 
-#define MODULE_OBJECT_ENVIRONMENT_SLOT        2
-#define MODULE_OBJECT_STATUS_SLOT             4
-#define MODULE_OBJECT_ERROR_SLOT              5
-#define MODULE_OBJECT_DFS_INDEX_SLOT          16
-#define MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT 17
+#define MODULE_OBJECT_ENVIRONMENT_SLOT        1
+#define MODULE_OBJECT_STATUS_SLOT             3
+#define MODULE_OBJECT_ERROR_SLOT              4
+#define MODULE_OBJECT_DFS_INDEX_SLOT          15
+#define MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT 16
 
 #define MODULE_STATUS_ERRORED        0
 #define MODULE_STATUS_UNINSTANTIATED 1
 #define MODULE_STATUS_INSTANTIATING  2
 #define MODULE_STATUS_INSTANTIATED   3
 #define MODULE_STATUS_EVALUATING     4
 #define MODULE_STATUS_EVALUATED      5
 
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -4299,18 +4299,16 @@ SetRNGState(JSContext* cx, unsigned argc
 
 static ModuleEnvironmentObject*
 GetModuleEnvironment(JSContext* cx, HandleModuleObject module)
 {
     // Use the initial environment so that tests can check bindings exists
     // before they have been instantiated.
     RootedModuleEnvironmentObject env(cx, &module->initialEnvironment());
     MOZ_ASSERT(env);
-    MOZ_ASSERT_IF(module->environment(), module->environment() == env);
-
     return env;
 }
 
 static bool
 GetModuleEnvironmentNames(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     if (args.length() != 1) {
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -1998,27 +1998,16 @@ intrinsic_HostResolveImportedModule(JSCo
         return false;
     }
 
     args.rval().set(result);
     return true;
 }
 
 static bool
-intrinsic_CreateModuleEnvironment(JSContext* cx, unsigned argc, Value* vp)
-{
-    CallArgs args = CallArgsFromVp(argc, vp);
-    MOZ_ASSERT(args.length() == 1);
-    RootedModuleObject module(cx, &args[0].toObject().as<ModuleObject>());
-    module->createEnvironment();
-    args.rval().setUndefined();
-    return true;
-}
-
-static bool
 intrinsic_CreateImportBinding(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args.length() == 4);
     RootedModuleEnvironmentObject environment(cx, &args[0].toObject().as<ModuleEnvironmentObject>());
     RootedAtom importedName(cx, &args[1].toString()->asAtom());
     RootedModuleObject module(cx, &args[2].toObject().as<ModuleObject>());
     RootedAtom localName(cx, &args[3].toString()->asAtom());
@@ -2527,17 +2516,16 @@ static const JSFunctionSpec intrinsic_fu
     JS_FN("regexp_test_no_statics", regexp_test_no_statics, 2,0),
     JS_FN("regexp_construct_raw_flags", regexp_construct_raw_flags, 2,0),
 
     JS_FN("IsModule", intrinsic_IsInstanceOfBuiltin<ModuleObject>, 1, 0),
     JS_FN("CallModuleMethodIfWrapped",
           CallNonGenericSelfhostedMethod<Is<ModuleObject>>, 2, 0),
     JS_FN("HostResolveImportedModule", intrinsic_HostResolveImportedModule, 2, 0),
     JS_FN("IsModuleEnvironment", intrinsic_IsInstanceOfBuiltin<ModuleEnvironmentObject>, 1, 0),
-    JS_FN("CreateModuleEnvironment", intrinsic_CreateModuleEnvironment, 1, 0),
     JS_FN("CreateImportBinding", intrinsic_CreateImportBinding, 4, 0),
     JS_FN("CreateNamespaceBinding", intrinsic_CreateNamespaceBinding, 3, 0),
     JS_FN("InstantiateModuleFunctionDeclarations",
           intrinsic_InstantiateModuleFunctionDeclarations, 1, 0),
     JS_FN("ExecuteModule", intrinsic_ExecuteModule, 1, 0),
     JS_FN("NewModuleNamespace", intrinsic_NewModuleNamespace, 2, 0),
     JS_FN("AddModuleNamespaceBinding", intrinsic_AddModuleNamespaceBinding, 4, 0),
     JS_FN("ModuleNamespaceExports", intrinsic_ModuleNamespaceExports, 1, 0),