Bug 1505689 part 1 - Tidy up JSScript BaselineScript/IonScript methods. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 15 Aug 2019 16:12:25 +0000
changeset 488273 b66c924e9a1fc5ed5613707b56d13894f0a46518
parent 488272 bc99fb7a9d125b00d33e54d002facbef7e8e3f2b
child 488274 c437f2caa6d04b33e445d6defcb49a0b8e882e1b
push id36440
push userncsoregi@mozilla.com
push dateFri, 16 Aug 2019 03:57:48 +0000
treeherdermozilla-central@a58b7dc85887 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1505689
milestone70.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 1505689 part 1 - Tidy up JSScript BaselineScript/IonScript methods. r=tcampbell The main goal was to abstract the special BASELINE_DISABLED_SCRIPT and ION_*_SCRIPT values better. Each of these values now has its own setter method instead of passing these values to setBaselineScript and setIonScript. Differential Revision: https://phabricator.services.mozilla.com/D41573
js/src/jit/BaselineDebugModeOSR.cpp
js/src/jit/BaselineJIT.cpp
js/src/jit/BaselineJIT.h
js/src/jit/Ion.cpp
js/src/vm/JSScript-inl.h
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
--- a/js/src/jit/BaselineDebugModeOSR.cpp
+++ b/js/src/jit/BaselineDebugModeOSR.cpp
@@ -408,17 +408,17 @@ static bool RecompileBaselineScriptForDe
     return true;
   }
 
   JitSpew(JitSpew_BaselineDebugModeOSR, "Recompiling (%s:%u:%u) for %s",
           script->filename(), script->lineno(), script->column(),
           observing ? "DEBUGGING" : "NORMAL EXECUTION");
 
   AutoKeepJitScripts keepJitScripts(cx);
-  script->setBaselineScript(cx->runtime(), nullptr);
+  script->clearBaselineScript(cx->defaultFreeOp());
 
   MethodStatus status =
       BaselineCompile(cx, script, /* forceDebugMode = */ observing);
   if (status != Method_Compiled) {
     // We will only fail to recompile for debug mode due to OOM. Restore
     // the old baseline script in case something doesn't properly
     // propagate OOM.
     MOZ_ASSERT(status == Method_Error);
--- a/js/src/jit/BaselineJIT.cpp
+++ b/js/src/jit/BaselineJIT.cpp
@@ -216,31 +216,31 @@ MethodStatus jit::BaselineCompile(JSCont
   }
 
   MethodStatus status = compiler.compile();
 
   MOZ_ASSERT_IF(status == Method_Compiled, script->hasBaselineScript());
   MOZ_ASSERT_IF(status != Method_Compiled, !script->hasBaselineScript());
 
   if (status == Method_CantCompile) {
-    script->setBaselineScript(cx->runtime(), BASELINE_DISABLED_SCRIPT);
+    script->disableBaselineCompile(cx->runtime());
   }
 
   return status;
 }
 
 static MethodStatus CanEnterBaselineJIT(JSContext* cx, HandleScript script,
                                         AbstractFramePtr osrSourceFrame) {
   // Skip if the script has been disabled.
   if (!script->canBaselineCompile()) {
     return Method_Skipped;
   }
 
   if (!IsBaselineJitEnabled()) {
-    script->setBaselineScript(cx->runtime(), BASELINE_DISABLED_SCRIPT);
+    script->disableBaselineCompile(cx->runtime());
     return Method_CantCompile;
   }
 
   // This check is needed in the following corner case. Consider a function h,
   //
   //   function h(x) {
   //      if (!x)
   //        return;
@@ -260,22 +260,22 @@ static MethodStatus CanEnterBaselineJIT(
   // code. This is incorrect as h's baseline script does not have debug
   // instrumentation.
   if (osrSourceFrame && osrSourceFrame.isDebuggee() &&
       !DebugAPI::ensureExecutionObservabilityOfOsrFrame(cx, osrSourceFrame)) {
     return Method_Error;
   }
 
   if (script->length() > BaselineMaxScriptLength) {
-    script->setBaselineScript(cx->runtime(), BASELINE_DISABLED_SCRIPT);
+    script->disableBaselineCompile(cx->runtime());
     return Method_CantCompile;
   }
 
   if (script->nslots() > BaselineMaxScriptSlots) {
-    script->setBaselineScript(cx->runtime(), BASELINE_DISABLED_SCRIPT);
+    script->disableBaselineCompile(cx->runtime());
     return Method_CantCompile;
   }
 
   if (script->hasBaselineScript()) {
     return Method_Compiled;
   }
 
   // Check script warm-up counter.
@@ -289,17 +289,17 @@ static MethodStatus CanEnterBaselineJIT(
     return Method_Skipped;
   }
 
   if (!cx->realm()->ensureJitRealmExists(cx)) {
     return Method_Error;
   }
 
   if (script->hasForceInterpreterOp()) {
-    script->setBaselineScript(cx->runtime(), BASELINE_DISABLED_SCRIPT);
+    script->disableBaselineCompile(cx->runtime());
     return Method_CantCompile;
   }
 
   // Frames can be marked as debuggee frames independently of its underlying
   // script being a debuggee script, e.g., when performing
   // Debugger.Frame.prototype.eval.
   bool forceDebugInstrumentation =
       osrSourceFrame && osrSourceFrame.isDebuggee();
@@ -509,18 +509,17 @@ void BaselineScript::writeBarrierPre(Zon
 
 void BaselineScript::Trace(JSTracer* trc, BaselineScript* script) {
   script->trace(trc);
 }
 
 void BaselineScript::Destroy(JSFreeOp* fop, BaselineScript* script) {
   MOZ_ASSERT(!script->hasPendingIonBuilder());
 
-  // This allocation is tracked by JSScript::setBaselineScript /
-  // clearBaselineScript.
+  // This allocation is tracked by JSScript::setBaselineScriptImpl.
   fop->deleteUntracked(script);
 }
 
 void JS::DeletePolicy<js::jit::BaselineScript>::operator()(
     const js::jit::BaselineScript* script) {
   BaselineScript::Destroy(rt_->defaultFreeOp(),
                           const_cast<BaselineScript*>(script));
 }
@@ -915,17 +914,17 @@ void BaselineInterpreter::toggleCodeCove
   toggleCodeCoverageInstrumentationUnchecked(enable);
 }
 
 void jit::FinishDiscardBaselineScript(JSFreeOp* fop, JSScript* script) {
   MOZ_ASSERT(script->hasBaselineScript());
   MOZ_ASSERT(!script->jitScript()->active());
 
   BaselineScript* baseline = script->baselineScript();
-  script->setBaselineScript(fop, nullptr);
+  script->clearBaselineScript(fop);
   BaselineScript::Destroy(fop, baseline);
 }
 
 void jit::AddSizeOfBaselineData(JSScript* script,
                                 mozilla::MallocSizeOf mallocSizeOf,
                                 size_t* data) {
   if (script->hasBaselineScript()) {
     script->baselineScript()->addSizeOfIncludingThis(mallocSizeOf, data);
--- a/js/src/jit/BaselineJIT.h
+++ b/js/src/jit/BaselineJIT.h
@@ -378,28 +378,26 @@ struct BaselineScript final {
     return pendingBuilder_;
   }
   void setPendingIonBuilder(JSRuntime* rt, JSScript* script,
                             js::jit::IonBuilder* builder) {
     MOZ_ASSERT(script->baselineScript() == this);
     MOZ_ASSERT(!builder || !hasPendingIonBuilder());
 
     if (script->isIonCompilingOffThread()) {
-      script->setIonScript(rt, ION_PENDING_SCRIPT);
+      script->setHasPendingIonScript(rt);
     }
 
     pendingBuilder_ = builder;
 
     script->updateJitCodeRaw(rt);
   }
   void removePendingIonBuilder(JSRuntime* rt, JSScript* script) {
     setPendingIonBuilder(rt, script, nullptr);
-    if (script->maybeIonScript() == ION_PENDING_SCRIPT) {
-      script->setIonScript(rt, nullptr);
-    }
+    script->clearHasPendingIonScript(rt);
   }
 
   size_t allocBytes() const { return allocBytes_; }
 };
 static_assert(
     sizeof(BaselineScript) % sizeof(uintptr_t) == 0,
     "The data attached to the script must be aligned for fast JIT access.");
 
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -442,22 +442,22 @@ void jit::FinishOffThreadBuilder(JSRunti
   // Clear the recompiling flag of the old ionScript, since we continue to
   // use the old ionScript if recompiling fails.
   if (builder->script()->hasIonScript()) {
     builder->script()->ionScript()->clearRecompiling();
   }
 
   // Clean up if compilation did not succeed.
   if (builder->script()->isIonCompilingOffThread()) {
-    IonScript* ion = nullptr;
+    builder->script()->clearIsIonCompilingOffThread(runtime);
+
     AbortReasonOr<Ok> status = builder->getOffThreadStatus();
     if (status.isErr() && status.unwrapErr() == AbortReason::Disable) {
-      ion = ION_DISABLED_SCRIPT;
+      builder->script()->disableIon(runtime);
     }
-    builder->script()->setIonScript(runtime, ion);
   }
 
   // Free Ion LifoAlloc off-thread. Free on the main thread if this OOMs.
   if (!StartOffThreadIonFree(builder, locked)) {
     FreeIonBuilder(builder);
   }
 }
 
@@ -1026,24 +1026,20 @@ const OsiIndex* IonScript::getOsiIndex(u
   JitSpew(JitSpew_IonInvalidate, "IonScript %p has method %p raw %p",
           (void*)this, (void*)method(), method()->raw());
 
   MOZ_ASSERT(containsCodeAddress(retAddr));
   uint32_t disp = retAddr - method()->raw();
   return getOsiIndex(disp);
 }
 
-void IonScript::Trace(JSTracer* trc, IonScript* script) {
-  if (script != ION_DISABLED_SCRIPT) {
-    script->trace(trc);
-  }
-}
+void IonScript::Trace(JSTracer* trc, IonScript* script) { script->trace(trc); }
 
 void IonScript::Destroy(JSFreeOp* fop, IonScript* script) {
-  // This allocation is tracked by JSScript::setIonScript / clearIonScript.
+  // This allocation is tracked by JSScript::setIonScriptImpl.
   fop->deleteUntracked(script);
 }
 
 void JS::DeletePolicy<js::jit::IonScript>::operator()(
     const js::jit::IonScript* script) {
   IonScript::Destroy(rt_->defaultFreeOp(), const_cast<IonScript*>(script));
 }
 
@@ -1940,17 +1936,17 @@ static AbortReason IonCompile(JSContext*
     AutoLockHelperThreadState lock;
     if (!StartOffThreadIonCompile(builder, lock)) {
       JitSpew(JitSpew_IonAbort, "Unable to start off-thread ion compilation.");
       builder->graphSpewer().endFunction();
       return AbortReason::Alloc;
     }
 
     if (!recompile) {
-      builderScript->setIonScript(cx->runtime(), ION_COMPILING_SCRIPT);
+      builderScript->setIsIonCompilingOffThread(cx->runtime());
     }
 
     // The allocator and associated data will be destroyed after being
     // processed in the finishedOffThreadCompilations list.
     mozilla::Unused << alloc.release();
 
     return AbortReason::NoAbort;
   }
@@ -2676,17 +2672,17 @@ void jit::InvalidateAll(JSFreeOp* fop, Z
       JitSpew(JitSpew_IonInvalidate, "Invalidating all frames for GC");
       InvalidateActivation(fop, iter, true);
     }
   }
 }
 
 static void ClearIonScriptAfterInvalidation(JSContext* cx, JSScript* script,
                                             bool resetUses) {
-  script->setIonScript(cx->runtime(), nullptr);
+  script->clearIonScript(cx->defaultFreeOp());
 
   // Wait for the scripts to get warm again before doing another
   // compile, unless we are recompiling *because* a script got hot
   // (resetUses is false).
   if (resetUses) {
     script->resetWarmUpCounterToDelayIonCompilation();
   }
 }
@@ -2822,17 +2818,17 @@ void jit::Invalidate(JSContext* cx, JSSc
 
 void jit::FinishInvalidation(JSFreeOp* fop, JSScript* script) {
   if (!script->hasIonScript()) {
     return;
   }
 
   // In all cases, null out script->ion to avoid re-entry.
   IonScript* ion = script->ionScript();
-  script->setIonScript(fop, nullptr);
+  script->clearIonScript(fop);
 
   // If this script has Ion code on the stack, invalidated() will return
   // true. In this case we have to wait until destroying it.
   if (!ion->invalidated()) {
     jit::IonScript::Destroy(fop, ion);
   }
 }
 
@@ -2841,17 +2837,17 @@ void jit::ForbidCompilation(JSContext* c
           script->filename(), script->lineno(), script->column());
 
   CancelOffThreadIonCompile(script);
 
   if (script->hasIonScript()) {
     Invalidate(cx, script, false);
   }
 
-  script->setIonScript(cx->runtime(), ION_DISABLED_SCRIPT);
+  script->disableIon(cx->runtime());
 }
 
 AutoFlushICache* JSContext::autoFlushICache() const { return autoFlushICache_; }
 
 void JSContext::setAutoFlushICache(AutoFlushICache* afc) {
   autoFlushICache_ = afc;
 }
 
--- a/js/src/vm/JSScript-inl.h
+++ b/js/src/vm/JSScript-inl.h
@@ -155,50 +155,16 @@ inline js::Shape* JSScript::initialEnvir
   } else if (scope->is<js::EvalScope>()) {
     return scope->environmentShape();
   }
   return nullptr;
 }
 
 inline JSPrincipals* JSScript::principals() { return realm()->principals(); }
 
-inline void JSScript::setBaselineScript(
-    JSRuntime* rt, js::jit::BaselineScript* baselineScript) {
-  setBaselineScript(rt->defaultFreeOp(), baselineScript);
-}
-
-inline void JSScript::setBaselineScript(
-    JSFreeOp* fop, js::jit::BaselineScript* baselineScript) {
-  if (hasBaselineScript()) {
-    js::jit::BaselineScript::writeBarrierPre(zone(), baseline);
-    clearBaselineScript(fop);
-  }
-  MOZ_ASSERT(!ion || ion == ION_DISABLED_SCRIPT);
-
-  baseline = baselineScript;
-  if (hasBaselineScript()) {
-    AddCellMemory(this, baseline->allocBytes(), js::MemoryUse::BaselineScript);
-  }
-  resetWarmUpResetCounter();
-  updateJitCodeRaw(fop->runtime());
-}
-
-inline void JSScript::clearBaselineScript(JSFreeOp* fop) {
-  MOZ_ASSERT(hasBaselineScript());
-  fop->removeCellMemory(this, baseline->allocBytes(),
-                        js::MemoryUse::BaselineScript);
-  baseline = nullptr;
-}
-
-inline void JSScript::clearIonScript(JSFreeOp* fop) {
-  MOZ_ASSERT(hasIonScript());
-  fop->removeCellMemory(this, ion->allocBytes(), js::MemoryUse::IonScript);
-  ion = nullptr;
-}
-
 inline bool JSScript::ensureHasAnalyzedArgsUsage(JSContext* cx) {
   if (analyzedArgsUsage()) {
     return true;
   }
   return js::jit::AnalyzeArgumentsUsage(cx, this);
 }
 
 inline bool JSScript::isDebuggee() const {
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -1506,26 +1506,50 @@ js::PCCounts* ScriptCounts::getThrowCoun
 }
 
 size_t ScriptCounts::sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) {
   return mallocSizeOf(this) + pcCounts_.sizeOfExcludingThis(mallocSizeOf) +
          throwCounts_.sizeOfExcludingThis(mallocSizeOf) +
          ionCounts_->sizeOfIncludingThis(mallocSizeOf);
 }
 
-void JSScript::setIonScript(JSRuntime* rt, js::jit::IonScript* ionScript) {
-  setIonScript(rt->defaultFreeOp(), ionScript);
-}
-
-void JSScript::setIonScript(JSFreeOp* fop, js::jit::IonScript* ionScript) {
+void JSScript::setBaselineScriptImpl(JSRuntime* rt,
+                                     js::jit::BaselineScript* baselineScript) {
+  setBaselineScriptImpl(rt->defaultFreeOp(), baselineScript);
+}
+
+void JSScript::setBaselineScriptImpl(JSFreeOp* fop,
+                                     js::jit::BaselineScript* baselineScript) {
+  if (hasBaselineScript()) {
+    js::jit::BaselineScript::writeBarrierPre(zone(), baseline);
+    fop->removeCellMemory(this, baseline->allocBytes(),
+                          js::MemoryUse::BaselineScript);
+    baseline = nullptr;
+  }
+  MOZ_ASSERT(!ion || ion == ION_DISABLED_SCRIPT);
+
+  baseline = baselineScript;
+  if (hasBaselineScript()) {
+    AddCellMemory(this, baseline->allocBytes(), js::MemoryUse::BaselineScript);
+  }
+  resetWarmUpResetCounter();
+  updateJitCodeRaw(fop->runtime());
+}
+
+void JSScript::setIonScriptImpl(JSRuntime* rt, js::jit::IonScript* ionScript) {
+  setIonScriptImpl(rt->defaultFreeOp(), ionScript);
+}
+
+void JSScript::setIonScriptImpl(JSFreeOp* fop, js::jit::IonScript* ionScript) {
   MOZ_ASSERT_IF(ionScript != ION_DISABLED_SCRIPT,
                 !baselineScript()->hasPendingIonBuilder());
   if (hasIonScript()) {
     js::jit::IonScript::writeBarrierPre(zone(), ion);
-    clearIonScript(fop);
+    fop->removeCellMemory(this, ion->allocBytes(), js::MemoryUse::IonScript);
+    ion = nullptr;
   }
   ion = ionScript;
   MOZ_ASSERT_IF(hasIonScript(), hasBaselineScript());
   if (hasIonScript()) {
     AddCellMemory(this, ion->allocBytes(), js::MemoryUse::IonScript);
   }
   updateJitCodeRaw(fop->runtime());
 }
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -2597,54 +2597,109 @@ class JSScript : public js::BaseScript {
   }
   static constexpr size_t offsetOfPrivateScriptData() {
     return offsetof(JSScript, data_);
   }
   static constexpr size_t offsetOfJitScript() {
     return offsetof(JSScript, jitScript_);
   }
 
-  bool hasAnyIonScript() const { return hasIonScript(); }
-
+ private:
+  // Methods to set |ion| to an IonScript*, nullptr, or one of the special
+  // ION_{DISABLED,COMPILING,PENDING}_SCRIPT values.
+  void setIonScriptImpl(JSFreeOp* fop, js::jit::IonScript* ionScript);
+  void setIonScriptImpl(JSRuntime* rt, js::jit::IonScript* ionScript);
+
+ public:
+  // Methods for getting/setting/clearing an IonScript*.
   bool hasIonScript() const {
     bool res = ion && ion != ION_DISABLED_SCRIPT &&
                ion != ION_COMPILING_SCRIPT && ion != ION_PENDING_SCRIPT;
     MOZ_ASSERT_IF(res, baseline);
     return res;
   }
-  bool canIonCompile() const { return ion != ION_DISABLED_SCRIPT; }
-  bool isIonCompilingOffThread() const { return ion == ION_COMPILING_SCRIPT; }
-
   js::jit::IonScript* ionScript() const {
     MOZ_ASSERT(hasIonScript());
     return ion;
   }
-  js::jit::IonScript* maybeIonScript() const { return ion; }
-  js::jit::IonScript* const* addressOfIonScript() const { return &ion; }
-  void setIonScript(JSRuntime* rt, js::jit::IonScript* ionScript);
-  void setIonScript(JSFreeOp* fop, js::jit::IonScript* ionScript);
-  inline void clearIonScript(JSFreeOp* fop);
-
+  void setIonScript(JSRuntime* rt, js::jit::IonScript* ionScript) {
+    MOZ_ASSERT(!hasIonScript());
+    setIonScriptImpl(rt, ionScript);
+    MOZ_ASSERT(hasIonScript());
+  }
+  void clearIonScript(JSFreeOp* fop) {
+    MOZ_ASSERT(hasIonScript());
+    setIonScriptImpl(fop, nullptr);
+  }
+
+  // Methods for the Ion-disabled status.
+  bool canIonCompile() const { return ion != ION_DISABLED_SCRIPT; }
+  void disableIon(JSRuntime* rt) { setIonScriptImpl(rt, ION_DISABLED_SCRIPT); }
+
+  // Methods for off-thread compilation.
+  bool isIonCompilingOffThread() const { return ion == ION_COMPILING_SCRIPT; }
+  void setIsIonCompilingOffThread(JSRuntime* rt) {
+    MOZ_ASSERT(!ion);
+    setIonScriptImpl(rt, ION_COMPILING_SCRIPT);
+  }
+  void clearIsIonCompilingOffThread(JSRuntime* rt) {
+    MOZ_ASSERT(isIonCompilingOffThread());
+    setIonScriptImpl(rt, nullptr);
+  }
+
+  // Methods for lazy linking after off-thread compilation. In this case the
+  // BaselineScript has a pending builder that is linked by the lazy link stub.
+  void setHasPendingIonScript(JSRuntime* rt) {
+    MOZ_ASSERT(isIonCompilingOffThread());
+    setIonScriptImpl(rt, ION_PENDING_SCRIPT);
+  }
+  void clearHasPendingIonScript(JSRuntime* rt) {
+    if (ion == ION_PENDING_SCRIPT) {
+      setIonScriptImpl(rt, nullptr);
+    }
+  }
+
+ private:
+  // Methods to set |baseline| to a BaselineScript*, nullptr, or
+  // BASELINE_DISABLED_SCRIPT.
+  void setBaselineScriptImpl(JSRuntime* rt,
+                             js::jit::BaselineScript* baselineScript);
+  void setBaselineScriptImpl(JSFreeOp* fop,
+                             js::jit::BaselineScript* baselineScript);
+
+ public:
+  // Methods for getting/setting/clearing a BaselineScript*.
   bool hasBaselineScript() const {
     bool res = baseline && baseline != BASELINE_DISABLED_SCRIPT;
-    MOZ_ASSERT_IF(!res, !ion || ion == ION_DISABLED_SCRIPT);
+    MOZ_ASSERT_IF(!res, !hasIonScript());
     return res;
   }
-  bool canBaselineCompile() const {
-    return baseline != BASELINE_DISABLED_SCRIPT;
-  }
   js::jit::BaselineScript* baselineScript() const {
     MOZ_ASSERT(hasBaselineScript());
     return baseline;
   }
-  inline void setBaselineScript(JSRuntime* rt,
-                                js::jit::BaselineScript* baselineScript);
-  inline void setBaselineScript(JSFreeOp* fop,
-                                js::jit::BaselineScript* baselineScript);
-  inline void clearBaselineScript(JSFreeOp* fop);
+  void setBaselineScript(JSRuntime* rt,
+                         js::jit::BaselineScript* baselineScript) {
+    MOZ_ASSERT(!hasBaselineScript());
+    setBaselineScriptImpl(rt, baselineScript);
+    MOZ_ASSERT(hasBaselineScript());
+  }
+  void clearBaselineScript(JSFreeOp* fop) {
+    MOZ_ASSERT(hasBaselineScript());
+    setBaselineScriptImpl(fop, nullptr);
+  }
+
+  // Methods for the Baseline-disabled status.
+  bool canBaselineCompile() const {
+    return baseline != BASELINE_DISABLED_SCRIPT;
+  }
+  void disableBaselineCompile(JSRuntime* rt) {
+    MOZ_ASSERT(!hasBaselineScript());
+    setBaselineScriptImpl(rt, BASELINE_DISABLED_SCRIPT);
+  }
 
   void updateJitCodeRaw(JSRuntime* rt);
 
   static size_t offsetOfBaselineScript() {
     return offsetof(JSScript, baseline);
   }
   static size_t offsetOfIonScript() { return offsetof(JSScript, ion); }