Bug 1524499 part 4 - Some minor changes to address review feedback. r=tcampbell

Differential Revision: https://phabricator.services.mozilla.com/D18569

--HG--
extra : moz-landing-system : lando

js/src/gc/Zone.cpp

@@ -220,31 +220,30 @@ void Zone::discardJitCode(FreeOp* fop,
     jit::MarkActiveTypeScripts(this);
   }
 
-  /* Only mark OSI points if code is being discarded. */
+  // Invalidate all Ion code in this zone.
   jit::InvalidateAll(fop, this);
 
   for (auto script = cellIter<JSScript>(); !script.done(); script.next()) {
     jit::FinishInvalidation(fop, script);
 
-    /*
-     * Discard baseline script if it's not marked as active. Note that
-     * this also resets the active flag.
-     */
-    if (discardBaselineCode) {
-      jit::FinishDiscardBaselineScript(fop, script);
+    // Discard baseline script if it's not marked as active.
+    if (discardBaselineCode && script->hasBaselineScript()) {
+      if (script->types()->active()) {
+        // ICs will be purged so the script will need to warm back up before it
+        // can be inlined during Ion compilation.
+        script->baselineScript()->clearIonCompiledOrInlined();
+      } else {
+        jit::FinishDiscardBaselineScript(fop, script);
+      }
     }
 
-    /*
-     * Warm-up counter for scripts are reset on GC. After discarding code we
-     * need to let it warm back up to get information such as which
-     * opcodes are setting array holes or accessing getter properties.
-     */
+    // Warm-up counter for scripts are reset on GC. After discarding code we
+    // need to let it warm back up to get information such as which
+    // opcodes are setting array holes or accessing getter properties.
     script->resetWarmUpCounter();
 
-    /*
-     * Make it impossible to use the control flow graphs cached on the
-     * BaselineScript. They get deleted.
-     */
+    // Clear the BaselineScript's control flow graph. The LifoAlloc is purged
+    // below.
     if (script->hasBaselineScript()) {
       script->baselineScript()->setControlFlowGraph(nullptr);
     }

js/src/jit/BaselineJIT.cpp

@@ -1138,16 +1138,8 @@ void jit::JitSpewBaselineICStats(JSScript* script, const char* dumpReason) {
 #endif
 
 void jit::FinishDiscardBaselineScript(FreeOp* fop, JSScript* script) {
-  if (!script->hasBaselineScript()) {
-    return;
-  }
-
-  if (script->types()->active()) {
-    // The baseline caches have been wiped out, so the script will need to
-    // warm back up before it can be inlined during Ion compilation.
-    script->baselineScript()->clearIonCompiledOrInlined();
-    return;
-  }
+  MOZ_ASSERT(script->hasBaselineScript());
+  MOZ_ASSERT(!script->types()->active());
 
   BaselineScript* baseline = script->baselineScript();
   script->setBaselineScript(fop->runtime(), nullptr);

js/src/vm/Debugger.cpp

@@ -2791,7 +2791,9 @@ static bool UpdateExecutionObservabilityOfScriptsInZone(
   // discard the BaselineScript on scripts that have no IonScript.
   for (size_t i = 0; i < scripts.length(); i++) {
     MOZ_ASSERT_IF(scripts[i]->isDebuggee(), observing);
-    FinishDiscardBaselineScript(fop, scripts[i]);
+    if (!scripts[i]->types()->active()) {
+      FinishDiscardBaselineScript(fop, scripts[i]);
+    }
     scripts[i]->types()->resetActive();
   }
 

js/src/vm/TypeInference.cpp

@@ -3593,12 +3593,7 @@ static size_t NumTypeSets(JSScript* script) {
 
 TypeScript::TypeScript(JSScript* script, ICScriptPtr&& icScript,
                        uint32_t numTypeSets)
-    : icScript_(std::move(icScript)),
-      numTypeSets_(numTypeSets),
-      bytecodeTypeMapHint_(0),
-      active_(false),
-      typesGeneration_(false),
-      hasFreezeConstraints_(false) {
+    : icScript_(std::move(icScript)), numTypeSets_(numTypeSets) {
   setTypesGeneration(script->zone()->types.generation);
 
   StackTypeSet* array = typeArrayDontCheckGeneration();
@@ -4741,7 +4736,7 @@ void ObjectGroup::sweep(const AutoSweepObjectGroup& sweep) {
   if (types.hadOOMSweepingTypes()) {
     // It's possible we OOM'd while copying freeze constraints, so they
     // need to be regenerated.
-    hasFreezeConstraints_ = false;
+    flags_.hasFreezeConstraints = false;
   }
 }
 

js/src/vm/TypeInference.h

@@ -221,18 +221,21 @@ class TypeScript {
 
   // This field is used to avoid binary searches for the sought entry when
   // bytecode map queries are in linear order.
-  uint32_t bytecodeTypeMapHint_;
+  uint32_t bytecodeTypeMapHint_ = 0;
 
-  // Flag set when discarding JIT code to indicate this script is on the stack
-  // and type information and JIT code should not be discarded.
-  bool active_ : 1;
+  struct Flags {
+    // Flag set when discarding JIT code to indicate this script is on the stack
+    // and type information and JIT code should not be discarded.
+    bool active : 1;
 
-  // Generation for type sweeping. If out of sync with the TypeZone's
-  // generation, this TypeScript needs to be swept.
-  bool typesGeneration_ : 1;
+    // Generation for type sweeping. If out of sync with the TypeZone's
+    // generation, this TypeScript needs to be swept.
+    bool typesGeneration : 1;
 
-  // Whether freeze constraints for stack type sets have been generated.
-  bool hasFreezeConstraints_ : 1;
+    // Whether freeze constraints for stack type sets have been generated.
+    bool hasFreezeConstraints : 1;
+  };
+  Flags flags_ = {};  // Zero-initialize flags.
 
   // Variable-size array. This is followed by the bytecode type map.
   StackTypeSet typeArray_[1];
@@ -245,10 +248,10 @@ class TypeScript {
     return const_cast<StackTypeSet*>(typeArray_);
   }
 
-  uint32_t typesGeneration() const { return uint32_t(typesGeneration_); }
+  uint32_t typesGeneration() const { return uint32_t(flags_.typesGeneration); }
   void setTypesGeneration(uint32_t generation) {
     MOZ_ASSERT(generation <= 1);
-    typesGeneration_ = generation;
+    flags_.typesGeneration = generation;
   }
 
  public:
@@ -256,11 +259,11 @@ class TypeScript {
 
   bool hasFreezeConstraints(const js::AutoSweepTypeScript& sweep) const {
     MOZ_ASSERT(sweep.typeScript() == this);
-    return hasFreezeConstraints_;
+    return flags_.hasFreezeConstraints;
   }
   void setHasFreezeConstraints(const js::AutoSweepTypeScript& sweep) {
     MOZ_ASSERT(sweep.typeScript() == this);
-    hasFreezeConstraints_ = true;
+    flags_.hasFreezeConstraints = true;
   }
 
   inline bool typesNeedsSweep(Zone* zone) const;
@@ -284,9 +287,9 @@ class TypeScript {
 
   uint32_t* bytecodeTypeMapHint() { return &bytecodeTypeMapHint_; }
 
-  bool active() const { return active_; }
-  void setActive() { active_ = true; }
-  void resetActive() { active_ = false; }
+  bool active() const { return flags_.active; }
+  void setActive() { flags_.active = true; }
+  void resetActive() { flags_.active = false; }
 
   jit::ICScript* icScript() const {
     MOZ_ASSERT(icScript_);