Bug 1140643 - JitcodeGlobalMap marking must participate in iterative weak reference marking. (r=terrence)

js/src/gc/GCRuntime.h

@@ -889,7 +889,6 @@ class GCRuntime
     void markGrayReferencesInCurrentGroup(gcstats::Phase phase);
     void markAllWeakReferences(gcstats::Phase phase);
     void markAllGrayReferences(gcstats::Phase phase);
-    void markJitcodeGlobalTable();
 
     void beginSweepPhase(bool lastGC);
     void findZoneGroups();

js/src/gc/Statistics.cpp

@@ -337,7 +337,6 @@ static const PhaseInfo phases[] = {
             { PHASE_SWEEP_MARK_INCOMING_GRAY, "Mark Incoming Gray Pointers", PHASE_SWEEP_MARK },
             { PHASE_SWEEP_MARK_GRAY, "Mark Gray", PHASE_SWEEP_MARK },
             { PHASE_SWEEP_MARK_GRAY_WEAK, "Mark Gray and Weak", PHASE_SWEEP_MARK },
-            { PHASE_SWEEP_MARK_JITCODE_GLOBAL_TABLE, "Mark JitcodeGlobalTable", PHASE_SWEEP_MARK },
         { PHASE_FINALIZE_START, "Finalize Start Callback", PHASE_SWEEP },
         { PHASE_SWEEP_ATOMS, "Sweep Atoms", PHASE_SWEEP },
         { PHASE_SWEEP_SYMBOL_REGISTRY, "Sweep Symbol Registry", PHASE_SWEEP },

js/src/gc/Statistics.h

@@ -43,7 +43,6 @@ enum Phase {
     PHASE_SWEEP_MARK_INCOMING_GRAY,
     PHASE_SWEEP_MARK_GRAY,
     PHASE_SWEEP_MARK_GRAY_WEAK,
-    PHASE_SWEEP_MARK_JITCODE_GLOBAL_TABLE,
     PHASE_FINALIZE_START,
     PHASE_SWEEP_ATOMS,
     PHASE_SWEEP_SYMBOL_REGISTRY,

js/src/jit-test/tests/profiler/bug1140643.js

@@ -0,0 +1,14 @@
+// |jit-test| allow-oom
+enableSPSProfiling();
+loadFile('\
+for (var i = 0; i < 2; i++) {\
+    obj = { m: function () {} };\
+    obj.watch("m", function () { float32 = 0 + obj.foo; });\
+    obj.m = 0;\
+}\
+');
+gcparam("maxBytes", gcparam("gcBytes") + (1)*1024);
+newGlobal("same-compartment");
+function loadFile(lfVarx) {
+  evaluate(lfVarx, { noScriptRval : true, compileAndGo : true }); 
+}

js/src/jit/Ion.cpp

@@ -500,14 +500,15 @@ JitRuntime::Mark(JSTracer *trc)
     }
 }
 
-/* static */ void
-JitRuntime::MarkJitcodeGlobalTable(JSTracer *trc)
+/* static */ bool
+JitRuntime::MarkJitcodeGlobalTableIteratively(JSTracer *trc)
 {
     if (trc->runtime()->hasJitRuntime() &&
         trc->runtime()->jitRuntime()->hasJitcodeGlobalTable())
     {
-        trc->runtime()->jitRuntime()->getJitcodeGlobalTable()->mark(trc);
+        return trc->runtime()->jitRuntime()->getJitcodeGlobalTable()->markIteratively(trc);
     }
+    return false;
 }
 
 /* static */ void

js/src/jit/JitCompartment.h

@@ -257,7 +257,7 @@ class JitRuntime
     void freeOsrTempData();
 
     static void Mark(JSTracer *trc);
-    static void MarkJitcodeGlobalTable(JSTracer *trc);
+    static bool MarkJitcodeGlobalTableIteratively(JSTracer *trc);
     static void SweepJitcodeGlobalTable(JSRuntime *rt);
 
     ExecutableAllocator &execAlloc() {

js/src/jit/JitcodeMap.cpp

@@ -718,8 +718,8 @@ JitcodeGlobalTable::verifySkiplist()
 }
 #endif // DEBUG
 
-void
-JitcodeGlobalTable::mark(JSTracer *trc)
+bool
+JitcodeGlobalTable::markIteratively(JSTracer *trc)
 {
     // JitcodeGlobalTable must keep entries that are in the sampler buffer
     // alive. This conditionality is akin to holding the entries weakly.
@@ -731,19 +731,17 @@ JitcodeGlobalTable::mark(JSTracer *trc)
     // itself.
     //
     // Instead, JitcodeGlobalTable is marked at the beginning of the sweep
-    // phase. The key assumption is the following. At the beginning of the
-    // sweep phase, any JS frames that the sampler may put in its buffer that
-    // are not already there at the beginning of the mark phase must have
-    // already been marked, as either 1) the frame was on-stack at the
-    // beginning of the sweep phase, or 2) the frame was pushed between
-    // incremental sweep slices. Frames of case 1) are already marked. Frames
-    // of case 2) must have been reachable to have been newly pushed, and thus
-    // are already marked.
+    // phase, along with weak references. The key assumption is the
+    // following. At the beginning of the sweep phase, any JS frames that the
+    // sampler may put in its buffer that are not already there at the
+    // beginning of the mark phase must have already been marked, as either 1)
+    // the frame was on-stack at the beginning of the sweep phase, or 2) the
+    // frame was pushed between incremental sweep slices. Frames of case 1)
+    // are already marked. Frames of case 2) must have been reachable to have
+    // been newly pushed, and thus are already marked.
     //
     // The approach above obviates the need for read barriers. The assumption
     // above is checked in JitcodeGlobalTable::lookupForSampler.
-    MOZ_ASSERT(trc->runtime()->gc.stats.currentPhase() ==
-               gcstats::PHASE_SWEEP_MARK_JITCODE_GLOBAL_TABLE);
 
     AutoSuppressProfilerSampling suppressSampling(trc->runtime());
     uint32_t gen = trc->runtime()->profilerSampleBufferGen();
@@ -752,7 +750,7 @@ JitcodeGlobalTable::mark(JSTracer *trc)
     if (!trc->runtime()->spsProfiler.enabled())
         gen = UINT32_MAX;
 
-    // Find start entry.
+    bool markedAny = false;
     for (Range r(*this); !r.empty(); r.popFront()) {
         JitcodeGlobalEntry *entry = r.front();
 
@@ -774,8 +772,10 @@ JitcodeGlobalTable::mark(JSTracer *trc)
         if (!entry->zone()->isCollecting() || entry->zone()->isGCFinished())
             continue;
 
-        entry->mark(trc);
+        markedAny |= entry->markIfUnmarked(trc);
     }
+
+    return markedAny;
 }
 
 void
@@ -795,15 +795,21 @@ JitcodeGlobalTable::sweep(JSRuntime *rt)
     }
 }
 
-void
-JitcodeGlobalEntry::BaseEntry::markJitcode(JSTracer *trc)
+bool
+JitcodeGlobalEntry::BaseEntry::markJitcodeIfUnmarked(JSTracer *trc)
 {
-    MarkJitCodeUnbarriered(trc, &jitcode_, "jitcodglobaltable-baseentry-jitcode");
+    if (!isJitcodeMarkedFromAnyThread()) {
+        MarkJitCodeUnbarriered(trc, &jitcode_, "jitcodglobaltable-baseentry-jitcode");
+        return true;
+    }
+    return false;
 }
 
 bool
 JitcodeGlobalEntry::BaseEntry::isJitcodeMarkedFromAnyThread()
 {
+    if (jitcode_->asTenured().arenaHeader()->allocatedDuringIncremental)
+        return false;
     return IsJitCodeMarkedFromAnyThread(&jitcode_);
 }
 
@@ -813,10 +819,14 @@ JitcodeGlobalEntry::BaseEntry::isJitcodeAboutToBeFinalized()
     return IsJitCodeAboutToBeFinalized(&jitcode_);
 }
 
-void
-JitcodeGlobalEntry::BaselineEntry::mark(JSTracer *trc)
+bool
+JitcodeGlobalEntry::BaselineEntry::markIfUnmarked(JSTracer *trc)
 {
-    MarkScriptUnbarriered(trc, &script_, "jitcodeglobaltable-baselineentry-script");
+    if (!isMarkedFromAnyThread()) {
+        MarkScriptUnbarriered(trc, &script_, "jitcodeglobaltable-baselineentry-script");
+        return true;
+    }
+    return false;
 }
 
 void
@@ -831,29 +841,41 @@ JitcodeGlobalEntry::BaselineEntry::isMarkedFromAnyThread()
     return IsScriptMarkedFromAnyThread(&script_);
 }
 
-void
-JitcodeGlobalEntry::IonEntry::mark(JSTracer *trc)
+bool
+JitcodeGlobalEntry::IonEntry::markIfUnmarked(JSTracer *trc)
 {
+    bool markedAny = false;
+
     for (unsigned i = 0; i < numScripts(); i++) {
-        MarkScriptUnbarriered(trc, &sizedScriptList()->pairs[i].script,
-                              "jitcodeglobaltable-ionentry-script");
+        if (!IsScriptMarkedFromAnyThread(&sizedScriptList()->pairs[i].script)) {
+            MarkScriptUnbarriered(trc, &sizedScriptList()->pairs[i].script,
+                                  "jitcodeglobaltable-ionentry-script");
+            markedAny = true;
+        }
     }
 
     if (!optsAllTypes_)
-        return;
+        return markedAny;
 
     for (IonTrackedTypeWithAddendum *iter = optsAllTypes_->begin();
          iter != optsAllTypes_->end(); iter++)
     {
-        TypeSet::MarkTypeUnbarriered(trc, &(iter->type), "jitcodeglobaltable-ionentry-type");
-        if (iter->hasAllocationSite()) {
+        if (!TypeSet::IsTypeMarkedFromAnyThread(&iter->type)) {
+            TypeSet::MarkTypeUnbarriered(trc, &iter->type, "jitcodeglobaltable-ionentry-type");
+            markedAny = true;
+        }
+        if (iter->hasAllocationSite() && !IsScriptMarkedFromAnyThread(&iter->script)) {
             MarkScriptUnbarriered(trc, &iter->script,
                                   "jitcodeglobaltable-ionentry-type-addendum-script");
-        } else if (iter->hasConstructor()) {
+            markedAny = true;
+        } else if (iter->hasConstructor() && !IsObjectMarkedFromAnyThread(&iter->constructor)) {
             MarkObjectUnbarriered(trc, &iter->constructor,
                                   "jitcodeglobaltable-ionentry-type-addendum-constructor");
+            markedAny = true;
         }
     }
+
+    return markedAny;
 }
 
 void

js/src/jit/JitcodeMap.h

@@ -207,7 +207,7 @@ class JitcodeGlobalEntry
             return startsBelowPointer(ptr) && endsAbovePointer(ptr);
         }
 
-        void markJitcode(JSTracer *trc);
+        bool markJitcodeIfUnmarked(JSTracer *trc);
         bool isJitcodeMarkedFromAnyThread();
         bool isJitcodeAboutToBeFinalized();
     };
@@ -358,7 +358,7 @@ class JitcodeGlobalEntry
 
         mozilla::Maybe<uint8_t> trackedOptimizationIndexAtAddr(void *ptr);
 
-        void mark(JSTracer *trc);
+        bool markIfUnmarked(JSTracer *trc);
         void sweep();
         bool isMarkedFromAnyThread();
     };
@@ -414,7 +414,7 @@ class JitcodeGlobalEntry
         void youngestFrameLocationAtAddr(JSRuntime *rt, void *ptr,
                                          JSScript **script, jsbytecode **pc) const;
 
-        void mark(JSTracer *trc);
+        bool markIfUnmarked(JSTracer *trc);
         void sweep();
         bool isMarkedFromAnyThread();
     };
@@ -815,14 +815,14 @@ class JitcodeGlobalEntry
         return baseEntry().jitcode()->zone();
     }
 
-    void mark(JSTracer *trc) {
-        baseEntry().markJitcode(trc);
+    bool markIfUnmarked(JSTracer *trc) {
+        bool markedAny = baseEntry().markJitcodeIfUnmarked(trc);
         switch (kind()) {
           case Ion:
-            ionEntry().mark(trc);
+            markedAny |= ionEntry().markIfUnmarked(trc);
             break;
           case Baseline:
-            baselineEntry().mark(trc);
+            markedAny |= baselineEntry().markIfUnmarked(trc);
             break;
           case IonCache:
           case Dummy:
@@ -830,6 +830,7 @@ class JitcodeGlobalEntry
           default:
             MOZ_CRASH("Invalid JitcodeGlobalEntry kind.");
         }
+        return markedAny;
     }
 
     void sweep() {
@@ -952,7 +953,7 @@ class JitcodeGlobalTable
     void removeEntry(JitcodeGlobalEntry &entry, JitcodeGlobalEntry **prevTower, JSRuntime *rt);
     void releaseEntry(JitcodeGlobalEntry &entry, JitcodeGlobalEntry **prevTower, JSRuntime *rt);
 
-    void mark(JSTracer *trc);
+    bool markIteratively(JSTracer *trc);
     void sweep(JSRuntime *rt);
 
   private:

js/src/jsgc.cpp

@@ -4102,6 +4102,7 @@ GCRuntime::markWeakReferences(gcstats::Phase phase)
             markedAny |= WeakMapBase::markCompartmentIteratively(c, &marker);
         }
         markedAny |= Debugger::markAllIteratively(&marker);
+        markedAny |= jit::JitRuntime::MarkJitcodeGlobalTableIteratively(&marker);
 
         if (!markedAny)
             break;
@@ -4153,15 +4154,6 @@ GCRuntime::markAllGrayReferences(gcstats::Phase phase)
     markGrayReferences<GCZonesIter, GCCompartmentsIter>(phase);
 }
 
-void
-GCRuntime::markJitcodeGlobalTable()
-{
-    gcstats::AutoPhase ap(stats, gcstats::PHASE_SWEEP_MARK_JITCODE_GLOBAL_TABLE);
-    jit::JitRuntime::MarkJitcodeGlobalTable(&marker);
-    SliceBudget budget;
-    marker.drainMarkStack(budget);
-}
-
 #ifdef DEBUG
 
 class js::gc::MarkingValidator
@@ -4279,7 +4271,6 @@ js::gc::MarkingValidator::nonIncrementalMark()
     {
         gcstats::AutoPhase ap1(gc->stats, gcstats::PHASE_SWEEP);
         gcstats::AutoPhase ap2(gc->stats, gcstats::PHASE_SWEEP_MARK);
-        gc->markJitcodeGlobalTable();
 
         gc->markAllWeakReferences(gcstats::PHASE_SWEEP_MARK_WEAK);
 
@@ -4823,8 +4814,6 @@ GCRuntime::endMarkingZoneGroup()
 {
     gcstats::AutoPhase ap(stats, gcstats::PHASE_SWEEP_MARK);
 
-    markJitcodeGlobalTable();
-
     /*
      * Mark any incoming black pointers from previously swept compartments
      * whose referents are not marked. This can occur when gray cells become