Bug 1505689 part 8 - Make JitScript::clear{Baseline,Ion}Script return the cleared script. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 15 Aug 2019 16:14:16 +0000
changeset 488280 11d01998d8f72baa0994f4a967dbe259fe7d55d1
parent 488279 f7a4be5909d3ee0fcf36d64979f50036e2938a07
child 488281 d024bd1f20c28bbf183decce5582427eff6f8785
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 8 - Make JitScript::clear{Baseline,Ion}Script return the cleared script. r=tcampbell Differential Revision: https://phabricator.services.mozilla.com/D42108
js/src/jit/BaselineDebugModeOSR.cpp
js/src/jit/BaselineJIT.cpp
js/src/jit/Ion.cpp
js/src/jit/JitScript.cpp
js/src/jit/JitScript.h
--- a/js/src/jit/BaselineDebugModeOSR.cpp
+++ b/js/src/jit/BaselineDebugModeOSR.cpp
@@ -395,30 +395,29 @@ static void SkipInterpreterFrameEntries(
     }
   }
 
   *start = entryIndex;
 }
 
 static bool RecompileBaselineScriptForDebugMode(
     JSContext* cx, JSScript* script, DebugAPI::IsObserving observing) {
-  BaselineScript* oldBaselineScript = script->baselineScript();
-
   // If a script is on the stack multiple times, it may have already
   // been recompiled.
-  if (oldBaselineScript->hasDebugInstrumentation() == observing) {
+  if (script->baselineScript()->hasDebugInstrumentation() == observing) {
     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->jitScript()->clearBaselineScript(cx->defaultFreeOp(), script);
+  BaselineScript* oldBaselineScript =
+      script->jitScript()->clearBaselineScript(cx->defaultFreeOp(), script);
 
   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
@@ -931,18 +931,18 @@ 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->jitScript()->clearBaselineScript(fop, script);
+  BaselineScript* baseline =
+      script->jitScript()->clearBaselineScript(fop, script);
   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/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*-
  * vim: set ts=8 sts=2 et sw=2 tw=80:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "jit/Ion.h"
 
+#include "mozilla/DebugOnly.h"
 #include "mozilla/IntegerPrintfMacros.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/ThreadLocal.h"
 #include "mozilla/Unused.h"
 
 #include "gc/FreeOp.h"
 #include "gc/Marking.h"
 #include "jit/AliasAnalysis.h"
@@ -63,16 +64,18 @@
 #include "vm/JSScript-inl.h"
 #include "vm/Realm-inl.h"
 #include "vm/Stack-inl.h"
 
 #if defined(ANDROID)
 #  include <sys/system_properties.h>
 #endif
 
+using mozilla::DebugOnly;
+
 using namespace js;
 using namespace js::jit;
 
 // Assert that JitCode is gc::Cell aligned.
 JS_STATIC_ASSERT(sizeof(JitCode) % gc::CellAlignBytes == 0);
 
 static MOZ_THREAD_LOCAL(JitContext*) TlsJitContext;
 
@@ -2661,18 +2664,23 @@ void jit::InvalidateAll(JSFreeOp* fop, Z
     if (iter->compartment()->zone() == zone) {
       JitSpew(JitSpew_IonInvalidate, "Invalidating all frames for GC");
       InvalidateActivation(fop, iter, true);
     }
   }
 }
 
 static void ClearIonScriptAfterInvalidation(JSContext* cx, JSScript* script,
+                                            IonScript* ionScript,
                                             bool resetUses) {
-  script->jitScript()->clearIonScript(cx->defaultFreeOp(), script);
+  // Null out the JitScript's IonScript pointer. The caller is responsible for
+  // destroying the IonScript using the invalidation count mechanism.
+  DebugOnly<IonScript*> clearedIonScript =
+      script->jitScript()->clearIonScript(cx->defaultFreeOp(), script);
+  MOZ_ASSERT(clearedIonScript == ionScript);
 
   // 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();
   }
 }
@@ -2722,34 +2730,35 @@ void jit::Invalidate(TypeZone& types, JS
   for (const RecompileInfo& info : invalid) {
     IonScript* ionScript = info.maybeIonScriptToInvalidate(types);
     if (!ionScript) {
       continue;
     }
 
     if (ionScript->invalidationCount() == 1) {
       // decrementInvalidationCount will destroy the IonScript so null out
-      // script->ion now. We don't want to do this unconditionally because
-      // maybeIonScriptToInvalidate depends on script->ion (we would leak
-      // the IonScript if |invalid| contains duplicates).
-      ClearIonScriptAfterInvalidation(cx, info.script(), resetUses);
+      // jitScript->ionScript_ now. We don't want to do this unconditionally
+      // because maybeIonScriptToInvalidate depends on script->ionScript() (we
+      // would leak the IonScript if |invalid| contains duplicates).
+      ClearIonScriptAfterInvalidation(cx, info.script(), ionScript, resetUses);
     }
 
     ionScript->decrementInvalidationCount(fop);
     numInvalidations--;
   }
 
   // Make sure we didn't leak references by invalidating the same IonScript
   // multiple times in the above loop.
   MOZ_ASSERT(!numInvalidations);
 
-  // Finally, null out script->ion for IonScripts that are still on the stack.
+  // Finally, null out jitScript->ionScript_ for IonScripts that are still on
+  // the stack.
   for (const RecompileInfo& info : invalid) {
-    if (info.maybeIonScriptToInvalidate(types)) {
-      ClearIonScriptAfterInvalidation(cx, info.script(), resetUses);
+    if (IonScript* ionScript = info.maybeIonScriptToInvalidate(types)) {
+      ClearIonScriptAfterInvalidation(cx, info.script(), ionScript, resetUses);
     }
   }
 }
 
 void jit::Invalidate(JSContext* cx, const RecompileInfoVector& invalid,
                      bool resetUses, bool cancelOffThread) {
   jit::Invalidate(cx->zone()->types, cx->runtime()->defaultFreeOp(), invalid,
                   resetUses, cancelOffThread);
@@ -2806,19 +2815,18 @@ void jit::Invalidate(JSContext* cx, JSSc
   Invalidate(cx, scripts, resetUses, cancelOffThread);
 }
 
 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->jitScript()->clearIonScript(fop, script);
+  // In all cases, null out jitScript->ionScript_ to avoid re-entry.
+  IonScript* ion = script->jitScript()->clearIonScript(fop, script);
 
   // 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);
   }
 }
 
--- a/js/src/jit/JitScript.cpp
+++ b/js/src/jit/JitScript.cpp
@@ -194,24 +194,22 @@ void JSScript::releaseJitScript(JSFreeOp
   jitScript_ = nullptr;
   updateJitCodeRaw(fop->runtime());
 }
 
 void JSScript::releaseJitScriptOnFinalize(JSFreeOp* fop) {
   MOZ_ASSERT(hasJitScript());
 
   if (hasIonScript()) {
-    IonScript* ion = ionScript();
-    jitScript()->clearIonScript(fop, this);
+    IonScript* ion = jitScript()->clearIonScript(fop, this);
     jit::IonScript::Destroy(fop, ion);
   }
 
   if (hasBaselineScript()) {
-    BaselineScript* baseline = baselineScript();
-    jitScript()->clearBaselineScript(fop, this);
+    BaselineScript* baseline = jitScript()->clearBaselineScript(fop, this);
     jit::BaselineScript::Destroy(fop, baseline);
   }
 
   releaseJitScript(fop);
 }
 
 void JitScript::CachedIonData::trace(JSTracer* trc) {
   TraceNullableEdge(trc, &templateEnv, "jitscript-iondata-template-env");
--- a/js/src/jit/JitScript.h
+++ b/js/src/jit/JitScript.h
@@ -504,19 +504,21 @@ class alignas(uintptr_t) JitScript final
     MOZ_ASSERT(hasBaselineScript());
     return baselineScript_;
   }
   void setBaselineScript(JSScript* script, BaselineScript* baselineScript) {
     MOZ_ASSERT(!hasBaselineScript());
     setBaselineScriptImpl(script, baselineScript);
     MOZ_ASSERT(hasBaselineScript());
   }
-  void clearBaselineScript(JSFreeOp* fop, JSScript* script) {
-    MOZ_ASSERT(hasBaselineScript());
+  MOZ_MUST_USE BaselineScript* clearBaselineScript(JSFreeOp* fop,
+                                                   JSScript* script) {
+    BaselineScript* baseline = baselineScript();
     setBaselineScriptImpl(fop, script, nullptr);
+    return baseline;
   }
 
  private:
   // Methods to set ionScript_ to an IonScript*, nullptr, or one of the special
   // ION_{DISABLED,COMPILING}_SCRIPT values.
   void setIonScriptImpl(JSFreeOp* fop, JSScript* script, IonScript* ionScript);
   void setIonScriptImpl(JSScript* script, IonScript* ionScript);
 
@@ -532,19 +534,20 @@ class alignas(uintptr_t) JitScript final
     MOZ_ASSERT(hasIonScript());
     return ionScript_;
   }
   void setIonScript(JSScript* script, IonScript* ionScript) {
     MOZ_ASSERT(!hasIonScript());
     setIonScriptImpl(script, ionScript);
     MOZ_ASSERT(hasIonScript());
   }
-  void clearIonScript(JSFreeOp* fop, JSScript* script) {
-    MOZ_ASSERT(hasIonScript());
+  MOZ_MUST_USE IonScript* clearIonScript(JSFreeOp* fop, JSScript* script) {
+    IonScript* ion = ionScript();
     setIonScriptImpl(fop, script, nullptr);
+    return ion;
   }
 
   // Methods for off-thread compilation.
   bool isIonCompilingOffThread() const {
     return ionScript_ == ION_COMPILING_SCRIPT;
   }
   void setIsIonCompilingOffThread(JSScript* script) {
     MOZ_ASSERT(ionScript_ == nullptr);