Bug 1594814 - move FileBlockCache over to a background thread; r=pehrsons
authorNathan Froyd <froydnj@mozilla.com>
Mon, 16 Dec 2019 07:52:41 +0000
changeset 507088 92e21dd89295b9abdb808574fbecc1055a7087f6
parent 507087 52e6fc030c2f9a9bf54d44dbade100644e5f3f41
child 507089 0d3256a2c780b9d2ca106d0703f51c2681b67d22
push id36922
push userncsoregi@mozilla.com
push dateMon, 16 Dec 2019 17:21:47 +0000
treeherdermozilla-central@27d0d6cc2131 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspehrsons
bugs1594814
milestone73.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 1594814 - move FileBlockCache over to a background thread; r=pehrsons Sharing a common thread(pool) is better than spinning up a private thread. Differential Revision: https://phabricator.services.mozilla.com/D57146
dom/media/FileBlockCache.cpp
dom/media/FileBlockCache.h
--- a/dom/media/FileBlockCache.cpp
+++ b/dom/media/FileBlockCache.cpp
@@ -26,78 +26,77 @@ static void CloseFD(PRFileDesc* aFD) {
   PRStatus prrc;
   prrc = PR_Close(aFD);
   if (prrc != PR_SUCCESS) {
     NS_WARNING("PR_Close() failed.");
   }
 }
 
 void FileBlockCache::SetCacheFile(PRFileDesc* aFD) {
-  LOG("SetFD(aFD=%p) mThread=%p", aFD, mThread.get());
+  LOG("SetFD(aFD=%p) mBackgroundET=%p", aFD, mBackgroundET.get());
 
   if (!aFD) {
     // Failed to get a temporary file. Shutdown.
     Close();
     return;
   }
   {
     MutexAutoLock lock(mFileMutex);
     mFD = aFD;
   }
   {
     MutexAutoLock lock(mDataMutex);
-    if (mThread) {
+    if (mBackgroundET) {
       // Still open, complete the initialization.
       mInitialized = true;
       if (mIsWriteScheduled) {
         // A write was scheduled while waiting for FD. We need to run/dispatch a
         // task to service the request.
         nsCOMPtr<nsIRunnable> event = mozilla::NewRunnableMethod(
             "FileBlockCache::SetCacheFile -> PerformBlockIOs", this,
             &FileBlockCache::PerformBlockIOs);
-        mThread->Dispatch(event.forget(), NS_DISPATCH_NORMAL);
+        mBackgroundET->Dispatch(event.forget(), NS_DISPATCH_EVENT_MAY_BLOCK);
       }
       return;
     }
   }
   // We've been closed while waiting for the file descriptor.
   // Close the file descriptor we've just received, if still there.
   MutexAutoLock lock(mFileMutex);
   if (mFD) {
     CloseFD(mFD);
     mFD = nullptr;
   }
 }
 
 nsresult FileBlockCache::Init() {
   LOG("Init()");
   MutexAutoLock mon(mDataMutex);
-  MOZ_ASSERT(!mThread);
-  nsresult rv =
-      NS_NewNamedThread("FileBlockCache", getter_AddRefs(mThread), nullptr,
-                        nsIThreadManager::kThreadPoolStackSize);
+  MOZ_ASSERT(!mBackgroundET);
+  nsresult rv = NS_CreateBackgroundTaskQueue("FileBlockCache",
+                                             getter_AddRefs(mBackgroundET));
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   if (XRE_IsParentProcess()) {
     RefPtr<FileBlockCache> self = this;
-    rv = mThread->Dispatch(
+    rv = mBackgroundET->Dispatch(
         NS_NewRunnableFunction("FileBlockCache::Init",
                                [self] {
                                  PRFileDesc* fd = nullptr;
                                  nsresult rv =
                                      NS_OpenAnonymousTemporaryFile(&fd);
                                  if (NS_SUCCEEDED(rv)) {
                                    self->SetCacheFile(fd);
                                  } else {
                                    self->Close();
                                  }
                                }),
-        NS_DISPATCH_NORMAL);
+        NS_DISPATCH_EVENT_MAY_BLOCK);
   } else {
     // We must request a temporary file descriptor from the parent process.
     RefPtr<FileBlockCache> self = this;
     rv = dom::ContentChild::GetSingleton()->AsyncOpenAnonymousTemporaryFile(
         [self](PRFileDesc* aFD) { self->SetCacheFile(aFD); });
   }
 
   if (NS_FAILED(rv)) {
@@ -105,22 +104,22 @@ nsresult FileBlockCache::Init() {
   }
 
   return rv;
 }
 
 void FileBlockCache::Flush() {
   LOG("Flush()");
   MutexAutoLock mon(mDataMutex);
-  MOZ_ASSERT(mThread);
+  MOZ_ASSERT(mBackgroundET);
 
   // Dispatch a task so we won't clear the arrays while PerformBlockIOs() is
   // dropping the data lock and cause InvalidArrayIndex.
   RefPtr<FileBlockCache> self = this;
-  mThread->Dispatch(NS_NewRunnableFunction("FileBlockCache::Flush", [self]() {
+  mBackgroundET->Dispatch(NS_NewRunnableFunction("FileBlockCache::Flush", [self]() {
     MutexAutoLock mon(self->mDataMutex);
     // Just discard pending changes, assume MediaCache won't read from
     // blocks it hasn't written to.
     self->mChangeIndexList.clear();
     self->mBlockChanges.Clear();
   }));
 }
 
@@ -154,70 +153,60 @@ FileBlockCache::FileBlockCache()
       mIsWriteScheduled(false),
       mIsReading(false) {}
 
 FileBlockCache::~FileBlockCache() { Close(); }
 
 void FileBlockCache::Close() {
   LOG("Close()");
 
-  nsCOMPtr<nsIThread> thread;
+  nsCOMPtr<nsISerialEventTarget> thread;
   {
     MutexAutoLock mon(mDataMutex);
-    if (!mThread) {
+    if (!mBackgroundET) {
       return;
     }
-    thread.swap(mThread);
+    thread.swap(mBackgroundET);
   }
 
   PRFileDesc* fd;
   {
     MutexAutoLock lock(mFileMutex);
     fd = mFD;
     mFD = nullptr;
   }
 
   // Let the thread close the FD, and then trigger its own shutdown.
-  // Note that mThread is now empty, so no other task will be posted there.
-  // Also mThread and mFD are empty and therefore can be reused immediately.
+  // Note that mBackgroundET is now empty, so no other task will be posted there.
+  // Also mBackgroundET and mFD are empty and therefore can be reused immediately.
   nsresult rv = thread->Dispatch(
       NS_NewRunnableFunction(
           "FileBlockCache::Close",
           [thread, fd] {
             if (fd) {
               CloseFD(fd);
             }
-            // We must shut down the thread in another
-            // runnable. This is called
-            // while we're shutting down the media cache, and
-            // nsIThread::Shutdown()
-            // can cause events to run before it completes,
-            // which could end up
-            // opening more streams, while the media cache is
-            // shutting down and
-            // releasing memory etc!
-            nsCOMPtr<nsIRunnable> event = new ShutdownThreadEvent(thread);
-            SystemGroup::Dispatch(TaskCategory::Other, event.forget());
+            // No need to shutdown background task queues.
           }),
-      NS_DISPATCH_NORMAL);
+      NS_DISPATCH_EVENT_MAY_BLOCK);
   NS_ENSURE_SUCCESS_VOID(rv);
 }
 
 template <typename Container, typename Value>
 bool ContainerContains(const Container& aContainer, const Value& value) {
   return std::find(aContainer.begin(), aContainer.end(), value) !=
          aContainer.end();
 }
 
 nsresult FileBlockCache::WriteBlock(uint32_t aBlockIndex,
                                     Span<const uint8_t> aData1,
                                     Span<const uint8_t> aData2) {
   MutexAutoLock mon(mDataMutex);
 
-  if (!mThread) {
+  if (!mBackgroundET) {
     return NS_ERROR_FAILURE;
   }
 
   // Check if we've already got a pending write scheduled for this block.
   mBlockChanges.EnsureLengthAtLeast(aBlockIndex + 1);
   bool blockAlreadyHadPendingChange = mBlockChanges[aBlockIndex] != nullptr;
   mBlockChanges[aBlockIndex] = new BlockChange(aData1, aData2);
 
@@ -236,31 +225,31 @@ nsresult FileBlockCache::WriteBlock(uint
 
   EnsureWriteScheduled();
 
   return NS_OK;
 }
 
 void FileBlockCache::EnsureWriteScheduled() {
   mDataMutex.AssertCurrentThreadOwns();
-  MOZ_ASSERT(mThread);
+  MOZ_ASSERT(mBackgroundET);
 
   if (mIsWriteScheduled || mIsReading) {
     return;
   }
   mIsWriteScheduled = true;
   if (!mInitialized) {
     // We're still waiting on a file descriptor. When it arrives,
     // the write will be scheduled.
     return;
   }
   nsCOMPtr<nsIRunnable> event = mozilla::NewRunnableMethod(
       "FileBlockCache::EnsureWriteScheduled -> PerformBlockIOs", this,
       &FileBlockCache::PerformBlockIOs);
-  mThread->Dispatch(event.forget(), NS_DISPATCH_NORMAL);
+  mBackgroundET->Dispatch(event.forget(), NS_DISPATCH_EVENT_MAY_BLOCK);
 }
 
 nsresult FileBlockCache::Seek(int64_t aOffset) {
   mFileMutex.AssertCurrentThreadOwns();
 
   if (mFDCurrentPos != aOffset) {
     MOZ_ASSERT(mFD);
     int64_t result = PR_Seek64(mFD, aOffset, PR_SEEK_SET);
@@ -321,24 +310,24 @@ nsresult FileBlockCache::MoveBlockInFile
   if (NS_FAILED(ReadFromFile(BlockIndexToOffset(aSourceBlockIndex), buf,
                              BLOCK_SIZE, bytesRead))) {
     return NS_ERROR_FAILURE;
   }
   return WriteBlockToFile(aDestBlockIndex, buf);
 }
 
 void FileBlockCache::PerformBlockIOs() {
-  MOZ_ASSERT(mThread->IsOnCurrentThread());
+  MOZ_ASSERT(mBackgroundET->IsOnCurrentThread());
   MutexAutoLock mon(mDataMutex);
   NS_ASSERTION(mIsWriteScheduled, "Should report write running or scheduled.");
 
-  LOG("Run() mFD=%p mThread=%p", mFD, mThread.get());
+  LOG("Run() mFD=%p mBackgroundET=%p", mFD, mBackgroundET.get());
 
   while (!mChangeIndexList.empty()) {
-    if (!mThread) {
+    if (!mBackgroundET) {
       // We've been closed, abort, discarding unwritten changes.
       mIsWriteScheduled = false;
       return;
     }
 
     if (mIsReading) {
       // We're trying to read; postpone all writes. (Reader will resume writes.)
       mIsWriteScheduled = false;
@@ -386,17 +375,17 @@ void FileBlockCache::PerformBlockIOs() {
 
   mIsWriteScheduled = false;
 }
 
 nsresult FileBlockCache::Read(int64_t aOffset, uint8_t* aData, int32_t aLength,
                               int32_t* aBytes) {
   MutexAutoLock mon(mDataMutex);
 
-  if (!mThread || (aOffset / BLOCK_SIZE) > INT32_MAX) {
+  if (!mBackgroundET || (aOffset / BLOCK_SIZE) > INT32_MAX) {
     return NS_ERROR_FAILURE;
   }
 
   mIsReading = true;
   auto exitRead = MakeScopeExit([&] {
     mIsReading = false;
     if (!mChangeIndexList.empty()) {
       // mReading has stopped or prevented pending writes, resume them.
@@ -457,17 +446,17 @@ nsresult FileBlockCache::Read(int64_t aO
   *aBytes = aLength - bytesToRead;
   return NS_OK;
 }
 
 nsresult FileBlockCache::MoveBlock(int32_t aSourceBlockIndex,
                                    int32_t aDestBlockIndex) {
   MutexAutoLock mon(mDataMutex);
 
-  if (!mThread) {
+  if (!mBackgroundET) {
     return NS_ERROR_FAILURE;
   }
 
   mBlockChanges.EnsureLengthAtLeast(
       std::max(aSourceBlockIndex, aDestBlockIndex) + 1);
 
   // The source block's contents may be the destination of another pending
   // move, which in turn can be the destination of another pending move,
--- a/dom/media/FileBlockCache.h
+++ b/dom/media/FileBlockCache.h
@@ -166,24 +166,23 @@ class FileBlockCache : public MediaBlock
   void EnsureWriteScheduled();
 
   // Array of block changes to made. If mBlockChanges[offset/BLOCK_SIZE] ==
   // nullptr, then the block has no pending changes to be written, but if
   // mBlockChanges[offset/BLOCK_SIZE] != nullptr, then either there's a block
   // cached in memory waiting to be written, or this block is the target of a
   // block move.
   nsTArray<RefPtr<BlockChange> > mBlockChanges;
-  // Thread upon which block writes and block moves are performed. This is
-  // created upon open, and shutdown (asynchronously) upon close (on the
-  // main thread).
-  nsCOMPtr<nsIThread> mThread;
+  // Event target upon which block writes and block moves are performed. This is
+  // created upon open, and dropped on close.
+  nsCOMPtr<nsISerialEventTarget> mBackgroundET;
   // Queue of pending block indexes that need to be written or moved.
   std::deque<int32_t> mChangeIndexList;
   // True if we've dispatched an event to commit all pending block changes
-  // to file on mThread.
+  // to file on mBackgroundET.
   bool mIsWriteScheduled;
   // True when a read is happening. Pending writes may be postponed, to give
   // higher priority to reads (which may be blocking the caller).
   bool mIsReading;
   // True if we've got a temporary file descriptor. Note: we don't use mFD
   // directly as that's synchronized via mFileMutex and we need to make
   // decisions about whether we can write while holding mDataMutex.
   bool mInitialized = false;