Bug 941827 - Use off-main-thread parsing even if GetCPUCount() == 1 (r=bhackett)
authorLuke Wagner <luke@mozilla.com>
Fri, 29 Nov 2013 08:54:26 -0600
changeset 158133 f4a802140bc7ec319cb7ba2b69afe3f516e35306
parent 158132 4c27e14de1af059fe3a16c49e0ac304f8273006b
child 158134 3ed81454baf97bfcb2b9551f3b146598deedd43a
push id25737
push usercbook@mozilla.com
push dateMon, 02 Dec 2013 11:42:38 +0000
treeherdermozilla-central@5b9a4d273114 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs941827
milestone28.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 941827 - Use off-main-thread parsing even if GetCPUCount() == 1 (r=bhackett)
js/src/builtin/TestingFunctions.cpp
js/src/jit/AsmJS.cpp
js/src/jit/Ion.cpp
js/src/jsapi.cpp
js/src/jscntxt.h
js/src/jsscript.cpp
js/src/jsworkers.cpp
js/src/jsworkers.h
js/src/shell/js.cpp
js/src/vm/Runtime.cpp
js/src/vm/Runtime.h
js/src/vm/ThreadPool.cpp
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -1368,17 +1368,17 @@ Neuter(JSContext *cx, unsigned argc, jsv
     args.rval().setUndefined();
     return true;
 }
 
 static bool
 WorkerThreadCount(JSContext *cx, unsigned argc, jsval *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
-    args.rval().setNumber(static_cast<double>(cx->runtime()->helperThreadCount()));
+    args.rval().setNumber(static_cast<double>(cx->runtime()->workerThreadCount()));
     return true;
 }
 
 static const JSFunctionSpecWithHelp TestingFunctions[] = {
     JS_FN_HELP("gc", ::GC, 0, 0,
 "gc([obj] | 'compartment')",
 "  Run the garbage collector. When obj is given, GC only its compartment.\n"
 "  If 'compartment' is given, GC any compartments that were scheduled for\n"
--- a/js/src/jit/AsmJS.cpp
+++ b/js/src/jit/AsmJS.cpp
@@ -5117,17 +5117,17 @@ ParallelCompilationEnabled(ExclusiveCont
     // off-thread compilation must be enabled. However, since there are a fixed
     // number of worker threads and one is already being consumed by this
     // parsing task, ensure that there another free thread to avoid deadlock.
     // (Note: there is at most one thread used for parsing so we don't have to
     // worry about general dining philosophers.)
     if (!cx->isJSContext())
         return cx->workerThreadState()->numThreads > 1;
 
-    return OffThreadIonCompilationEnabled(cx->asJSContext()->runtime());
+    return cx->asJSContext()->runtime()->canUseParallelIonCompilation();
 }
 
 // State of compilation as tracked and updated by the main thread.
 struct ParallelGroupState
 {
     WorkerThreadState &state;
     js::Vector<AsmJSParallelTask> &tasks;
     int32_t outstandingJobs; // Good work, jobs!
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -1555,17 +1555,17 @@ OffThreadCompilationAvailable(JSContext 
     //
     // Skip off thread compilation if PC count profiling is enabled, as
     // CodeGenerator::maybeCreateScriptCounts will not attach script profiles
     // when running off thread.
     //
     // Also skip off thread compilation if the SPS profiler is enabled, as it
     // stores strings in the spsProfiler data structure, which is not protected
     // by a lock.
-    return OffThreadIonCompilationEnabled(cx->runtime())
+    return cx->runtime()->canUseParallelIonCompilation()
         && cx->runtime()->gcIncrementalState == gc::NO_INCREMENTAL
         && !cx->runtime()->profilingScripts
         && !cx->runtime()->spsProfiler.enabled();
 }
 
 static void
 TrackAllProperties(JSContext *cx, JSObject *obj)
 {
@@ -1785,31 +1785,31 @@ CheckScriptSize(JSContext *cx, JSScript*
         return Method_CantCompile;
     }
 
     uint32_t numLocalsAndArgs = analyze::TotalSlots(script);
     if (cx->runtime()->isWorkerRuntime()) {
         // DOM Workers don't have off thread compilation enabled. Since workers
         // don't block the browser's event loop, allow them to compile larger
         // scripts.
-        JS_ASSERT(!OffThreadIonCompilationEnabled(cx->runtime()));
+        JS_ASSERT(!cx->runtime()->canUseParallelIonCompilation());
 
         if (script->length > MAX_DOM_WORKER_SCRIPT_SIZE ||
             numLocalsAndArgs > MAX_DOM_WORKER_LOCALS_AND_ARGS)
         {
             return Method_CantCompile;
         }
 
         return Method_Compiled;
     }
 
     if (script->length > MAX_MAIN_THREAD_SCRIPT_SIZE ||
         numLocalsAndArgs > MAX_MAIN_THREAD_LOCALS_AND_ARGS)
     {
-        if (OffThreadIonCompilationEnabled(cx->runtime())) {
+        if (cx->runtime()->canUseParallelIonCompilation()) {
             // Even if off thread compilation is enabled, there are cases where
             // compilation must still occur on the main thread. Don't compile
             // in these cases (except when profiling scripts, as compilations
             // occurring with profiling should reflect those without), but do
             // not forbid compilation so that the script may be compiled later.
             if (!OffThreadCompilationAvailable(cx) && !cx->runtime()->profilingScripts) {
                 IonSpew(IonSpew_Abort,
                         "Script too large for main thread, skipping (%u bytes) (%u locals/args)",
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4500,47 +4500,36 @@ JS::Compile(JSContext *cx, HandleObject 
     options.setFileAndLine(filename, 1);
     JSScript *script = Compile(cx, obj, options, file.fp());
     return script;
 }
 
 JS_PUBLIC_API(bool)
 JS::CanCompileOffThread(JSContext *cx, const ReadOnlyCompileOptions &options)
 {
-#ifdef JS_WORKER_THREADS
-    if (!cx->runtime()->useHelperThreads() || !cx->runtime()->helperThreadCount())
-        return false;
-
-    if (!cx->runtime()->useHelperThreadsForParsing())
+    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;
-#else
-    return false;
-#endif
 }
 
 JS_PUBLIC_API(bool)
 JS::CompileOffThread(JSContext *cx, Handle<JSObject*> obj, const ReadOnlyCompileOptions &options,
                      const jschar *chars, size_t length,
                      OffThreadCompileCallback callback, void *callbackData)
 {
-#ifdef JS_WORKER_THREADS
     JS_ASSERT(CanCompileOffThread(cx, options));
     return StartOffThreadParseScript(cx, options, chars, length, obj, callback, callbackData);
-#else
-    MOZ_ASSUME_UNREACHABLE("Off thread compilation is not available.");
-#endif
 }
 
 JS_PUBLIC_API(JSScript *)
 JS::FinishOffThreadScript(JSContext *maybecx, JSRuntime *rt, void *token)
 {
 #ifdef JS_WORKER_THREADS
     JS_ASSERT(CurrentThreadCanAccessRuntime(rt));
 
@@ -6003,25 +5992,25 @@ JS_ScheduleGC(JSContext *cx, uint32_t co
     cx->runtime()->gcNextScheduled = count;
 }
 #endif
 
 JS_PUBLIC_API(void)
 JS_SetParallelParsingEnabled(JSContext *cx, bool enabled)
 {
 #ifdef JS_ION
-    cx->runtime()->setCanUseHelperThreadsForParsing(enabled);
+    cx->runtime()->setParallelParsingEnabled(enabled);
 #endif
 }
 
 JS_PUBLIC_API(void)
 JS_SetParallelIonCompilationEnabled(JSContext *cx, bool enabled)
 {
 #ifdef JS_ION
-    cx->runtime()->setCanUseHelperThreadsForIonCompilation(enabled);
+    cx->runtime()->setParallelIonCompilationEnabled(enabled);
 #endif
 }
 
 JS_PUBLIC_API(void)
 JS_SetGlobalJitCompilerOption(JSContext *cx, JSJitCompilerOption opt, uint32_t value)
 {
 #ifdef JS_ION
     jit::IonOptions defaultValues;
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -277,17 +277,18 @@ struct ThreadSafeContext : ContextFriend
 
     // Accessors for immutable runtime data.
     JSAtomState &names() { return runtime_->atomState; }
     StaticStrings &staticStrings() { return runtime_->staticStrings; }
     const JS::AsmJSCacheOps &asmJSCacheOps() { return runtime_->asmJSCacheOps; }
     PropertyName *emptyString() { return runtime_->emptyString; }
     FreeOp *defaultFreeOp() { return runtime_->defaultFreeOp(); }
     bool useHelperThreads() { return runtime_->useHelperThreads(); }
-    size_t helperThreadCount() { return runtime_->helperThreadCount(); }
+    unsigned cpuCount() { return runtime_->cpuCount(); }
+    size_t workerThreadCount() { return runtime_->workerThreadCount(); }
     void *runtimeAddressForJit() { return runtime_; }
     void *stackLimitAddress(StackKind kind) { return &runtime_->mainThread.nativeStackLimit[kind]; }
     size_t gcSystemPageSize() { return runtime_->gcSystemPageSize; }
     bool signalHandlersInstalled() const { return runtime_->signalHandlersInstalled(); }
     bool jitSupportsFloatingPoint() const { return runtime_->jitSupportsFloatingPoint; }
 
     // Thread local data that may be accessed freely.
     DtoaState *dtoaState() {
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -1178,21 +1178,28 @@ ScriptSource::substring(JSContext *cx, u
 bool
 ScriptSource::setSourceCopy(ExclusiveContext *cx, const jschar *src, uint32_t length,
                             bool argumentsNotIncluded, SourceCompressionTask *task)
 {
     JS_ASSERT(!hasSourceData());
     length_ = length;
     argumentsNotIncluded_ = argumentsNotIncluded;
 
-    // Only compress off thread if there is at least one more thread
-    // available to do the compression.
-    size_t minThreads = cx->isJSContext() ? 1 : 2;
-
-    if (task && cx->useHelperThreads() && cx->helperThreadCount() >= minThreads) {
+    // Don't use background compression if there is only one core since this
+    // will contend with JS execution (which affects benchmarketting). Also,
+    // since this thread is about to perform a blocking wait, require that there
+    // are at least 2 worker threads:
+    //  - If we are on a worker thread, there must be another worker thread to
+    //    execute our compression task.
+    //  - If we are on the main thread, there must be at least two worker
+    //    threads since at most one worker thread can be blocking on the main
+    //    thread (see WorkerThreadState::canStartParseTask) which would cause a
+    //    deadlock if there wasn't a second worker thread that could make
+    //    progress on our compression task.
+    if (task && cx->cpuCount() > 1 && cx->workerThreadCount() >= 2) {
         task->ss = this;
         task->chars = src;
         ready_ = false;
         if (!StartOffThreadCompression(cx, task))
             return false;
     } else {
         if (!adjustDataSize(sizeof(jschar) * length))
             return false;
--- a/js/src/jsworkers.cpp
+++ b/js/src/jsworkers.cpp
@@ -312,59 +312,55 @@ js::WaitForOffThreadParsingToFinish(JSRu
 }
 
 static const uint32_t WORKER_STACK_SIZE = 512 * 1024;
 static const uint32_t WORKER_STACK_QUOTA = 450 * 1024;
 
 bool
 WorkerThreadState::init()
 {
-    if (!runtime->useHelperThreads()) {
-        numThreads = 0;
+    JS_ASSERT(numThreads == 0);
+
+    if (!runtime->useHelperThreads())
         return true;
-    }
 
     workerLock = PR_NewLock();
     if (!workerLock)
         return false;
 
     consumerWakeup = PR_NewCondVar(workerLock);
     if (!consumerWakeup)
         return false;
 
     producerWakeup = PR_NewCondVar(workerLock);
     if (!producerWakeup)
         return false;
 
-    numThreads = runtime->helperThreadCount();
+    threads = (WorkerThread*) js_pod_calloc<WorkerThread>(runtime->workerThreadCount());
+    if (!threads)
+        return false;
 
-    threads = (WorkerThread*) js_pod_calloc<WorkerThread>(numThreads);
-    if (!threads) {
-        numThreads = 0;
-        return false;
-    }
-
-    for (size_t i = 0; i < numThreads; i++) {
+    for (size_t i = 0; i < runtime->workerThreadCount(); i++) {
         WorkerThread &helper = threads[i];
         helper.runtime = runtime;
         helper.threadData.construct(runtime);
         helper.threadData.ref().addToThreadList();
         helper.thread = PR_CreateThread(PR_USER_THREAD,
                                         WorkerThread::ThreadMain, &helper,
                                         PR_PRIORITY_NORMAL, PR_LOCAL_THREAD, PR_JOINABLE_THREAD, WORKER_STACK_SIZE);
         if (!helper.thread || !helper.threadData.ref().init()) {
-            for (size_t j = 0; j < numThreads; j++)
+            for (size_t j = 0; j < runtime->workerThreadCount(); j++)
                 threads[j].destroy();
             js_free(threads);
             threads = nullptr;
-            numThreads = 0;
             return false;
         }
     }
 
+    numThreads = runtime->workerThreadCount();
     resetAsmJSFailureState();
     return true;
 }
 
 void
 WorkerThreadState::cleanup()
 {
     // Do preparatory work for shutdown before the final GC has destroyed most
--- a/js/src/jsworkers.h
+++ b/js/src/jsworkers.h
@@ -192,28 +192,16 @@ struct WorkerThread
     void handleCompressionWorkload(WorkerThreadState &state);
 
     static void ThreadMain(void *arg);
     void threadLoop();
 };
 
 #endif /* JS_WORKER_THREADS */
 
-inline bool
-OffThreadIonCompilationEnabled(JSRuntime *rt)
-{
-#ifdef JS_WORKER_THREADS
-    return rt->useHelperThreads()
-        && rt->helperThreadCount() != 0
-        && rt->useHelperThreadsForIonCompilation();
-#else
-    return false;
-#endif
-}
-
 /* Methods for interacting with worker threads. */
 
 /* Initialize worker threads unless already initialized. */
 bool
 EnsureWorkerThreadsInitialized(ExclusiveContext *cx);
 
 /* Perform MIR optimization and LIR generation on a single function. */
 bool
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -5403,17 +5403,17 @@ ProcessArgs(JSContext *cx, JSObject *obj
     if (op->getBoolOption('D')) {
         cx->runtime()->profilingScripts = true;
         enableDisassemblyDumps = true;
     }
 
 #ifdef JS_THREADSAFE
     int32_t threadCount = op->getIntOption("thread-count");
     if (threadCount >= 0)
-        cx->runtime()->requestHelperThreadCount(threadCount);
+        cx->runtime()->setFakeCPUCount(threadCount);
 #endif /* JS_THREADSAFE */
 
 #if defined(JS_ION)
     if (op->getBoolOption("no-ion")) {
         enableIon = false;
         JS::ContextOptionsRef(cx).toggleIon();
     }
     if (op->getBoolOption("no-asmjs")) {
@@ -5524,30 +5524,30 @@ ProcessArgs(JSContext *cx, JSObject *obj
 
     if (op->getBoolOption("ion-compile-try-catch"))
         jit::js_IonOptions.compileTryCatch = true;
 
 #ifdef JS_THREADSAFE
     bool parallelCompilation = false;
     if (const char *str = op->getStringOption("ion-parallel-compile")) {
         if (strcmp(str, "on") == 0) {
-            if (cx->runtime()->helperThreadCount() == 0) {
+            if (cx->runtime()->workerThreadCount() == 0) {
                 fprintf(stderr, "Parallel compilation not available without helper threads");
                 return EXIT_FAILURE;
             }
             parallelCompilation = true;
         } else if (strcmp(str, "off") != 0) {
             return OptionFailure("ion-parallel-compile", str);
         }
     }
     /*
      * Note: In shell builds, parallel compilation is only enabled with an
      * explicit option.
      */
-    cx->runtime()->setCanUseHelperThreadsForIonCompilation(parallelCompilation);
+    cx->runtime()->setParallelIonCompilationEnabled(parallelCompilation);
 #endif /* JS_THREADSAFE */
 
 #endif /* JS_ION */
 
 #ifdef DEBUG
     if (op->getBoolOption("dump-entrained-variables"))
         dumpEntrainedVariables = true;
 #endif
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -288,24 +288,30 @@ JSRuntime::JSRuntime(JSUseHelperThreads 
     jitSupportsFloatingPoint(false),
     ionPcScriptCache(nullptr),
     threadPool(this),
     defaultJSContextCallback(nullptr),
     ctypesActivityCallback(nullptr),
     parallelWarmup(0),
     ionReturnOverride_(MagicValue(JS_ARG_POISON)),
     useHelperThreads_(useHelperThreads),
-    requestedHelperThreadCount(-1),
-    useHelperThreadsForIonCompilation_(true),
-    useHelperThreadsForParsing_(true),
+#ifdef JS_THREADSAFE
+    cpuCount_(GetCPUCount()),
+#else
+    cpuCount_(1),
+#endif
+    parallelIonCompilationEnabled_(true),
+    parallelParsingEnabled_(true),
     isWorkerRuntime_(false)
 #ifdef DEBUG
     , enteredPolicy(nullptr)
 #endif
 {
+    MOZ_ASSERT(cpuCount_ > 0, "GetCPUCount() seems broken");
+
     liveRuntimesCount++;
 
     setGCMode(JSGC_MODE_GLOBAL);
 
     /* Initialize infallibly first, so we can goto bad and JS_DestroyRuntime. */
     JS_INIT_CLIST(&onNewGlobalObjectWatchers);
 
     PodZero(&debugHooks);
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -1704,65 +1704,80 @@ struct JSRuntime : public JS::shadow::Ru
         return jitHardening;
     }
 
     void addSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf, JS::RuntimeSizes *runtime);
 
   private:
 
     JSUseHelperThreads useHelperThreads_;
-    int32_t requestedHelperThreadCount;
+    unsigned cpuCount_;
 
     // Settings for how helper threads can be used.
-    bool useHelperThreadsForIonCompilation_;
-    bool useHelperThreadsForParsing_;
+    bool parallelIonCompilationEnabled_;
+    bool parallelParsingEnabled_;
 
     // True iff this is a DOM Worker runtime.
     bool isWorkerRuntime_;
 
   public:
 
+    // This controls whether the JSRuntime is allowed to create any helper
+    // threads at all. This means both specific threads (background GC thread)
+    // and the general JS worker thread pool.
     bool useHelperThreads() const {
 #ifdef JS_THREADSAFE
         return useHelperThreads_ == JS_USE_HELPER_THREADS;
 #else
         return false;
 #endif
     }
 
-    void requestHelperThreadCount(size_t count) {
-        requestedHelperThreadCount = count;
+    // This allows the JS shell to override GetCPUCount() when passed the
+    // --thread-count=N option.
+    void setFakeCPUCount(size_t count) {
+        cpuCount_ = count;
+    }
+
+    // Return a cached value of GetCPUCount() to avoid making the syscall all
+    // the time. Furthermore, this avoids pathological cases where the result of
+    // GetCPUCount() changes during execution.
+    unsigned cpuCount() const {
+        JS_ASSERT(cpuCount_ > 0);
+        return cpuCount_;
     }
 
-    /* Number of helper threads which should be created for this runtime. */
-    size_t helperThreadCount() const {
-#ifdef JS_WORKER_THREADS
-        if (requestedHelperThreadCount < 0) {
-            unsigned ncpus = js::GetCPUCount();
-            return ncpus == 1 ? 0 : ncpus;
-        }
-        return requestedHelperThreadCount;
-#else
-        return 0;
-#endif
+    // The number of worker threads that will be available after
+    // EnsureWorkerThreadsInitialized has been called successfully.
+    unsigned workerThreadCount() const {
+        if (!useHelperThreads())
+            return 0;
+        return js::Max(2u, cpuCount());
     }
 
-    void setCanUseHelperThreadsForIonCompilation(bool value) {
-        useHelperThreadsForIonCompilation_ = value;
+    // Note: these values may be toggled dynamically (in response to about:config
+    // prefs changing).
+    void setParallelIonCompilationEnabled(bool value) {
+        parallelIonCompilationEnabled_ = value;
     }
-    bool useHelperThreadsForIonCompilation() const {
-        return useHelperThreadsForIonCompilation_;
+    bool canUseParallelIonCompilation() const {
+        // Require cpuCount_ > 1 so that Ion compilation jobs and main-thread
+        // execution are not competing for the same resources.
+        return useHelperThreads() &&
+               parallelIonCompilationEnabled_ &&
+               cpuCount_ > 1;
+    }
+    void setParallelParsingEnabled(bool value) {
+        parallelParsingEnabled_ = value;
+    }
+    bool canUseParallelParsing() const {
+        return useHelperThreads() &&
+               parallelParsingEnabled_;
     }
 
-    void setCanUseHelperThreadsForParsing(bool value) {
-        useHelperThreadsForParsing_ = value;
-    }
-    bool useHelperThreadsForParsing() const {
-        return useHelperThreadsForParsing_;
-    }
     void setIsWorkerRuntime() {
         isWorkerRuntime_ = true;
     }
     bool isWorkerRuntime() const {
         return isWorkerRuntime_;
     }
 
 #ifdef DEBUG
--- a/js/src/vm/ThreadPool.cpp
+++ b/js/src/vm/ThreadPool.cpp
@@ -190,17 +190,17 @@ ThreadPool::ThreadPool(JSRuntime *rt)
 ThreadPool::~ThreadPool()
 {
     terminateWorkers();
 }
 
 size_t
 ThreadPool::numWorkers() const
 {
-    return runtime_->helperThreadCount();
+    return runtime_->workerThreadCount();
 }
 
 bool
 ThreadPool::lazyStartWorkers(JSContext *cx)
 {
     // Starts the workers if they have not already been started.  If
     // something goes wrong, reports an error and ensures that all
     // partially started threads are terminated.  Therefore, upon exit