Bug 777776 - Properly barrier JSScript::scriptSource_. r=terrence
☠☠ backed out by a04448be734a ☠ ☠
authorBenjamin Peterson <benjamin@python.org>
Fri, 27 Jul 2012 17:51:49 -0700
changeset 100799 4cdd2356937121f0ef2e28c0238981efca4d81b3
parent 100798 02d63f48e7526de741d9f02610ec9e8f53d0fa6d
child 100800 088951918af8bc2fb36c2d9aa0645c6ffbefdb2f
push id23193
push userryanvm@gmail.com
push dateSat, 28 Jul 2012 21:54:39 +0000
treeherdermozilla-central@29bff59d3bbe [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
@@ -992,17 +999,17 @@ struct ScriptSource
   public:
     static ScriptSource *createFromSource(JSContext *cx,
                                           const jschar *src,
                                           uint32_t length,
                                           bool argumentsNotIncluded = false,
                                           SourceCompressionToken *tok = NULL,
                                           bool ownSource = false);
     void attachToRuntime(JSRuntime *rt);
-    void mark() { JS_ASSERT(ready_); JS_ASSERT(onRuntime_); marked = true; }
+    void mark() { marked = true; }
     void destroy(JSRuntime *rt);
     uint32_t length() const { return length_; }
     bool onRuntime() const { return onRuntime_; }
     bool argumentsNotIncluded() const { return argumentsNotIncluded_; }
 #ifdef DEBUG
     bool ready() const { return ready_; }
 #endif
     JSFixedString *substring(JSContext *cx, uint32_t start, uint32_t stop);