Bug 1591747 - Unify JSFunction script_/lazy_ fields r=jandem
authorTed Campbell <tcampbell@mozilla.com>
Mon, 28 Oct 2019 08:26:32 +0000
changeset 499681 e53af14c60890ca102e6ddf0912f75af35438c1c
parent 499680 a9d7651009089ec16b5fe2d978604994183c2d04
child 499682 e2135df66bd70a704b0de260f5b9cb4324657001
push id99134
push usertcampbell@mozilla.com
push dateTue, 29 Oct 2019 18:36:57 +0000
treeherderautoland@e53af14c6089 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1591747
milestone72.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 1591747 - Unify JSFunction script_/lazy_ fields r=jandem Store js::BaseScript* to unify both the script_ and lazy_ fields of JSFunction. Remove the mutableScript() and setScript() accessors and make manual barriers more consistent. Depends on D50719 Differential Revision: https://phabricator.services.mozilla.com/D50720
js/src/vm/JSFunction.cpp
js/src/vm/JSFunction.h
js/src/vm/JSScript.cpp
--- a/js/src/vm/JSFunction.cpp
+++ b/js/src/vm/JSFunction.cpp
@@ -759,19 +759,23 @@ inline void JSFunction::trace(JSTracer* 
 
   TraceNullableEdge(trc, &atom_, "atom");
 
   if (isInterpreted()) {
     // Functions can be be marked as interpreted despite having no script
     // yet at some points when parsing, and can be lazy with no lazy script
     // for self-hosted code.
     if (hasScript() && !hasUncompletedScript()) {
-      TraceManuallyBarrieredEdge(trc, &u.scripted.s.script_, "script");
+      JSScript* script = static_cast<JSScript*>(u.scripted.s.script_);
+      TraceManuallyBarrieredEdge(trc, &script, "script");
+      u.scripted.s.script_ = script;
     } else if (hasLazyScript()) {
-      TraceManuallyBarrieredEdge(trc, &u.scripted.s.lazy_, "lazyScript");
+      LazyScript* lazy = static_cast<LazyScript*>(u.scripted.s.script_);
+      TraceManuallyBarrieredEdge(trc, &lazy, "lazy");
+      u.scripted.s.script_ = lazy;
     }
     // NOTE: The u.scripted.s.selfHostedLazy_ does not point to GC things.
 
     if (u.scripted.env_) {
       TraceManuallyBarrieredEdge(trc, &u.scripted.env_, "fun_environment");
     }
   }
 }
@@ -1673,17 +1677,17 @@ bool JSFunction::delazifyLazilyInterpret
   Rooted<PropertyName*> funName(cx, funAtom->asPropertyName());
   return cx->runtime()->cloneSelfHostedFunctionScript(cx, funName, fun);
 }
 
 void JSFunction::maybeRelazify(JSRuntime* rt) {
   // Try to relazify functions with a non-lazy script. Note: functions can be
   // marked as interpreted despite having no script yet at some points when
   // parsing.
-  if (!hasScript() || !u.scripted.s.script_) {
+  if (!hasScript() || hasUncompletedScript()) {
     return;
   }
 
   // Don't relazify functions in compartments that are active.
   Realm* realm = this->realm();
   if (!rt->allowRelazificationForTesting) {
     if (realm->compartment()->gcState.hasEnteredRealm) {
       return;
@@ -1703,39 +1707,38 @@ void JSFunction::maybeRelazify(JSRuntime
 
   // Don't relazify if the realm and/or runtime is instrumented to
   // collect code coverage for analysis.
   if (realm->collectCoverageForDebug()) {
     return;
   }
 
   // Don't relazify functions with JIT code.
-  if (!u.scripted.s.script_->isRelazifiable()) {
+  JSScript* script = nonLazyScript();
+  if (!script->isRelazifiable()) {
     return;
   }
 
   // To delazify self-hosted builtins we need the name of the function
   // to clone. This name is stored in the first extended slot.
   if (isSelfHostedBuiltin() &&
       (!isExtended() || !getExtendedSlot(LAZY_FUNCTION_NAME_SLOT).isString())) {
     return;
   }
 
-  JSScript* script = nonLazyScript();
-
   flags_.clearInterpreted();
   flags_.setInterpretedLazy();
 
   MOZ_ASSERT(!script->isAsync() && !script->isGenerator(),
              "Generator resume code in the JITs assumes non-lazy function");
 
   LazyScript* lazy = script->maybeLazyScript();
   if (lazy) {
-    u.scripted.s.lazy_ = lazy;
-    MOZ_ASSERT(!isSelfHostedBuiltin());
+    u.scripted.s.script_ = lazy;
+    MOZ_ASSERT(hasLazyScript());
   } else {
     // Lazy self-hosted builtins point to a SelfHostedLazyScript that may be
     // called from JIT scripted calls.
     u.scripted.s.selfHostedLazy_ = &rt->selfHostedLazyScript.ref();
     MOZ_ASSERT(isSelfHostedBuiltin());
     MOZ_ASSERT(isExtended());
     MOZ_ASSERT(GetClonedSelfHostedFunctionName(this));
   }
--- a/js/src/vm/JSFunction.h
+++ b/js/src/vm/JSFunction.h
@@ -344,19 +344,17 @@ class JSFunction : public js::NativeObje
         size_t wasmFuncIndex_;
         // for wasm that has been given a jit entry
         void** wasmJitEntry_;
       } extra;
     } native;
     struct {
       JSObject* env_; /* environment for new activations */
       union {
-        JSScript* script_;     /* interpreted bytecode descriptor or
-                                  null; use the accessor! */
-        js::LazyScript* lazy_; /* lazily compiled script, or nullptr */
+        js::BaseScript* script_;
         js::SelfHostedLazyScript* selfHostedLazy_;
       } s;
     } scripted;
   } u;
 
   // The |atom_| field can have different meanings depending on the function
   // type and flags. It is used for diagnostics, decompiling, and
   //
@@ -720,45 +718,43 @@ class JSFunction : public js::NativeObje
   // to be complete (see the comment above JSScript::initFromFunctionBox
   // callsite in JSScript::fullyInitFromEmitter).
   bool hasUncompletedScript() const {
     MOZ_ASSERT(hasScript());
     return !u.scripted.s.script_;
   }
 
   JSScript* nonLazyScript() const {
-    MOZ_ASSERT(!hasUncompletedScript());
+    MOZ_ASSERT(hasScript());
+    MOZ_ASSERT(u.scripted.s.script_);
+    return static_cast<JSScript*>(u.scripted.s.script_);
+  }
+
+  js::LazyScript* lazyScript() const {
+    MOZ_ASSERT(hasLazyScript());
+    MOZ_ASSERT(u.scripted.s.script_);
+    return static_cast<js::LazyScript*>(u.scripted.s.script_);
+  }
+
+  js::SelfHostedLazyScript* selfHostedLazyScript() const {
+    MOZ_ASSERT(hasSelfHostedLazyScript());
+    MOZ_ASSERT(u.scripted.s.selfHostedLazy_);
+    return u.scripted.s.selfHostedLazy_;
+  }
+
+  // Access fields defined on both lazy and non-lazy scripts.
+  js::BaseScript* baseScript() const {
+    MOZ_ASSERT(hasBaseScript());
+    MOZ_ASSERT(u.scripted.s.script_);
     return u.scripted.s.script_;
   }
 
   static bool getLength(JSContext* cx, js::HandleFunction fun,
                         uint16_t* length);
 
-  js::LazyScript* lazyScript() const {
-    MOZ_ASSERT(hasLazyScript());
-    MOZ_ASSERT(u.scripted.s.lazy_,
-               "JSFunction::lazy_ should also be valid when hasLazyScript()");
-    return u.scripted.s.lazy_;
-  }
-
-  js::SelfHostedLazyScript* selfHostedLazyScript() const {
-    MOZ_ASSERT(hasSelfHostedLazyScript() && u.scripted.s.selfHostedLazy_);
-    return u.scripted.s.selfHostedLazy_;
-  }
-
-  // Access fields defined on both lazy and non-lazy scripts. This should
-  // optimize away the branch since the union arms are compatible.
-  js::BaseScript* baseScript() const {
-    if (hasScript()) {
-      return nonLazyScript();
-    }
-    MOZ_ASSERT(hasLazyScript());
-    return lazyScript();
-  }
-
   js::GeneratorKind generatorKind() const {
     if (hasBaseScript()) {
       return baseScript()->generatorKind();
     }
     return js::GeneratorKind::NotGenerator;
   }
 
   bool isGenerator() const {
@@ -771,55 +767,57 @@ class JSFunction : public js::NativeObje
     }
     return js::FunctionAsyncKind::SyncFunction;
   }
 
   bool isAsync() const {
     return asyncKind() == js::FunctionAsyncKind::AsyncFunction;
   }
 
-  void setScript(JSScript* script) {
-    MOZ_ASSERT(realm() == script->realm());
-    mutableScript() = script;
+  void initScript(JSScript* script) {
+    MOZ_ASSERT_IF(script, realm() == script->realm());
+
+    u.scripted.s.script_ = script;
+    MOZ_ASSERT(hasScript());
   }
 
-  void initScript(JSScript* script) {
-    MOZ_ASSERT_IF(script, realm() == script->realm());
-    mutableScript().init(script);
+  void initLazyScript(js::LazyScript* lazy) {
+    MOZ_ASSERT(isInterpreted());
+    flags_.clearInterpreted();
+    flags_.setInterpretedLazy();
+    u.scripted.s.script_ = lazy;
+    MOZ_ASSERT(hasLazyScript());
   }
 
+  void initSelfHostLazyScript(js::SelfHostedLazyScript* lazy) {
+    MOZ_ASSERT(isInterpreted());
+    flags_.clearInterpreted();
+    flags_.setInterpretedLazy();
+    u.scripted.s.selfHostedLazy_ = lazy;
+    MOZ_ASSERT(hasSelfHostedLazyScript());
+  }
+
+  // Transform from lazy to non-lazy mode.
   void setUnlazifiedScript(JSScript* script) {
     MOZ_ASSERT(isInterpretedLazy());
     if (hasLazyScript()) {
       // Trigger a pre barrier on the lazy script being overwritten.
-      js::LazyScript::writeBarrierPre(lazyScript());
+      if (shadowZone()->needsIncrementalBarrier()) {
+        js::LazyScript::writeBarrierPre(lazyScript());
+      }
+
       if (!lazyScript()->maybeScript()) {
         lazyScript()->initScript(script);
       }
     }
     flags_.clearInterpretedLazy();
     flags_.setInterpreted();
     initScript(script);
   }
 
-  void initLazyScript(js::LazyScript* lazy) {
-    MOZ_ASSERT(isInterpreted());
-    flags_.clearInterpreted();
-    flags_.setInterpretedLazy();
-    u.scripted.s.lazy_ = lazy;
-  }
-
-  void initSelfHostLazyScript(js::SelfHostedLazyScript* lazy) {
-    MOZ_ASSERT(isInterpreted());
-    MOZ_ASSERT(isSelfHostedBuiltin());
-    flags_.clearInterpreted();
-    flags_.setInterpretedLazy();
-    u.scripted.s.selfHostedLazy_ = lazy;
-  }
-
   JSNative native() const {
     MOZ_ASSERT(isNative());
     return u.native.func_;
   }
 
   JSNative maybeNative() const { return isInterpreted() ? nullptr : native(); }
 
   void initNative(js::Native native, const JSJitInfo* jitInfo) {
@@ -886,19 +884,16 @@ class JSFunction : public js::NativeObje
   }
   static unsigned offsetOfNativeOrEnv() {
     static_assert(
         offsetof(U, native.func_) == offsetof(U, scripted.env_),
         "U.native.func_ must be at the same offset as U.scripted.env_");
     return offsetOfNative();
   }
   static unsigned offsetOfScriptOrLazyScript() {
-    static_assert(
-        offsetof(U, scripted.s.script_) == offsetof(U, scripted.s.lazy_),
-        "U.scripted.s.script_ must be at the same offset as lazy_");
     return offsetof(JSFunction, u.scripted.s.script_);
   }
 
   static unsigned offsetOfJitInfo() {
     return offsetof(JSFunction, u.native.extra.jitInfo_);
   }
 
   inline void trace(JSTracer* trc);
@@ -914,21 +909,16 @@ class JSFunction : public js::NativeObje
    * Used to mark bound functions as such and make them constructible if the
    * target is. Also assigns the prototype and sets the name and correct length.
    */
   static bool finishBoundFunctionInit(JSContext* cx, js::HandleFunction bound,
                                       js::HandleObject targetObj,
                                       int32_t argCount);
 
  private:
-  js::GCPtrScript& mutableScript() {
-    MOZ_ASSERT(hasScript());
-    return *(js::GCPtrScript*)&u.scripted.s.script_;
-  }
-
   inline js::FunctionExtended* toExtended();
   inline const js::FunctionExtended* toExtended() const;
 
  public:
   inline bool isExtended() const {
     bool extended = flags_.isExtended();
     MOZ_ASSERT_IF(isTenured(),
                   extended == (asTenured().getAllocKind() ==
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -4087,17 +4087,18 @@ bool JSScript::fullyInitFromEmitter(JSCo
   // NOTE: JSScript is now constructed and should be linked in.
 
   // Link JSFunction to this JSScript.
   if (bce->sc->isFunctionBox()) {
     JSFunction* fun = bce->sc->asFunctionBox()->function();
     if (fun->isInterpretedLazy()) {
       fun->setUnlazifiedScript(script);
     } else {
-      fun->setScript(script);
+      MOZ_ASSERT(fun->hasUncompletedScript());
+      fun->initScript(script);
     }
   }
 
   // Part of the parse result – the scope containing each inner function – must
   // be stored in the inner function itself. Do this now that compilation is
   // complete and can no longer fail.
   bce->perScriptData().gcThingList().finishInnerFunctions();