Bug 777776 - Properly barrier JSScript::scriptSource_. r=terrence
☠☠ backed out by c394a354eef7 ☠ ☠
authorBenjamin Peterson <benjamin@python.org>
Fri, 27 Jul 2012 14:42:56 -0700
changeset 100773 b69f5004fde895ea61615139b4fd705f49116514
parent 100772 f58214b53c9aa7d8c7e7db18f4c8ca9ff5f1191f
child 100774 551b6acdafb9f3a0474de38a8dde22042446ed16
push id12654
push userbpeterson@mozilla.com
push dateFri, 27 Jul 2012 21:43:08 +0000
treeherdermozilla-inbound@b69f5004fde8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs777776
milestone17.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 777776 - Properly barrier JSScript::scriptSource_. r=terrence
js/src/frontend/BytecodeEmitter.cpp
js/src/jsfun.cpp
js/src/jsscript.cpp
js/src/jsscript.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -4859,17 +4859,17 @@ EmitFunc(JSContext *cx, BytecodeEmitter 
         CompileOptions options(cx);
         options.setPrincipals(parent->principals)
                .setOriginPrincipals(parent->originPrincipals)
                .setCompileAndGo(parent->compileAndGo)
                .setNoScriptRval(false)
                .setVersion(parent->getVersion());
         Rooted<JSScript*> script(cx, JSScript::Create(cx, enclosingScope, false, options,
                                                       parent->staticLevel + 1,
-                                                      bce->script->source,
+                                                      bce->script->scriptSource(),
                                                       funbox->bufStart, funbox->bufEnd));
         if (!script)
             return false;
 
         BytecodeEmitter bce2(bce, bce->parser, &sc, script, bce->callerFrame, bce->hasGlobalScope,
                              pn->pn_pos.begin.lineno);
         if (!bce2.init())
             return false;
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -598,31 +598,31 @@ js::FunctionToString(JSContext *cx, Hand
         if (!out.append("function "))
             return NULL;
         if (fun->atom) {
             if (!out.append(fun->atom))
                 return NULL;
         }
     }
     bool haveSource = fun->isInterpreted();
-    if (haveSource && !fun->script()->source && !fun->script()->loadSource(cx, &haveSource))
+    if (haveSource && !fun->script()->scriptSource() && !fun->script()->loadSource(cx, &haveSource))
             return NULL;
     if (haveSource) {
         RootedScript script(cx, fun->script());
         RootedString src(cx, fun->script()->sourceData(cx));
         if (!src)
             return NULL;
         const jschar *chars = src->getChars(cx);
         if (!chars)
             return NULL;
         bool exprBody = fun->flags & JSFUN_EXPR_CLOSURE;
 
         // The source data for functions created by calling the Function
         // constructor is only the function's body.
-        bool funCon = script->sourceStart == 0 && script->source->argumentsNotIncluded();
+        bool funCon = script->sourceStart == 0 && script->scriptSource()->argumentsNotIncluded();
 
         // Functions created with the constructor should not be using the
         // expression body extension.
         JS_ASSERT_IF(funCon, !exprBody);
         JS_ASSERT_IF(!funCon, src->length() > 0 && chars[0] == '(');
 
         // If a function inherits strict mode by having scopes above it that
         // have "use strict", we insert "use strict" into the body of the
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -514,19 +514,19 @@ js::XDRScript(XDRState<mode> *xdr, Handl
             scriptBits |= (1 << ArgumentsHasVarBinding);
         if (script->analyzedArgsUsage() && script->needsArgsObj())
             scriptBits |= (1 << NeedsArgsObj);
         if (script->filename) {
             scriptBits |= (enclosingScript && enclosingScript->filename == script->filename)
                           ? (1 << ParentFilename)
                           : (1 << OwnFilename);
         }
-        if (script->source) {
+        if (script->scriptSource()) {
             scriptBits |= (1 << HaveSource);
-            if (!enclosingScript || enclosingScript->source != script->source)
+            if (!enclosingScript || enclosingScript->scriptSource() != script->scriptSource())
                 scriptBits |= (1 << OwnSource);
         }
         if (script->isGenerator)
             scriptBits |= (1 << IsGenerator);
         if (script->isGeneratorExp)
             scriptBits |= (1 << IsGeneratorExp);
 
         JS_ASSERT(!script->compileAndGo);
@@ -630,27 +630,29 @@ js::XDRScript(XDRState<mode> *xdr, Handl
         }
     } else if (scriptBits & (1 << ParentFilename)) {
         JS_ASSERT(enclosingScript);
         if (mode == XDR_DECODE)
             script->filename = enclosingScript->filename;
     }
 
     if (scriptBits & (1 << HaveSource)) {
+        ScriptSource *ss = script->scriptSource();
         if (scriptBits & (1 << OwnSource)) {
-            if (!ScriptSource::performXDR<mode>(xdr, &script->source))
+            if (!ScriptSource::performXDR<mode>(xdr, &ss))
                 return false;
         } else {
             JS_ASSERT(enclosingScript);
-            if (mode == XDR_DECODE)
-                script->source = enclosingScript->source;
+            ss = enclosingScript->scriptSource();
         }
+        if (mode == XDR_DECODE)
+            script->setScriptSource(cx, ss);
     } else if (mode == XDR_DECODE) {
-        script->source = NULL;
-        JS_ASSERT_IF(enclosingScript, !enclosingScript->source);
+        script->setScriptSource(cx, NULL);
+        JS_ASSERT_IF(enclosingScript, !enclosingScript->scriptSource());
     }
     if (!xdr->codeUint32(&script->sourceStart))
         return false;
     if (!xdr->codeUint32(&script->sourceEnd))
         return false;
 
     if (mode == XDR_DECODE) {
         script->lineno = lineno;
@@ -1066,45 +1068,56 @@ SourceCompressorThread::waitOnCompressio
     JS_ASSERT(tok->ss->ready());
     tok->ss = NULL;
     tok->chars = NULL;
     tok = NULL;
     PR_Unlock(lock);
 }
 #endif /* JS_THREADSAFE */
 
+void
+JSScript::setScriptSource(JSContext *cx, ScriptSource *ss)
+{
+#ifdef JSGC_INCREMENTAL
+    // During IGC, we need to barrier writing to scriptSource_.
+    if (ss && cx->runtime->gcIncrementalState == MARK && cx->runtime->gcIsFull)
+        ss->mark();
+#endif
+    scriptSource_ = ss;
+}
+
 bool
 JSScript::loadSource(JSContext *cx, bool *worked)
 {
-    JS_ASSERT(!source);
+    JS_ASSERT(!scriptSource_);
     *worked = false;
     if (!cx->runtime->sourceHook)
         return true;
     jschar *src = NULL;
     uint32_t length;
     if (!cx->runtime->sourceHook(cx, this, &src, &length))
         return false;
     if (!src)
         return true;
     ScriptSource *ss = ScriptSource::createFromSource(cx, src, length, false, NULL, true);
     if (!ss) {
         cx->free_(src);
         return false;
     }
-    source = ss;
+    setScriptSource(cx, ss);
     ss->attachToRuntime(cx->runtime);
     *worked = true;
     return true;
 }
 
 JSFixedString *
 JSScript::sourceData(JSContext *cx)
 {
-    JS_ASSERT(source);
-    return source->substring(cx, sourceStart, sourceEnd);
+    JS_ASSERT(scriptSource_);
+    return scriptSource_->substring(cx, sourceStart, sourceEnd);
 }
 
 JSFixedString *
 SourceDataCache::lookup(ScriptSource *ss)
 {
     if (!map_)
         return NULL;
     if (Map::Ptr p = map_->lookup(ss))
@@ -1194,26 +1207,16 @@ ScriptSource::createFromSource(JSContext
     ss->length_ = length;
     ss->compressedLength = 0;
     ss->marked = ss->onRuntime_ = false;
     ss->argumentsNotIncluded_ = argumentsNotIncluded;
 #ifdef DEBUG
     ss->ready_ = false;
 #endif
 
-#ifdef JSGC_INCREMENTAL
-    /*
-     * During the IGC we need to ensure that source is marked whenever it is
-     * accessed even if the name was already in the table. At this point old
-     * scripts pointing to the source may no longer be reachable.
-     */
-    if (cx->runtime->gcIncrementalState != NO_INCREMENTAL && cx->runtime->gcIsFull)
-        ss->marked = true;
-#endif
-
     JS_ASSERT_IF(ownSource, !tok);
 
 #ifdef JS_THREADSAFE
     if (tok && 0) {
         tok->ss = ss;
         tok->chars = src;
         cx->runtime->sourceCompressorThread.compress(tok);
     } else
@@ -1293,16 +1296,17 @@ ScriptSource::sizeOfIncludingThis(JSMall
     // data is a union, but both members are pointers to allocated memory, so
     // just using compressed will work.
     return mallocSizeOf(this) + mallocSizeOf(data.compressed);
 }
 
 void
 ScriptSource::sweep(JSRuntime *rt)
 {
+    JS_ASSERT(rt->gcIsFull);
     ScriptSource *next = rt->scriptSources, **prev = &rt->scriptSources;
     while (next) {
         ScriptSource *cur = next;
         next = cur->next;
         JS_ASSERT(cur->ready());
         JS_ASSERT(cur->onRuntime());
         if (cur->marked) {
             cur->marked = false;
@@ -1595,17 +1599,17 @@ JSScript::Create(JSContext *cx, HandleOb
     // stack if we nest functions more than a few hundred deep, so this will
     // never trigger.  Oh well.
     if (staticLevel > UINT16_MAX) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_TOO_DEEP, js_function_str);
         return NULL;
     }
     script->staticLevel = uint16_t(staticLevel);
 
-    script->source = ss;
+    script->setScriptSource(cx, ss);
     script->sourceStart = bufStart;
     script->sourceEnd = bufEnd;
 
     return script;
 }
 
 static inline uint8_t *
 AllocScriptData(JSContext *cx, size_t size)
@@ -2259,17 +2263,17 @@ js::CloneScript(JSContext *cx, HandleObj
     CompileOptions options(cx);
     options.setPrincipals(cx->compartment->principals)
            .setOriginPrincipals(src->originPrincipals)
            .setCompileAndGo(src->compileAndGo)
            .setNoScriptRval(src->noScriptRval)
            .setVersion(src->getVersion());
     JSScript *dst = JSScript::Create(cx, enclosingScope, src->savedCallerFun,
                                      options, src->staticLevel,
-                                     src->source, src->sourceStart, src->sourceEnd);
+                                     src->scriptSource(), src->sourceStart, src->sourceEnd);
     if (!dst) {
         Foreground::free_(data);
         return NULL;
     }
 
     new (&dst->bindings) Bindings;
     dst->bindings.transfer(&bindings);
 
@@ -2592,18 +2596,18 @@ JSScript::markChildren(JSTracer *trc)
 
     if (enclosingScope_)
         MarkObject(trc, &enclosingScope_, "enclosing");
 
     if (IS_GC_MARKING_TRACER(trc)) {
         if (filename)
             MarkScriptFilename(trc->runtime, filename);
 
-        if (trc->runtime->gcIsFull && source && source->onRuntime())
-            source->mark();
+        if (trc->runtime->gcIsFull && scriptSource_ && scriptSource_->onRuntime())
+            scriptSource_->mark();
     }
 
     bindings.trace(trc);
 
 #ifdef JS_METHODJIT
     for (int constructing = 0; constructing <= 1; constructing++) {
         for (int barriers = 0; barriers <= 1; barriers++) {
             mjit::JITScript *jit = getJIT((bool) constructing, (bool) barriers);
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -432,19 +432,18 @@ struct JSScript : public js::gc::Cell
     js::HeapPtrAtom *atoms;     /* maps immediate index to literal struct */
 
     JSPrincipals    *principals;/* principals for this script */
     JSPrincipals    *originPrincipals; /* see jsapi.h 'originPrincipals' comment */
 
     /* Persistent type information retained across GCs. */
     js::types::TypeScript *types;
 
-    js::ScriptSource *source; /* source code */
-
   private:
+    js::ScriptSource *scriptSource_; /* source code */
 #ifdef JS_METHODJIT
     JITScriptSet *mJITInfo;
 #endif
     js::HeapPtrFunction function_;
     js::HeapPtrObject   enclosingScope_;
 
     // 32-bit fields.
 
@@ -616,16 +615,24 @@ struct JSScript : public js::gc::Cell
      */
     JSFunction *function() const { return function_; }
     void setFunction(JSFunction *fun);
 
     JSFixedString *sourceData(JSContext *cx);
 
     bool loadSource(JSContext *cx, bool *worked);
 
+    js::ScriptSource *scriptSource() {
+        return scriptSource_;
+    }
+
+    void setScriptSource(JSContext *cx, js::ScriptSource *ss);
+
+  public:
+
     /* Return whether this script was compiled for 'eval' */
     bool isForEval() { return isCachedEval || isActiveEval; }
 
 #ifdef DEBUG
     unsigned id();
 #else
     unsigned id() { return 0; }
 #endif