Bug 1290108 - Make SharedScriptData refcounted so we can free them without doing a full GC r=terrence

js/src/jsgc.cpp

@@ -3893,9 +3893,6 @@ GCRuntime::beginMarkPhase(JS::gcreason::Reason reason, AutoLockForExclusiveAcces
             /* Unmark all weak maps in the zones being collected. */
             WeakMapBase::unmarkZone(zone);
         }
-
-        if (isFull)
-            UnmarkScriptData(rt, lock);
     }
 
     markRuntime(gcmarker, MarkRuntime, lock);
@@ -5421,8 +5418,7 @@ GCRuntime::endSweepPhase(bool destroyingRuntime, AutoLockForExclusiveAccess& loc
          * script and calls rt->destroyScriptHook, the hook can still access the
          * script's filename. See bug 323267.
          */
-        if (isFull)
-            SweepScriptData(rt, lock);
+        SweepScriptData(rt, lock);
 
         /* Clear out any small pools that we're hanging on to. */
         if (jit::JitRuntime* jitRuntime = rt->jitRuntime()) {

js/src/jsscript.cpp

@@ -2417,10 +2417,10 @@ js::SharedScriptData::new_(ExclusiveContext* cx, uint32_t codeLength,
         return nullptr;
     }
 
+    entry->refCount_ = 0;
     entry->dataLength_ = dataLength;
     entry->natoms_ = natoms;
     entry->codeLength_ = codeLength;
-    entry->marked_ = false;
 
     /*
      * Call constructors to initialize the storage that will be accessed as a
@@ -2450,7 +2450,8 @@ JSScript::createScriptData(ExclusiveContext* cx, uint32_t codeLength, uint32_t s
 void
 JSScript::freeScriptData()
 {
-    js_free(scriptData_);
+    MOZ_ASSERT(scriptData_->refCount() == 1);
+    scriptData_->decRefCount();
     scriptData_ = nullptr;
 }
 
@@ -2459,6 +2460,7 @@ JSScript::setScriptData(js::SharedScriptData* data)
 {
     MOZ_ASSERT(!scriptData_);
     scriptData_ = data;
+    scriptData_->incRefCount();
 }
 
 /*
@@ -2472,6 +2474,7 @@ JSScript::shareScriptData(ExclusiveContext* cx)
 {
     SharedScriptData* ssd = scriptData();
     MOZ_ASSERT(ssd);
+    MOZ_ASSERT(ssd->refCount() == 1);
 
     AutoLockForExclusiveAccess lock(cx);
 
@@ -2488,45 +2491,27 @@ JSScript::shareScriptData(ExclusiveContext* cx)
             ReportOutOfMemory(cx);
             return false;
         }
+
+        // Being in the table counts as a reference on the script data.
+        scriptData()->incRefCount();
     }
 
-    /*
-     * During the IGC we need to ensure that bytecode is marked whenever it is
-     * accessed even if the bytecode was already in the table: at this point
-     * old scripts or exceptions pointing to the bytecode may no longer be
-     * reachable. This is effectively a read barrier.
-     */
-    if (cx->isJSContext()) {
-        JSContext* ncx = cx->asJSContext();
-        if (JS::IsIncrementalGCInProgress(ncx) && ncx->gc.isFullGc())
-            scriptData()->setMarked(true);
-    }
-
+    MOZ_ASSERT(scriptData()->refCount() >= 2);
     return true;
 }
 
-void
-js::UnmarkScriptData(JSRuntime* rt, AutoLockForExclusiveAccess& lock)
-{
-    MOZ_ASSERT(rt->gc.isFullGc());
-    ScriptDataTable& table = rt->scriptDataTable(lock);
-    for (ScriptDataTable::Enum e(table); !e.empty(); e.popFront())
-        e.front()->setMarked(false);
-}
-
 void
 js::SweepScriptData(JSRuntime* rt, AutoLockForExclusiveAccess& lock)
 {
-    MOZ_ASSERT(rt->gc.isFullGc());
+    // Entries are removed from the table when their reference count is one,
+    // i.e. when the only reference to them is from the table entry.
+
     ScriptDataTable& table = rt->scriptDataTable(lock);
 
-    if (rt->keepAtoms())
-        return;
-
     for (ScriptDataTable::Enum e(table); !e.empty(); e.popFront()) {
-        SharedScriptData* entry = e.front();
-        if (!entry->marked()) {
-            js_free(entry);
+        SharedScriptData* scriptData = e.front();
+        if (scriptData->refCount() == 1) {
+            scriptData->decRefCount();
             e.removeFront();
         }
     }
@@ -2539,8 +2524,14 @@ js::FreeScriptData(JSRuntime* rt, AutoLockForExclusiveAccess& lock)
     if (!table.initialized())
         return;
 
-    for (ScriptDataTable::Enum e(table); !e.empty(); e.popFront())
+    for (ScriptDataTable::Enum e(table); !e.empty(); e.popFront()) {
+#ifdef DEBUG
+        SharedScriptData* scriptData = e.front();
+        fprintf(stderr, "ERROR: GC found live SharedScriptData %p with ref count %d at shutdown\n",
+                scriptData, scriptData->refCount());
+#endif
         js_free(e.front());
+    }
 
     table.clear();
 }
@@ -3140,6 +3131,9 @@ JSScript::finalize(FreeOp* fop)
         fop->free_(data);
     }
 
+    if (scriptData_)
+        scriptData_->decRefCount();
+
     fop->runtime()->contextFromMainThread()->caches.lazyScriptCache.remove(this);
 
     // In most cases, our LazyScript's script pointer will reference this
@@ -3902,16 +3896,9 @@ JSScript::hasBreakpointsAt(jsbytecode* pc)
 void
 SharedScriptData::traceChildren(JSTracer* trc)
 {
+    MOZ_ASSERT(refCount() != 0);
     for (uint32_t i = 0; i < natoms(); ++i)
         TraceNullableEdge(trc, &atoms()[i], "atom");
-
-    /*
-     * As an invariant, a ScriptBytecodeEntry should not be 'marked' outside of
-     * a GC. Since SweepScriptBytecodes is only called during a full gc,
-     * to preserve this invariant, only mark during a full gc.
-     */
-    if (trc->isMarkingTracer() && trc->runtime()->gc.isFullGc())
-        setMarked(true);
 }
 
 void

js/src/jsscript.h

@@ -939,16 +939,33 @@ XDRScriptConst(XDRState<mode>* xdr, MutableHandleValue vp);
  */
 class SharedScriptData
 {
+    // This class is reference counted as follows: each pointer from a JSScript
+    // counts as one reference plus there may be one reference from the shared
+    // script data table.
+    mozilla::Atomic<uint32_t> refCount_;
+
     uint32_t dataLength_;
     uint32_t natoms_;
     uint32_t codeLength_;
-    mozilla::Atomic<bool, mozilla::ReleaseAcquire> marked_;
     uintptr_t data_[1];
 
   public:
     static SharedScriptData* new_(ExclusiveContext* cx, uint32_t codeLength,
                                   uint32_t srcnotesLength, uint32_t natoms);
 
+    uint32_t refCount() const {
+        return refCount_;
+    }
+    void incRefCount() {
+        refCount_++;
+    }
+    void decRefCount() {
+        MOZ_ASSERT(refCount_ != 0);
+        refCount_--;
+        if (refCount_ == 0)
+            js_free(this);
+    }
+
     uint32_t dataLength() const {
         return dataLength_;
     }
@@ -972,13 +989,6 @@ class SharedScriptData
         return reinterpret_cast<jsbytecode*>(data() + natoms_ * sizeof(GCPtrAtom));
     }
 
-    bool marked() const {
-        return marked_;
-    }
-    void setMarked(bool marked) {
-        marked_ = marked;
-    }
-
     void traceChildren(JSTracer* trc);
 
   private:
@@ -1008,9 +1018,6 @@ typedef HashSet<SharedScriptData*,
                 ScriptBytecodeHasher,
                 SystemAllocPolicy> ScriptDataTable;
 
-extern void
-UnmarkScriptData(JSRuntime* rt, AutoLockForExclusiveAccess& lock);
-
 extern void
 SweepScriptData(JSRuntime* rt, AutoLockForExclusiveAccess& lock);