Bug 931864 - remove the activeGCInAtomsZone limitation in JS::CanCompileOffThread (r=billm)

--HG--
extra : rebase_source : d0652c47ca940810ad32e4f34aa23a032f585479

browser/devtools/styleeditor/test/browser.ini

@@ -35,6 +35,8 @@ support-files =
 [browser_styleeditor_new.js]
 [browser_styleeditor_nostyle.js]
 [browser_styleeditor_pretty.js]
+# Disabled because of intermittent failures - See Bug 942473
+skip-if = true
 [browser_styleeditor_private_perwindowpb.js]
 [browser_styleeditor_reload.js]
 [browser_styleeditor_sv_keynav.js]

js/src/jsapi.cpp

@@ -4501,17 +4501,7 @@ JS::Compile(JSContext *cx, HandleObject obj, const ReadOnlyCompileOptions &optio
 JS_PUBLIC_API(bool)
 JS::CanCompileOffThread(JSContext *cx, const ReadOnlyCompileOptions &options)
 {
-    if (!cx->runtime()->canUseParallelParsing())
-        return false;
-
-    // Off thread compilation can't occur during incremental collections on the
-    // atoms compartment, to avoid triggering barriers. Outside the atoms
-    // compartment, the compilation will use a new zone which doesn't require
-    // barriers itself.
-    if (cx->runtime()->activeGCInAtomsZone())
-        return false;
-
-    return true;
+    return cx->runtime()->canUseParallelParsing();
 }
 
 JS_PUBLIC_API(bool)

js/src/jsgc.cpp

@@ -4946,6 +4946,12 @@ Collect(JSRuntime *rt, bool incremental, int64_t budget,
          */
         repeat = (rt->gcPoke && rt->gcShouldCleanUpEverything) || wasReset;
     } while (repeat);
+
+    if (rt->gcIncrementalState == NO_INCREMENTAL) {
+#ifdef JS_WORKER_THREADS
+        EnqueuePendingParseTasksAfterGC(rt);
+#endif
+    }
 }
 
 void
@@ -5406,6 +5412,23 @@ js::PurgeJITCaches(Zone *zone)
 #endif
 }
 
+void
+ArenaLists::normalizeBackgroundFinalizeState(AllocKind thingKind)
+{
+    volatile uintptr_t *bfs = &backgroundFinalizeState[thingKind];
+    switch (*bfs) {
+      case BFS_DONE:
+        break;
+      case BFS_JUST_FINISHED:
+        // No allocations between end of last sweep and now.
+        // Transfering over arenas is a kind of allocation.
+        *bfs = BFS_DONE;
+        break;
+      default:
+        JS_ASSERT(!"Background finalization in progress, but it should not be.");
+        break;
+    }
+}
 
 void
 ArenaLists::adoptArenas(JSRuntime *rt, ArenaLists *fromArenaLists)
@@ -5422,21 +5445,9 @@ ArenaLists::adoptArenas(JSRuntime *rt, ArenaLists *fromArenaLists)
         // When we enter a parallel section, we join the background
         // thread, and we do not run GC while in the parallel section,
         // so no finalizer should be active!
-        volatile uintptr_t *bfs = &backgroundFinalizeState[thingKind];
-        switch (*bfs) {
-          case BFS_DONE:
-            break;
-          case BFS_JUST_FINISHED:
-            // No allocations between end of last sweep and now.
-            // Transfering over arenas is a kind of allocation.
-            *bfs = BFS_DONE;
-            break;
-          default:
-            JS_ASSERT(!"Background finalization in progress, but it should not be.");
-            break;
-        }
-#endif /* JS_THREADSAFE */
-
+        normalizeBackgroundFinalizeState(AllocKind(thingKind));
+        fromArenaLists->normalizeBackgroundFinalizeState(AllocKind(thingKind));
+#endif
         ArenaList *fromList = &fromArenaLists->arenaLists[thingKind];
         ArenaList *toList = &arenaLists[thingKind];
         while (fromList->head != nullptr) {

js/src/jsgc.h

@@ -616,6 +616,8 @@ class ArenaLists
     void *allocateFromArena(JS::Zone *zone, AllocKind thingKind);
     inline void *allocateFromArenaInline(JS::Zone *zone, AllocKind thingKind);
 
+    inline void normalizeBackgroundFinalizeState(AllocKind thingKind);
+
     friend class js::Nursery;
 };
 

js/src/jsworkers.cpp

@@ -176,18 +176,20 @@ static const JSClass workerGlobalClass = {
     JS_ConvertStub,   nullptr
 };
 
-ParseTask::ParseTask(ExclusiveContext *cx, JSContext *initCx,
+ParseTask::ParseTask(ExclusiveContext *cx, JSObject *exclusiveContextGlobal, JSContext *initCx,
                      const jschar *chars, size_t length, JSObject *scopeChain,
                      JS::OffThreadCompileCallback callback, void *callbackData)
   : cx(cx), options(initCx), chars(chars), length(length),
     alloc(JSRuntime::TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE), scopeChain(scopeChain),
-    callback(callback), callbackData(callbackData), script(nullptr),
-    errors(cx), overRecursed(false)
+    exclusiveContextGlobal(exclusiveContextGlobal), callback(callback),
+    callbackData(callbackData), script(nullptr), errors(cx), overRecursed(false)
 {
     JSRuntime *rt = scopeChain->runtimeFromMainThread();
 
     if (!AddObjectRoot(rt, &this->scopeChain, "ParseTask::scopeChain"))
         MOZ_CRASH();
+    if (!AddObjectRoot(rt, &this->exclusiveContextGlobal, "ParseTask::exclusiveContextGlobal"))
+        MOZ_CRASH();
 }
 
 bool
@@ -196,11 +198,19 @@ ParseTask::init(JSContext *cx, const ReadOnlyCompileOptions &options)
     return this->options.copy(cx, options);
 }
 
+void
+ParseTask::activate(JSRuntime *rt)
+{
+    rt->setUsedByExclusiveThread(exclusiveContextGlobal->zone());
+    cx->enterCompartment(exclusiveContextGlobal->compartment());
+}
+
 ParseTask::~ParseTask()
 {
     JSRuntime *rt = scopeChain->runtimeFromMainThread();
 
     JS_RemoveObjectRootRT(rt, &scopeChain);
+    JS_RemoveObjectRootRT(rt, &exclusiveContextGlobal);
 
     // ParseTask takes over ownership of its input exclusive context.
     js_delete(cx);
@@ -257,38 +267,75 @@ js::StartOffThreadParseScript(JSContext *cx, const ReadOnlyCompileOptions &optio
         }
     }
 
-    cx->runtime()->setUsedByExclusiveThread(global->zone());
-
     ScopedJSDeletePtr<ExclusiveContext> workercx(
         cx->new_<ExclusiveContext>(cx->runtime(), (PerThreadData *) nullptr,
                                    ThreadSafeContext::Context_Exclusive));
     if (!workercx)
         return false;
 
-    workercx->enterCompartment(global->compartment());
-
     ScopedJSDeletePtr<ParseTask> task(
-        cx->new_<ParseTask>(workercx.get(), cx, chars, length,
+        cx->new_<ParseTask>(workercx.get(), global, cx, chars, length,
                             scopeChain, callback, callbackData));
-    if (!task || !task->init(cx, options))
+    if (!task)
         return false;
 
     workercx.forget();
 
+    if (!task->init(cx, options))
+        return false;
+
     WorkerThreadState &state = *cx->runtime()->workerThreadState;
     JS_ASSERT(state.numThreads);
 
-    AutoLockWorkerThreadState lock(state);
+    // Off thread parsing can't occur during incremental collections on the
+    // atoms compartment, to avoid triggering barriers. (Outside the atoms
+    // compartment, the compilation will use a new zone which doesn't require
+    // barriers itself.) If an atoms-zone GC is in progress, hold off on
+    // executing the parse task until the atoms-zone GC completes (see
+    // EnqueuePendingParseTasksAfterGC).
+    if (cx->runtime()->activeGCInAtomsZone()) {
+        if (!state.parseWaitingOnGC.append(task.get()))
+            return false;
+    } else {
+        task->activate(cx->runtime());
 
-    if (!state.parseWorklist.append(task.get()))
-        return false;
+        AutoLockWorkerThreadState lock(state);
+
+        if (!state.parseWorklist.append(task.get()))
+            return false;
+
+        state.notifyAll(WorkerThreadState::PRODUCER);
+    }
 
     task.forget();
 
-    state.notifyAll(WorkerThreadState::PRODUCER);
     return true;
 }
 
+void
+js::EnqueuePendingParseTasksAfterGC(JSRuntime *rt)
+{
+    JS_ASSERT(!rt->activeGCInAtomsZone());
+
+    if (!rt->workerThreadState || rt->workerThreadState->parseWaitingOnGC.empty())
+        return;
+
+    // This logic should mirror the contents of the !activeGCInAtomsZone()
+    // branch in StartOffThreadParseScript:
+
+    WorkerThreadState &state = *rt->workerThreadState;
+
+    for (size_t i = 0; i < state.parseWaitingOnGC.length(); i++)
+        state.parseWaitingOnGC[i]->activate(rt);
+
+    AutoLockWorkerThreadState lock(state);
+
+    JS_ASSERT(state.parseWorklist.empty());
+    state.parseWorklist.swap(state.parseWaitingOnGC);
+
+    state.notifyAll(WorkerThreadState::PRODUCER);
+}
+
 void
 js::WaitForOffThreadParsingToFinish(JSRuntime *rt)
 {

js/src/jsworkers.h

@@ -71,6 +71,9 @@ class WorkerThreadState
     /* Shared worklist for parsing/emitting scripts on worker threads. */
     Vector<ParseTask*, 0, SystemAllocPolicy> parseWorklist, parseFinishedList;
 
+    /* Main-thread-only list of parse tasks waiting for an atoms-zone GC to complete. */
+    Vector<ParseTask*, 0, SystemAllocPolicy> parseWaitingOnGC;
+
     /* Worklist for source compression worker threads. */
     Vector<SourceCompressionTask *, 0, SystemAllocPolicy> compressionWorklist;
 
@@ -230,6 +233,13 @@ StartOffThreadParseScript(JSContext *cx, const ReadOnlyCompileOptions &options,
                           const jschar *chars, size_t length, HandleObject scopeChain,
                           JS::OffThreadCompileCallback callback, void *callbackData);
 
+/*
+ * Called at the end of GC to enqueue any Parse tasks that were waiting on an
+ * atoms-zone GC to finish.
+ */
+void
+EnqueuePendingParseTasksAfterGC(JSRuntime *rt);
+
 /* Block until in progress and pending off thread parse jobs have finished. */
 void
 WaitForOffThreadParsingToFinish(JSRuntime *rt);
@@ -329,6 +339,9 @@ struct ParseTask
     // main thread.
     JSObject *scopeChain;
 
+    // Rooted pointer to the global object used by 'cx'.
+    JSObject *exclusiveContextGlobal;
+
     // Callback invoked off the main thread when the parse finishes.
     JS::OffThreadCompileCallback callback;
     void *callbackData;
@@ -343,11 +356,13 @@ struct ParseTask
     Vector<frontend::CompileError *> errors;
     bool overRecursed;
 
-    ParseTask(ExclusiveContext *cx, JSContext *initCx,
+    ParseTask(ExclusiveContext *cx, JSObject *exclusiveContextGlobal, JSContext *initCx,
               const jschar *chars, size_t length, JSObject *scopeChain,
               JS::OffThreadCompileCallback callback, void *callbackData);
     bool init(JSContext *cx, const ReadOnlyCompileOptions &options);
 
+    void activate(JSRuntime *rt);
+
     ~ParseTask();
 };