Bug 1552979 - Make StartOffThreadParseTask accept UniquePtr<ParseTask> and not just a raw ParseTask* borrowed (gulp) from one. r=arai
authorJeff Walden <jwalden@mit.edu>
Sun, 19 May 2019 01:06:35 -0700
changeset 475403 41adfcfb70bf8e12c98eb15d11185c97dc6afabd
parent 475402 38beafecef9b5074a7bc9dcacfc7a89ef919ad80
child 475404 3ce85949f2e16fa924f40aaa8e8b69df918dcd1b
push id36061
push usercbrindusan@mozilla.com
push dateFri, 24 May 2019 21:49:59 +0000
treeherdermozilla-central@5d3e1ea77693 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1552979
milestone69.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 1552979 - Make StartOffThreadParseTask accept UniquePtr<ParseTask> and not just a raw ParseTask* borrowed (gulp) from one. r=arai Differential Revision: https://phabricator.services.mozilla.com/D31890
js/src/vm/HelperThreads.cpp
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -840,38 +840,43 @@ static JSObject* CreateGlobalForOffThrea
   // Don't falsely inherit the host's global trace hook.
   creationOptions.setTrace(nullptr);
 
   return JS_NewGlobalObject(cx, &parseTaskGlobalClass,
                             currentRealm->principals(),
                             JS::DontFireOnNewGlobalHook, realmOptions);
 }
 
-static bool QueueOffThreadParseTask(JSContext* cx, ParseTask* task) {
+static bool QueueOffThreadParseTask(JSContext* cx, UniquePtr<ParseTask> task) {
   AutoLockHelperThreadState lock;
 
   bool mustWait = OffThreadParsingMustWaitForGC(cx->runtime());
 
+  // Append null first, then overwrite it on  success, to avoid having two
+  // |task| pointers (one ostensibly "unique") in flight at once.  (Obviously it
+  // would be better if these vectors stored unique pointers themselves....)
   auto& queue = mustWait ? HelperThreadState().parseWaitingOnGC(lock)
                          : HelperThreadState().parseWorklist(lock);
-  if (!queue.append(task)) {
+  if (!queue.append(nullptr)) {
     ReportOutOfMemory(cx);
     return false;
   }
 
+  queue.back() = task.release();
+
   if (!mustWait) {
-    task->activate(cx->runtime());
+    queue.back()->activate(cx->runtime());
     HelperThreadState().notifyOne(GlobalHelperThreadState::PRODUCER, lock);
   }
 
   return true;
 }
 
-bool StartOffThreadParseTask(JSContext* cx, ParseTask* task,
-                             const ReadOnlyCompileOptions& options) {
+static bool StartOffThreadParseTask(JSContext* cx, UniquePtr<ParseTask> task,
+                                    const ReadOnlyCompileOptions& options) {
   // Suppress GC so that calls below do not trigger a new incremental GC
   // which could require barriers on the atoms zone.
   gc::AutoSuppressGC nogc(cx);
   gc::AutoSuppressNurseryCellAlloc noNurseryAlloc(cx);
   AutoSuppressAllocationMetadataBuilder suppressMetadata(cx);
 
   JSObject* global = CreateGlobalForOffThreadParse(cx, nogc);
   if (!global) {
@@ -883,103 +888,98 @@ bool StartOffThreadParseTask(JSContext* 
   // parsing is complete. If this function exits due to error this state is
   // cleared automatically.
   AutoSetCreatedForHelperThread createdForHelper(global);
 
   if (!task->init(cx, options, global)) {
     return false;
   }
 
-  if (!QueueOffThreadParseTask(cx, task)) {
+  if (!QueueOffThreadParseTask(cx, std::move(task))) {
     return false;
   }
 
   createdForHelper.forget();
   return true;
 }
 
 bool js::StartOffThreadParseScript(JSContext* cx,
                                    const ReadOnlyCompileOptions& options,
                                    JS::SourceText<char16_t>& srcBuf,
                                    JS::OffThreadCompileCallback callback,
                                    void* callbackData) {
   auto task = cx->make_unique<ScriptParseTask<char16_t>>(cx, srcBuf, callback,
                                                          callbackData);
-  if (!task || !StartOffThreadParseTask(cx, task.get(), options)) {
+  if (!task) {
     return false;
   }
 
-  Unused << task.release();
-  return true;
+  return StartOffThreadParseTask(cx, std::move(task), options);
 }
 
 bool js::StartOffThreadParseModule(JSContext* cx,
                                    const ReadOnlyCompileOptions& options,
                                    JS::SourceText<char16_t>& srcBuf,
                                    JS::OffThreadCompileCallback callback,
                                    void* callbackData) {
   auto task = cx->make_unique<ModuleParseTask<char16_t>>(cx, srcBuf, callback,
                                                          callbackData);
-  if (!task || !StartOffThreadParseTask(cx, task.get(), options)) {
+  if (!task) {
     return false;
   }
 
-  Unused << task.release();
-  return true;
+  return StartOffThreadParseTask(cx, std::move(task), options);
 }
 
 bool js::StartOffThreadDecodeScript(JSContext* cx,
                                     const ReadOnlyCompileOptions& options,
                                     const JS::TranscodeRange& range,
                                     JS::OffThreadCompileCallback callback,
                                     void* callbackData) {
   auto task =
       cx->make_unique<ScriptDecodeTask>(cx, range, callback, callbackData);
-  if (!task || !StartOffThreadParseTask(cx, task.get(), options)) {
+  if (!task) {
     return false;
   }
 
-  Unused << task.release();
-  return true;
+  return StartOffThreadParseTask(cx, std::move(task), options);
 }
 
 bool js::StartOffThreadDecodeMultiScripts(JSContext* cx,
                                           const ReadOnlyCompileOptions& options,
                                           JS::TranscodeSources& sources,
                                           JS::OffThreadCompileCallback callback,
                                           void* callbackData) {
   auto task = cx->make_unique<MultiScriptsDecodeTask>(cx, sources, callback,
                                                       callbackData);
-  if (!task || !StartOffThreadParseTask(cx, task.get(), options)) {
+  if (!task) {
     return false;
   }
 
-  Unused << task.release();
-  return true;
+  return StartOffThreadParseTask(cx, std::move(task), options);
 }
 
 #if defined(JS_BUILD_BINAST)
 
 bool js::StartOffThreadDecodeBinAST(JSContext* cx,
                                     const ReadOnlyCompileOptions& options,
                                     const uint8_t* buf, size_t length,
                                     JS::OffThreadCompileCallback callback,
                                     void* callbackData) {
   if (!cx->runtime()->binast().ensureBinTablesInitialized(cx)) {
     return false;
   }
 
   auto task = cx->make_unique<BinASTDecodeTask>(cx, buf, length, callback,
                                                 callbackData);
-  if (!task || !StartOffThreadParseTask(cx, task.get(), options)) {
+  if (!task) {
     return false;
   }
 
-  Unused << task.release();
-  return true;
+  return StartOffThreadParseTask(cx, std::move(task), options);
 }
 
 #endif /* JS_BUILD_BINAST */
 
 void js::EnqueuePendingParseTasksAfterGC(JSRuntime* rt) {
   MOZ_ASSERT(!OffThreadParsingMustWaitForGC(rt));
 
   GlobalHelperThreadState::ParseTaskVector newTasks;