Bug 1845668 - Part 1: Rewrite OffThreadCompilationCompleteRunnable based on Task. r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D184898

dom/script/ScriptLoadContext.cpp

@@ -37,7 +37,6 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(ScriptLoadContext)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(ScriptLoadContext,
                                                 JS::loader::LoadContextBase)
   MOZ_ASSERT(!tmp->mCompileOrDecodeTask);
-  MOZ_ASSERT(!tmp->mRunnable);
   tmp->MaybeUnblockOnload();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
@@ -61,7 +60,6 @@ ScriptLoadContext::ScriptLoadContext()
       mInCompilingList(false),
       mIsTracking(false),
       mWasCompiledOMT(false),
-      mRunnable(nullptr),
       mLineNo(1),
       mColumnNo(0),
       mIsPreload(false),
@@ -71,7 +69,7 @@ ScriptLoadContext::~ScriptLoadContext() {
   MOZ_ASSERT(NS_IsMainThread());
 
   // Off-thread parsing must have completed or cancelled by this point.
-  MOZ_DIAGNOSTIC_ASSERT(!mCompileOrDecodeTask && !mRunnable);
+  MOZ_DIAGNOSTIC_ASSERT(!mCompileOrDecodeTask);
 
   mRequest = nullptr;
 
@@ -102,11 +100,6 @@ void ScriptLoadContext::MaybeCancelOffThreadScript() {
   mCompileOrDecodeTask->Cancel();
   mCompileOrDecodeTask = nullptr;
 
-  // Clear the pointer to the runnable. It may still run later if we didn't
-  // cancel in time. In this case the runnable is held live by the reference
-  // passed to Dispatch, which is dropped after it runs.
-  mRunnable = nullptr;
-
   MaybeUnblockOnload();
 }
 

dom/script/ScriptLoadContext.h

@@ -73,19 +73,15 @@ class Element;
  *
  * In addition to describing how the ScriptLoadRequest will be loaded by the
  * DOM ScriptLoader, the ScriptLoadContext contains fields that facilitate
- * those custom behaviors, including support for offthread parsing, pointers
+ * those custom behaviors, including support for offthread parsing and preload
- * to runnables (for cancellation and cleanup if a script is parsed offthread)
+ * element specific controls.
- * and preload element specific controls.
  *
  */
 
-class OffThreadCompilationCompleteRunnable;
-
 // Base class for the off-thread compile or off-thread decode tasks.
 class CompileOrDecodeTask : public mozilla::Task {
  protected:
-  explicit CompileOrDecodeTask(
+  CompileOrDecodeTask();
-      OffThreadCompilationCompleteRunnable* aCompleteRunnable);
   virtual ~CompileOrDecodeTask();
 
   nsresult InitFrontendContext();
@@ -94,7 +90,7 @@ class CompileOrDecodeTask : public mozilla::Task {
                   RefPtr<JS::Stencil>&& aStencil);
 
   bool IsCancelled(const MutexAutoLock& aProofOfLock) const {
-    return !mCompleteRunnable;
+    return mIsCancelled;
   }
 
  public:
@@ -129,13 +125,7 @@ class CompileOrDecodeTask : public mozilla::Task {
   // and is freed on any thread in the destructor.
   JS::FrontendContext* mFrontendContext = nullptr;
 
-  // The pointed OffThreadCompilationCompleteRunnable is kept alive by
+  bool mIsCancelled = false;
-  // ScriptLoadContext::mRunnable.
-  //
-  // This shouldn't be RefPtr, given this task can be freed off main thread.
-  //
-  // If this task is cancelled before running, this field is cleared.
-  OffThreadCompilationCompleteRunnable* mCompleteRunnable;
 
  private:
   // The result of the compilation or decode.
@@ -247,10 +237,6 @@ class ScriptLoadContext : public JS::loader::LoadContextBase,
   // result or cancelling the task.
   RefPtr<CompileOrDecodeTask> mCompileOrDecodeTask;
 
-  // Runnable that is dispatched to the main thread when off-thread compilation
-  // completes.
-  RefPtr<Runnable> mRunnable;
-
   uint32_t mLineNo;
   uint32_t mColumnNo;
 

dom/script/ScriptLoader.cpp

@@ -1503,39 +1503,49 @@ nsresult ScriptLoader::CompileOffThreadOrProcessRequest(
   return ProcessRequest(aRequest);
 }
 
-class OffThreadCompilationCompleteRunnable : public Runnable {
+namespace {
-  nsMainThreadPtrHandle<ScriptLoadRequest> mRequest;
-  nsMainThreadPtrHandle<ScriptLoader> mLoader;
-  nsCOMPtr<nsISerialEventTarget> mEventTarget;
-  TimeStamp mStartTime;
-  TimeStamp mStopTime;
 
+class OffThreadCompilationCompleteTask : public Task {
  public:
-  OffThreadCompilationCompleteRunnable(ScriptLoadRequest* aRequest,
+  OffThreadCompilationCompleteTask(ScriptLoadRequest* aRequest,
-                                       ScriptLoader* aLoader)
+                                   ScriptLoader* aLoader)
-      : Runnable("dom::OffThreadCompilationCompleteRunnable"),
+      : Task(Kind::MainThreadOnly, EventQueuePriority::Normal),
-        mRequest(
+        mRequest(aRequest),
-            new nsMainThreadPtrHolder<ScriptLoadRequest>("mRequest", aRequest)),
+        mLoader(aLoader) {
-        mLoader(new nsMainThreadPtrHolder<ScriptLoader>("mLoader", aLoader)) {
     MOZ_ASSERT(NS_IsMainThread());
-    if (DocGroup* docGroup = aLoader->GetDocGroup()) {
-      mEventTarget = docGroup->EventTargetFor(TaskCategory::Other);
-    }
   }
 
   void RecordStartTime() { mStartTime = TimeStamp::Now(); }
   void RecordStopTime() { mStopTime = TimeStamp::Now(); }
 
-  static void Dispatch(
+#ifdef MOZ_COLLECTING_RUNNABLE_TELEMETRY
-      already_AddRefed<OffThreadCompilationCompleteRunnable>&& aSelf) {
+  bool GetName(nsACString& aName) override {
-    RefPtr<OffThreadCompilationCompleteRunnable> self = aSelf;
+    aName.AssignLiteral("dom::OffThreadCompilationCompleteTask");
-    nsCOMPtr<nsISerialEventTarget> eventTarget = self->mEventTarget;
+    return true;
-    eventTarget->Dispatch(self.forget());
   }
+#endif
 
-  NS_DECL_NSIRUNNABLE
+  bool Run() override;
+
+ private:
+  // NOTE:
+  // These fields are main-thread only, and this task shouldn't be freed off
+  // main thread.
+  //
+  // This is guaranteed by not having off-thread tasks which depends on this
+  // task, because otherwise the off-thread task's mDependencies can be the
+  // last reference, which results in freeing this task off main thread.
+  //
+  // If such task is added, these fields must be moved to separate storage.
+  RefPtr<ScriptLoadRequest> mRequest;
+  RefPtr<ScriptLoader> mLoader;
+
+  TimeStamp mStartTime;
+  TimeStamp mStopTime;
 };
 
+} /* anonymous namespace */
+
 nsresult ScriptLoader::AttemptOffThreadScriptCompile(
     ScriptLoadRequest* aRequest, bool* aCouldCompileOut) {
   // If speculative parsing is enabled, the request may not be ready to run if
@@ -1598,21 +1608,25 @@ nsresult ScriptLoader::AttemptOffThreadScriptCompile(
     }
   }
 
-  RefPtr<OffThreadCompilationCompleteRunnable> runnable =
+  RefPtr<CompileOrDecodeTask> compileOrDecodeTask;
-      new OffThreadCompilationCompleteRunnable(aRequest, this);
+  rv = CreateOffThreadTask(cx, aRequest, options,
-
+                           getter_AddRefs(compileOrDecodeTask));
-  LogRunnable::LogDispatch(runnable);
-
-  runnable->RecordStartTime();
-
-  rv = StartOffThreadCompilation(cx, aRequest, options, runnable);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  aRequest->GetScriptLoadContext()->mRunnable = runnable;
+  RefPtr<OffThreadCompilationCompleteTask> completeTask =
+      new OffThreadCompilationCompleteTask(aRequest, this);
+
+  completeTask->RecordStartTime();
+
+  aRequest->GetScriptLoadContext()->mCompileOrDecodeTask = compileOrDecodeTask;
+  completeTask->AddDependency(compileOrDecodeTask);
+
+  TaskController::Get()->AddTask(compileOrDecodeTask.forget());
+  TaskController::Get()->AddTask(completeTask.forget());
 
   aRequest->GetScriptLoadContext()->BlockOnload(mDocument);
 
-  // Once the compilation is finished, the runnable will be dispatched to
+  // Once the compilation is finished, the completeTask will be run on
   // the main thread to call ScriptLoader::ProcessOffThreadRequest for the
   // request.
   aRequest->mState = ScriptLoadRequest::State::Compiling;
@@ -1633,25 +1647,22 @@ nsresult ScriptLoader::AttemptOffThreadScriptCompile(
   return NS_OK;
 }
 
-CompileOrDecodeTask::CompileOrDecodeTask(
+CompileOrDecodeTask::CompileOrDecodeTask()
-    OffThreadCompilationCompleteRunnable* aCompleteRunnable)
     : Task(Kind::OffMainThreadOnly, EventQueuePriority::Normal),
       mMutex("CompileOrDecodeTask"),
-      mOptions(JS::OwningCompileOptions::ForFrontendContext()),
+      mOptions(JS::OwningCompileOptions::ForFrontendContext()) {}
-      mCompleteRunnable(aCompleteRunnable) {}
 
 CompileOrDecodeTask::~CompileOrDecodeTask() {
   if (mFrontendContext) {
     JS::DestroyFrontendContext(mFrontendContext);
     mFrontendContext = nullptr;
   }
-  MOZ_ASSERT(!mCompleteRunnable);
 }
 
 nsresult CompileOrDecodeTask::InitFrontendContext() {
   mFrontendContext = JS::NewFrontendContext();
   if (!mFrontendContext) {
-    mCompleteRunnable = nullptr;
+    mIsCancelled = true;
     return NS_ERROR_OUT_OF_MEMORY;
   }
   return NS_OK;
@@ -1667,13 +1678,6 @@ void CompileOrDecodeTask::DidRunTask(const MutexAutoLock& aProofOfLock,
   }
 
   mStencil = std::move(aStencil);
-
-  RefPtr<OffThreadCompilationCompleteRunnable> runnable = mCompleteRunnable;
-  mCompleteRunnable = nullptr;
-
-  LogRunnable::Run run(runnable);
-
-  OffThreadCompilationCompleteRunnable::Dispatch(runnable.forget());
 }
 
 already_AddRefed<JS::Stencil> CompileOrDecodeTask::StealResult(
@@ -1715,7 +1719,7 @@ void CompileOrDecodeTask::Cancel() {
 
   MutexAutoLock lock(mMutex);
 
-  mCompleteRunnable = nullptr;
+  mIsCancelled = true;
 }
 
 enum class CompilationTarget { Script, Module };
@@ -1723,16 +1727,16 @@ enum class CompilationTarget { Script, Module };
 template <CompilationTarget target>
 class ScriptOrModuleCompileTask final : public CompileOrDecodeTask {
  public:
-  ScriptOrModuleCompileTask(ScriptLoader::MaybeSourceText&& aMaybeSource,
+  explicit ScriptOrModuleCompileTask(
-                            OffThreadCompilationCompleteRunnable* aRunnable)
+      ScriptLoader::MaybeSourceText&& aMaybeSource)
-      : CompileOrDecodeTask(aRunnable), mMaybeSource(std::move(aMaybeSource)) {}
+      : CompileOrDecodeTask(), mMaybeSource(std::move(aMaybeSource)) {}
 
   nsresult Init(JS::CompileOptions& aOptions) {
     nsresult rv = InitFrontendContext();
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (!mOptions.copy(mFrontendContext, aOptions)) {
-      mCompleteRunnable = nullptr;
+      mIsCancelled = true;
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
@@ -1791,16 +1795,15 @@ using ModuleCompileTask =
 
 class ScriptDecodeTask final : public CompileOrDecodeTask {
  public:
-  ScriptDecodeTask(const JS::TranscodeRange& aRange,
+  explicit ScriptDecodeTask(const JS::TranscodeRange& aRange)
-                   OffThreadCompilationCompleteRunnable* aRunnable)
+      : CompileOrDecodeTask(), mRange(aRange) {}
-      : CompileOrDecodeTask(aRunnable), mRange(aRange) {}
 
   nsresult Init(JS::DecodeOptions& aOptions) {
     nsresult rv = InitFrontendContext();
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (!mDecodeOptions.copy(mFrontendContext, aOptions)) {
-      mCompleteRunnable = nullptr;
+      mIsCancelled = true;
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
@@ -1849,20 +1852,18 @@ class ScriptDecodeTask final : public CompileOrDecodeTask {
   JS::TranscodeRange mRange;
 };
 
-nsresult ScriptLoader::StartOffThreadCompilation(
+nsresult ScriptLoader::CreateOffThreadTask(
     JSContext* aCx, ScriptLoadRequest* aRequest, JS::CompileOptions& aOptions,
-    OffThreadCompilationCompleteRunnable* aRunnable) {
+    CompileOrDecodeTask** aCompileOrDecodeTask) {
   if (aRequest->IsBytecode()) {
     JS::DecodeOptions decodeOptions(aOptions);
     JS::TranscodeRange range(
         aRequest->mScriptBytecode.begin() + aRequest->mBytecodeOffset,
         aRequest->mScriptBytecode.length() - aRequest->mBytecodeOffset);
-    RefPtr<ScriptDecodeTask> decodeTask =
+    RefPtr<ScriptDecodeTask> decodeTask = new ScriptDecodeTask(range);
-        new ScriptDecodeTask(range, aRunnable);
     nsresult rv = decodeTask->Init(decodeOptions);
     NS_ENSURE_SUCCESS(rv, rv);
-    aRequest->GetScriptLoadContext()->mCompileOrDecodeTask = decodeTask;
+    decodeTask.forget(aCompileOrDecodeTask);
-    TaskController::Get()->AddTask(decodeTask.forget());
     return NS_OK;
   }
 
@@ -1886,11 +1887,10 @@ nsresult ScriptLoader::StartOffThreadCompilation(
 
   if (aRequest->IsModuleRequest()) {
     RefPtr<ModuleCompileTask> compileTask =
-        new ModuleCompileTask(std::move(maybeSource), aRunnable);
+        new ModuleCompileTask(std::move(maybeSource));
     rv = compileTask->Init(aOptions);
     NS_ENSURE_SUCCESS(rv, rv);
-    aRequest->GetScriptLoadContext()->mCompileOrDecodeTask = compileTask;
+    compileTask.forget(aCompileOrDecodeTask);
-    TaskController::Get()->AddTask(compileTask.forget());
     return NS_OK;
   }
 
@@ -1917,28 +1917,21 @@ nsresult ScriptLoader::StartOffThreadCompilation(
   }
 
   RefPtr<ScriptCompileTask> compileTask =
-      new ScriptCompileTask(std::move(maybeSource), aRunnable);
+      new ScriptCompileTask(std::move(maybeSource));
   rv = compileTask->Init(aOptions);
   NS_ENSURE_SUCCESS(rv, rv);
-  aRequest->GetScriptLoadContext()->mCompileOrDecodeTask = compileTask;
+  compileTask.forget(aCompileOrDecodeTask);
-  TaskController::Get()->AddTask(compileTask.forget());
   return NS_OK;
 }
 
-NS_IMETHODIMP
+bool OffThreadCompilationCompleteTask::Run() {
-OffThreadCompilationCompleteRunnable::Run() {
   MOZ_ASSERT(NS_IsMainThread());
 
   RefPtr<ScriptLoadContext> context = mRequest->GetScriptLoadContext();
-  MOZ_ASSERT_IF(context->mRunnable, context->mRunnable == this);
-
-  // Clear the pointer to the runnable. The final reference will be released
-  // when this method returns.
-  context->mRunnable = nullptr;
 
   if (!context->mCompileOrDecodeTask) {
     // Request has been cancelled by MaybeCancelOffThreadScript.
-    return NS_OK;
+    return true;
   }
 
   RecordStopTime();
@@ -1959,11 +1952,11 @@ OffThreadCompilationCompleteRunnable::Run() {
                          profilerLabelString);
   }
 
-  nsresult rv = mLoader->ProcessOffThreadRequest(mRequest);
+  (void)mLoader->ProcessOffThreadRequest(mRequest);
 
   mRequest = nullptr;
   mLoader = nullptr;
-  return rv;
+  return true;
 }
 
 nsresult ScriptLoader::ProcessOffThreadRequest(ScriptLoadRequest* aRequest) {

dom/script/ScriptLoader.h

@@ -104,8 +104,6 @@ class AsyncCompileShutdownObserver final : public nsIObserver {
 // Script loader implementation
 //////////////////////////////////////////////////////////////
 
-class OffThreadCompilationCompleteRunnable;
-
 class ScriptLoader final : public JS::loader::ScriptLoaderInterface {
   class MOZ_STACK_CLASS AutoCurrentScriptUpdater {
    public:
@@ -574,9 +572,9 @@ class ScriptLoader final : public JS::loader::ScriptLoaderInterface {
   nsresult AttemptOffThreadScriptCompile(ScriptLoadRequest* aRequest,
                                          bool* aCouldCompileOut);
 
-  nsresult StartOffThreadCompilation(
+  nsresult CreateOffThreadTask(JSContext* aCx, ScriptLoadRequest* aRequest,
-      JSContext* aCx, ScriptLoadRequest* aRequest, JS::CompileOptions& aOptions,
+                               JS::CompileOptions& aOptions,
-      OffThreadCompilationCompleteRunnable* aRunnable);
+                               CompileOrDecodeTask** aCompileOrDecodeTask);
 
   nsresult ProcessRequest(ScriptLoadRequest* aRequest);
   nsresult CompileOffThreadOrProcessRequest(ScriptLoadRequest* aRequest);