Bug 1005306 - Improve GC-handling of canonical JSScript for LazyScripts. r=jonco,billm
☠☠ backed out by 6f3aaae237e9 ☠ ☠
authorTill Schneidereit <till@tillschneidereit.net>
Thu, 28 Aug 2014 02:39:08 +0200
changeset 223724 b9c155d1b30d62fffa89a7fa2f859409961eae31
parent 223723 396db9a7dceac23b2a2f645a4866b495f99cfbd5
child 223725 6dd4ea56ab112af2cb68277babd89a9cb0b9f63d
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco, billm
bugs1005306
milestone34.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 1005306 - Improve GC-handling of canonical JSScript for LazyScripts. r=jonco,billm
js/src/gc/Marking.cpp
js/src/jsfun.cpp
js/src/jsscript.cpp
js/src/jsscript.h
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -484,30 +484,21 @@ IsAboutToBeFinalized(T **thingp)
             if (IsInsideNursery(thing))
                 return !nursery.getForwardedPointer(thingp);
             return false;
         }
     }
 #endif  // JSGC_GENERATIONAL
 
     Zone *zone = thing->tenuredZone();
-    if (zone->isGCSweeping()) {
-        /*
-         * We should return false for things that have been allocated during
-         * incremental sweeping, but this possibility doesn't occur at the moment
-         * because this function is only called at the very start of the sweeping a
-         * compartment group and during minor gc. Rather than do the extra check,
-         * we just assert that it's not necessary.
-         */
-        JS_ASSERT_IF(!rt->isHeapMinorCollecting(), !thing->arenaHeader()->allocatedDuringIncremental);
+    if (zone->isGCSweeping())
+        return !(thing->isMarked() || thing->arenaHeader()->allocatedDuringIncremental);
 
-        return !thing->isMarked();
-    }
 #ifdef JSGC_COMPACTING
-    else if (zone->isGCCompacting() && IsForwarded(thing)) {
+    if (zone->isGCCompacting() && IsForwarded(thing)) {
         *thingp = Forwarded(thing);
         return false;
     }
 #endif
 
     return false;
 }
 
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1526,21 +1526,16 @@ JSFunction::relazify(JSTracer *trc)
         MarkScriptUnbarriered(trc, &u.i.s.script_, "script");
 
     flags_ &= ~INTERPRETED;
     flags_ |= INTERPRETED_LAZY;
     LazyScript *lazy = script->maybeLazyScript();
     u.i.s.lazy_ = lazy;
     if (lazy) {
         JS_ASSERT(!isSelfHostedBuiltin());
-        // If this is the script stored in the lazy script to be cloned
-        // for un-lazifying other functions, reset it so the script can
-        // be freed.
-        if (lazy->maybeScript() == script)
-            lazy->resetScript();
         MarkLazyScriptUnbarriered(trc, &u.i.s.lazy_, "lazyScript");
     } else {
         JS_ASSERT(isSelfHostedBuiltin());
         JS_ASSERT(isExtended());
         JS_ASSERT(getExtendedSlot(0).toString()->isAtom());
     }
 }
 
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -2629,16 +2629,24 @@ JSScript::finalize(FreeOp *fop)
     destroyDebugScript(fop);
 
     if (data) {
         JS_POISON(data, 0xdb, computedSizeOfData());
         fop->free_(data);
     }
 
     fop->runtime()->lazyScriptCache.remove(this);
+    // Calling lazyScript->maybeScript here will reset lazyScript::script_ iff
+    // it points to this JSScript or another JSScript that is about to be
+    // finalized, so just the call guarantees that the lazyScript doesn't keep
+    // a pointer to the finalized script around.
+    if (lazyScript) {
+        lazyScript->maybeScript();
+        MOZ_ASSERT(lazyScript->maybeScript() != this);
+    }
 }
 
 static const uint32_t GSN_CACHE_THRESHOLD = 100;
 
 void
 GSNCache::purge()
 {
     code = nullptr;
@@ -3345,18 +3353,25 @@ JSScript::markChildren(JSTracer *trc)
     }
 
     if (functionNonDelazifying())
         MarkObject(trc, &function_, "function");
 
     if (enclosingScopeOrOriginalFunction_)
         MarkObject(trc, &enclosingScopeOrOriginalFunction_, "enclosing");
 
-    if (maybeLazyScript())
+    if (maybeLazyScript()) {
         MarkLazyScriptUnbarriered(trc, &lazyScript, "lazyScript");
+        // We might be operating on a clone of a script, in which case
+        // lazyScript->maybeScript will return the canonical script. Marking
+        // the lazyScript's script here guarantees that it stays around as
+        // long as any clones do.
+        if (lazyScript->maybeScript())
+            lazyScript->markScript(trc);
+    }
 
     if (IS_GC_MARKING_TRACER(trc)) {
         compartment()->mark();
 
         if (code())
             MarkScriptData(trc->runtime(), code());
     }
 
@@ -3372,19 +3387,16 @@ LazyScript::markChildren(JSTracer *trc)
         MarkObject(trc, &function_, "function");
 
     if (sourceObject_)
         MarkObject(trc, &sourceObject_, "sourceObject");
 
     if (enclosingScope_)
         MarkObject(trc, &enclosingScope_, "enclosingScope");
 
-    if (script_)
-        MarkScript(trc, &script_, "realScript");
-
     HeapPtrAtom *freeVariables = this->freeVariables();
     for (size_t i = 0; i < numFreeVariables(); i++)
         MarkString(trc, &freeVariables[i], "lazyScriptFreeVariable");
 
     HeapPtrFunction *innerFunctions = this->innerFunctions();
     for (size_t i = 0; i < numInnerFunctions(); i++)
         MarkObject(trc, &innerFunctions[i], "lazyScriptInnerFunction");
 }
@@ -3615,16 +3627,23 @@ LazyScript::initScript(JSScript *script)
 void
 LazyScript::resetScript()
 {
     JS_ASSERT(script_);
     script_ = nullptr;
 }
 
 void
+LazyScript::markScript(JSTracer *trc)
+{
+    JS_ASSERT(script_);
+    MarkScript(trc, &script_, "script");
+}
+
+void
 LazyScript::setParent(JSObject *enclosingScope, ScriptSourceObject *sourceObject)
 {
     JS_ASSERT(!sourceObject_ && !enclosingScope_);
     JS_ASSERT_IF(enclosingScope, function_->compartment() == enclosingScope->compartment());
     JS_ASSERT(function_->compartment() == sourceObject->compartment());
 
     enclosingScope_ = enclosingScope;
     sourceObject_ = sourceObject;
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -1772,17 +1772,27 @@ class LazyScript : public gc::BarrieredC
 
     inline JSFunction *functionDelazifying(JSContext *cx) const;
     JSFunction *functionNonDelazifying() const {
         return function_;
     }
 
     void initScript(JSScript *script);
     void resetScript();
+    void markScript(JSTracer *trc);
     JSScript *maybeScript() {
+        // script_ is a weak pointer that only gets marked if the JSScript
+        // it points to is. If the script isn't marked and about to be
+        // collected, we manually reset the pointer here.
+        if (script_) {
+            if (gc::IsAboutToBeFinalized(&script_))
+                script_ = nullptr;
+            else
+                JSScript::readBarrier(script_);
+        }
         return script_;
     }
 
     JSObject *enclosingScope() const {
         return enclosingScope_;
     }
     ScriptSourceObject *sourceObject() const;
     ScriptSource *scriptSource() const {