Bug 1173756 - Part 3: Merge File and ChildRunnable; r=luke
authorJan Varga <jan.varga@gmail.com>
Sat, 29 Aug 2015 07:45:24 +0200
changeset 259965 1f1422d84e9dc9d24c0257709dfcd28d2d74c933
parent 259964 23d5ef182400250b989411323106f9ae61832381
child 259966 ae64d16898395607093db2e2dd667b6544cb1f55
push id29296
push userryanvm@gmail.com
push dateSun, 30 Aug 2015 19:45:10 +0000
treeherdermozilla-central@2ad5077d86ba [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1173756
milestone43.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 1173756 - Part 3: Merge File and ChildRunnable; r=luke
dom/asmjscache/AsmJSCache.cpp
--- a/dom/asmjscache/AsmJSCache.cpp
+++ b/dom/asmjscache/AsmJSCache.cpp
@@ -232,19 +232,17 @@ EvictEntries(nsIFile* aDirectory, const 
     }
 
     entry.clear();
   }
 }
 
 // FileDescriptorHolder owns a file descriptor and its memory mapping.
 // FileDescriptorHolder is derived by two runnable classes (that is,
-// (Parent|Child)Runnable. FileDescriptorHolder is derived by File and
-// ParentRunnable. Since File and ParentRunnable both need to be runnables
-// FileDescriptorHolder also derives nsRunnable.
+// (Parent|Child)Runnable.
 class FileDescriptorHolder : public nsRunnable
 {
 public:
   FileDescriptorHolder()
   : mQuotaObject(nullptr),
     mFileSize(INT64_MIN),
     mFileDesc(nullptr),
     mFileMap(nullptr),
@@ -326,163 +324,16 @@ protected:
 
   nsRefPtr<QuotaObject> mQuotaObject;
   int64_t mFileSize;
   PRFileDesc* mFileDesc;
   PRFileMap* mFileMap;
   void* mMappedMemory;
 };
 
-// File is a base class derived by ChildRunnable that presents a single
-// interface to the AsmJSCache ops which need to wait until the file is open.
-class File : public FileDescriptorHolder
-{
-public:
-  class AutoClose
-  {
-    File* mFile;
-
-  public:
-    explicit AutoClose(File* aFile = nullptr)
-    : mFile(aFile)
-    { }
-
-    void
-    Init(File* aFile)
-    {
-      MOZ_ASSERT(!mFile);
-      mFile = aFile;
-    }
-
-    File*
-    operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN
-    {
-      MOZ_ASSERT(mFile);
-      return mFile;
-    }
-
-    void
-    Forget(File** aFile)
-    {
-      *aFile = mFile;
-      mFile = nullptr;
-    }
-
-    ~AutoClose()
-    {
-      if (mFile) {
-        mFile->Close();
-      }
-    }
-  };
-
-  JS::AsmJSCacheResult
-  BlockUntilOpen(AutoClose* aCloser)
-  {
-    MOZ_ASSERT(!mWaiting, "Can only call BlockUntilOpen once");
-    MOZ_ASSERT(!mOpened, "Can only call BlockUntilOpen once");
-
-    mWaiting = true;
-
-    nsresult rv = NS_DispatchToMainThread(this);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return JS::AsmJSCache_InternalError;
-    }
-
-    {
-      MutexAutoLock lock(mMutex);
-      while (mWaiting) {
-        mCondVar.Wait();
-      }
-    }
-
-    if (!mOpened) {
-      return mResult;
-    }
-
-    // Now that we're open, we're guarnateed a Close() call. However, we are
-    // not guarnateed someone is holding an outstanding reference until the File
-    // is closed, so we do that ourselves and Release() in OnClose().
-    aCloser->Init(this);
-    AddRef();
-    return JS::AsmJSCache_Success;
-  }
-
-  // This method must be called if BlockUntilOpen returns 'true'. AutoClose
-  // mostly takes care of this. A derived class that implements Close() must
-  // guarnatee that OnClose() is called (eventually).
-  virtual void
-  Close() = 0;
-
-protected:
-  File()
-  : mMutex("File::mMutex"),
-    mCondVar(mMutex, "File::mCondVar"),
-    mWaiting(false),
-    mOpened(false),
-    mResult(JS::AsmJSCache_InternalError)
-  { }
-
-  ~File()
-  {
-    MOZ_ASSERT(!mWaiting, "Shouldn't be destroyed while thread is waiting");
-    MOZ_ASSERT(!mOpened, "OnClose() should have been called");
-  }
-
-  void
-  OnOpen()
-  {
-    Notify(JS::AsmJSCache_Success);
-  }
-
-  void
-  OnFailure(JS::AsmJSCacheResult aResult)
-  {
-    MOZ_ASSERT(aResult != JS::AsmJSCache_Success);
-
-    FileDescriptorHolder::Finish();
-    Notify(aResult);
-  }
-
-  void
-  OnClose()
-  {
-    FileDescriptorHolder::Finish();
-
-    MOZ_ASSERT(mOpened);
-    mOpened = false;
-
-    // Match the AddRef in BlockUntilOpen(). The main thread event loop still
-    // holds an outstanding ref which will keep 'this' alive until returning to
-    // the event loop.
-    Release();
-  }
-
-private:
-  void
-  Notify(JS::AsmJSCacheResult aResult)
-  {
-    MOZ_ASSERT(NS_IsMainThread());
-
-    MutexAutoLock lock(mMutex);
-    MOZ_ASSERT(mWaiting);
-
-    mWaiting = false;
-    mOpened = aResult == JS::AsmJSCache_Success;
-    mResult = aResult;
-    mCondVar.Notify();
-  }
-
-  Mutex mMutex;
-  CondVar mCondVar;
-  bool mWaiting;
-  bool mOpened;
-  JS::AsmJSCacheResult mResult;
-};
-
 class UnlockDirectoryRunnable final
   : public nsRunnable
 {
   nsRefPtr<DirectoryLock> mDirectoryLock;
 
 public:
   explicit
   UnlockDirectoryRunnable(already_AddRefed<DirectoryLock> aDirectoryLock)
@@ -1308,52 +1159,131 @@ DeallocEntryParent(PAsmJSCacheEntryParen
 {
   // Transfer ownership back from IPDL.
   nsRefPtr<ParentRunnable> op =
     dont_AddRef(static_cast<ParentRunnable*>(aActor));
 }
 
 namespace {
 
+// A runnable that presents a single interface to the AsmJSCache ops which need
+// to wait until the file is open.
 class ChildRunnable final
-  : public File
+  : public FileDescriptorHolder
   , public PAsmJSCacheEntryChild
   , public nsIIPCBackgroundChildCreateCallback
 {
   typedef mozilla::ipc::PBackgroundChild PBackgroundChild;
 
 public:
+  class AutoClose
+  {
+    ChildRunnable* mChildRunnable;
+
+  public:
+    explicit AutoClose(ChildRunnable* aChildRunnable = nullptr)
+    : mChildRunnable(aChildRunnable)
+    { }
+
+    void
+    Init(ChildRunnable* aChildRunnable)
+    {
+      MOZ_ASSERT(!mChildRunnable);
+      mChildRunnable = aChildRunnable;
+    }
+
+    ChildRunnable*
+    operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN
+    {
+      MOZ_ASSERT(mChildRunnable);
+      return mChildRunnable;
+    }
+
+    void
+    Forget(ChildRunnable** aChildRunnable)
+    {
+      *aChildRunnable = mChildRunnable;
+      mChildRunnable = nullptr;
+    }
+
+    ~AutoClose()
+    {
+      if (mChildRunnable) {
+        mChildRunnable->Close();
+      }
+    }
+  };
+
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_NSIRUNNABLE
   NS_DECL_NSIIPCBACKGROUNDCHILDCREATECALLBACK
 
   ChildRunnable(nsIPrincipal* aPrincipal,
                 OpenMode aOpenMode,
                 WriteParams aWriteParams,
                 ReadParams aReadParams)
   : mPrincipal(aPrincipal),
-    mOpenMode(aOpenMode),
     mWriteParams(aWriteParams),
     mReadParams(aReadParams),
+    mMutex("ChildRunnable::mMutex"),
+    mCondVar(mMutex, "ChildRunnable::mCondVar"),
+    mOpenMode(aOpenMode),
+    mState(eInitial),
+    mResult(JS::AsmJSCache_InternalError),
     mActorDestroyed(false),
-    mState(eInitial)
+    mWaiting(false),
+    mOpened(false)
   {
     MOZ_ASSERT(!NS_IsMainThread());
     MOZ_COUNT_CTOR(ChildRunnable);
   }
 
-protected:
+  JS::AsmJSCacheResult
+  BlockUntilOpen(AutoClose* aCloser)
+  {
+    MOZ_ASSERT(!mWaiting, "Can only call BlockUntilOpen once");
+    MOZ_ASSERT(!mOpened, "Can only call BlockUntilOpen once");
+
+    mWaiting = true;
+
+    nsresult rv = NS_DispatchToMainThread(this);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return JS::AsmJSCache_InternalError;
+    }
+
+    {
+      MutexAutoLock lock(mMutex);
+      while (mWaiting) {
+        mCondVar.Wait();
+      }
+    }
+
+    if (!mOpened) {
+      return mResult;
+    }
+
+    // Now that we're open, we're guaranteed a Close() call. However, we are
+    // not guaranteed someone is holding an outstanding reference until the File
+    // is closed, so we do that ourselves and Release() in OnClose().
+    aCloser->Init(this);
+    AddRef();
+    return JS::AsmJSCache_Success;
+  }
+
+private:
   ~ChildRunnable()
   {
+    MOZ_ASSERT(!mWaiting, "Shouldn't be destroyed while thread is waiting");
+    MOZ_ASSERT(!mOpened);
     MOZ_ASSERT(mState == eFinished);
     MOZ_ASSERT(mActorDestroyed);
     MOZ_COUNT_DTOR(ChildRunnable);
   }
 
-private:
+  // IPDL methods.
   bool
   RecvOnOpenMetadataForRead(const Metadata& aMetadata) override
   {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(mState == eOpening);
 
     uint32_t moduleIndex;
     if (FindHashMatch(aMetadata, mReadParams, &moduleIndex)) {
@@ -1373,17 +1303,17 @@ private:
     mFileSize = aFileSize;
 
     mFileDesc = PR_ImportFile(PROsfd(aFileDesc.PlatformHandle()));
     if (!mFileDesc) {
       return false;
     }
 
     mState = eOpened;
-    File::OnOpen();
+    Notify(JS::AsmJSCache_Success);
     return true;
   }
 
   bool
   Recv__delete__(const JS::AsmJSCacheResult& aResult) override
   {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(mState == eOpening);
@@ -1395,51 +1325,74 @@ private:
   void
   ActorDestroy(ActorDestroyReason why) override
   {
     MOZ_ASSERT(NS_IsMainThread());
     mActorDestroyed = true;
   }
 
   void
-  Close() override final
+  Close()
   {
     MOZ_ASSERT(mState == eOpened);
 
     mState = eClosing;
     NS_DispatchToMainThread(this);
   }
 
-private:
   void
   Fail(JS::AsmJSCacheResult aResult)
   {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(mState == eInitial || mState == eOpening);
+    MOZ_ASSERT(aResult != JS::AsmJSCache_Success);
 
     mState = eFinished;
-    File::OnFailure(aResult);
+
+    FileDescriptorHolder::Finish();
+    Notify(aResult);
+  }
+
+  void
+  Notify(JS::AsmJSCacheResult aResult)
+  {
+    MOZ_ASSERT(NS_IsMainThread());
+
+    MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(mWaiting);
+
+    mWaiting = false;
+    mOpened = aResult == JS::AsmJSCache_Success;
+    mResult = aResult;
+    mCondVar.Notify();
   }
 
   nsIPrincipal* const mPrincipal;
   nsAutoPtr<PrincipalInfo> mPrincipalInfo;
-  const OpenMode mOpenMode;
   WriteParams mWriteParams;
   ReadParams mReadParams;
-  bool mActorDestroyed;
+  Mutex mMutex;
+  CondVar mCondVar;
 
+  // Couple enums and bools together
+  const OpenMode mOpenMode;
   enum State {
     eInitial, // Just created, waiting to be dispatched to the main thread
     eBackgroundChildPending, // Waiting for the background child to be created
     eOpening, // Waiting for the parent process to respond
     eOpened, // Parent process opened the entry and sent it back
     eClosing, // Waiting to be dispatched to the main thread to Send__delete__
     eFinished // Terminal state
   };
   State mState;
+  JS::AsmJSCacheResult mResult;
+
+  bool mActorDestroyed;
+  bool mWaiting;
+  bool mOpened;
 };
 
 NS_IMETHODIMP
 ChildRunnable::Run()
 {
   switch (mState) {
     case eInitial: {
       MOZ_ASSERT(NS_IsMainThread());
@@ -1482,17 +1435,25 @@ ChildRunnable::Run()
     }
 
     case eClosing: {
       MOZ_ASSERT(NS_IsMainThread());
 
       // Per FileDescriptorHolder::Finish()'s comment, call before
       // releasing the directory lock (which happens in the parent upon receipt
       // of the Send__delete__ message).
-      File::OnClose();
+      FileDescriptorHolder::Finish();
+
+      MOZ_ASSERT(mOpened);
+      mOpened = false;
+
+      // Match the AddRef in BlockUntilOpen(). The main thread event loop still
+      // holds an outstanding ref which will keep 'this' alive until returning to
+      // the event loop.
+      Release();
 
       if (!mActorDestroyed) {
         unused << Send__delete__(this, JS::AsmJSCache_Success);
       }
 
       mState = eFinished;
       return NS_OK;
     }
@@ -1554,17 +1515,17 @@ DeallocEntryChild(PAsmJSCacheEntryChild*
 
 namespace {
 
 JS::AsmJSCacheResult
 OpenFile(nsIPrincipal* aPrincipal,
          OpenMode aOpenMode,
          WriteParams aWriteParams,
          ReadParams aReadParams,
-         File::AutoClose* aFile)
+         ChildRunnable::AutoClose* aChildRunnable)
 {
   MOZ_ASSERT_IF(aOpenMode == eOpenForRead, aWriteParams.mSize == 0);
   MOZ_ASSERT_IF(aOpenMode == eOpenForWrite, aReadParams.mBegin == nullptr);
 
   // There are three reasons we don't attempt caching from the main thread:
   //  1. In the parent process: QuotaManager::WaitForOpenAllowed prevents
   //     synchronous waiting on the main thread requiring a runnable to be
   //     dispatched to the main thread.
@@ -1578,25 +1539,26 @@ OpenFile(nsIPrincipal* aPrincipal,
   // semantically observable.
   if (NS_IsMainThread()) {
     return JS::AsmJSCache_SynchronousScript;
   }
 
   // We need to synchronously call into the parent to open the file and
   // interact with the QuotaManager. The child can then map the file into its
   // address space to perform I/O.
-  nsRefPtr<File> file =
+  nsRefPtr<ChildRunnable> childRunnable =
     new ChildRunnable(aPrincipal, aOpenMode, aWriteParams, aReadParams);
 
-  JS::AsmJSCacheResult openResult = file->BlockUntilOpen(aFile);
+  JS::AsmJSCacheResult openResult =
+    childRunnable->BlockUntilOpen(aChildRunnable);
   if (openResult != JS::AsmJSCache_Success) {
     return openResult;
   }
 
-  if (!file->MapMemory(aOpenMode)) {
+  if (!childRunnable->MapMemory(aOpenMode)) {
     return JS::AsmJSCache_InternalError;
   }
 
   return JS::AsmJSCache_Success;
 }
 
 } // namespace
 
@@ -1604,78 +1566,80 @@ typedef uint32_t AsmJSCookieType;
 static const uint32_t sAsmJSCookie = 0x600d600d;
 
 bool
 OpenEntryForRead(nsIPrincipal* aPrincipal,
                  const char16_t* aBegin,
                  const char16_t* aLimit,
                  size_t* aSize,
                  const uint8_t** aMemory,
-                 intptr_t* aFile)
+                 intptr_t* aHandle)
 {
   if (size_t(aLimit - aBegin) < sMinCachedModuleLength) {
     return false;
   }
 
   ReadParams readParams;
   readParams.mBegin = aBegin;
   readParams.mLimit = aLimit;
 
-  File::AutoClose file;
+  ChildRunnable::AutoClose childRunnable;
   WriteParams notAWrite;
   JS::AsmJSCacheResult openResult =
-    OpenFile(aPrincipal, eOpenForRead, notAWrite, readParams, &file);
+    OpenFile(aPrincipal, eOpenForRead, notAWrite, readParams, &childRunnable);
   if (openResult != JS::AsmJSCache_Success) {
     return false;
   }
 
   // Although we trust that the stored cache files have not been arbitrarily
   // corrupted, it is possible that a previous execution aborted in the middle
   // of writing a cache file (crash, OOM-killer, etc). To protect against
   // partially-written cache files, we use the following scheme:
   //  - Allocate an extra word at the beginning of every cache file which
   //    starts out 0 (OpenFile opens with PR_TRUNCATE).
   //  - After the asm.js serialization is complete, PR_SyncMemMap to write
   //    everything to disk and then store a non-zero value (sAsmJSCookie)
   //    in the first word.
   //  - When attempting to read a cache file, check whether the first word is
   //    sAsmJSCookie.
-  if (file->FileSize() < sizeof(AsmJSCookieType) ||
-      *(AsmJSCookieType*)file->MappedMemory() != sAsmJSCookie) {
+  if (childRunnable->FileSize() < sizeof(AsmJSCookieType) ||
+      *(AsmJSCookieType*)childRunnable->MappedMemory() != sAsmJSCookie) {
     return false;
   }
 
-  *aSize = file->FileSize() - sizeof(AsmJSCookieType);
-  *aMemory = (uint8_t*) file->MappedMemory() + sizeof(AsmJSCookieType);
+  *aSize = childRunnable->FileSize() - sizeof(AsmJSCookieType);
+  *aMemory = (uint8_t*) childRunnable->MappedMemory() + sizeof(AsmJSCookieType);
 
   // The caller guarnatees a call to CloseEntryForRead (on success or
   // failure) at which point the file will be closed.
-  file.Forget(reinterpret_cast<File**>(aFile));
+  childRunnable.Forget(reinterpret_cast<ChildRunnable**>(aHandle));
   return true;
 }
 
 void
 CloseEntryForRead(size_t aSize,
                   const uint8_t* aMemory,
-                  intptr_t aFile)
+                  intptr_t aHandle)
 {
-  File::AutoClose file(reinterpret_cast<File*>(aFile));
+  ChildRunnable::AutoClose childRunnable(
+    reinterpret_cast<ChildRunnable*>(aHandle));
 
-  MOZ_ASSERT(aSize + sizeof(AsmJSCookieType) == file->FileSize());
-  MOZ_ASSERT(aMemory - sizeof(AsmJSCookieType) == file->MappedMemory());
+  MOZ_ASSERT(aSize + sizeof(AsmJSCookieType) == childRunnable->FileSize());
+  MOZ_ASSERT(aMemory - sizeof(AsmJSCookieType) ==
+               childRunnable->MappedMemory());
 }
 
 JS::AsmJSCacheResult
 OpenEntryForWrite(nsIPrincipal* aPrincipal,
                   bool aInstalled,
                   const char16_t* aBegin,
                   const char16_t* aEnd,
                   size_t aSize,
                   uint8_t** aMemory,
-                  intptr_t* aFile)
+                  intptr_t* aHandle)
 {
   if (size_t(aEnd - aBegin) < sMinCachedModuleLength) {
     return JS::AsmJSCache_ModuleTooSmall;
   }
 
   // Add extra space for the AsmJSCookieType (see OpenEntryForRead).
   aSize += sizeof(AsmJSCookieType);
 
@@ -1683,50 +1647,52 @@ OpenEntryForWrite(nsIPrincipal* aPrincip
 
   WriteParams writeParams;
   writeParams.mInstalled = aInstalled;
   writeParams.mSize = aSize;
   writeParams.mFastHash = HashString(aBegin, sNumFastHashChars);
   writeParams.mNumChars = aEnd - aBegin;
   writeParams.mFullHash = HashString(aBegin, writeParams.mNumChars);
 
-  File::AutoClose file;
+  ChildRunnable::AutoClose childRunnable;
   ReadParams notARead;
   JS::AsmJSCacheResult openResult =
-    OpenFile(aPrincipal, eOpenForWrite, writeParams, notARead, &file);
+    OpenFile(aPrincipal, eOpenForWrite, writeParams, notARead, &childRunnable);
   if (openResult != JS::AsmJSCache_Success) {
     return openResult;
   }
 
   // Strip off the AsmJSCookieType from the buffer returned to the caller,
   // which expects a buffer of aSize, not a buffer of sizeWithCookie starting
   // with a cookie.
-  *aMemory = (uint8_t*) file->MappedMemory() + sizeof(AsmJSCookieType);
+  *aMemory = (uint8_t*) childRunnable->MappedMemory() + sizeof(AsmJSCookieType);
 
   // The caller guarnatees a call to CloseEntryForWrite (on success or
   // failure) at which point the file will be closed
-  file.Forget(reinterpret_cast<File**>(aFile));
+  childRunnable.Forget(reinterpret_cast<ChildRunnable**>(aHandle));
   return JS::AsmJSCache_Success;
 }
 
 void
 CloseEntryForWrite(size_t aSize,
                    uint8_t* aMemory,
-                   intptr_t aFile)
+                   intptr_t aHandle)
 {
-  File::AutoClose file(reinterpret_cast<File*>(aFile));
+  ChildRunnable::AutoClose childRunnable(
+    reinterpret_cast<ChildRunnable*>(aHandle));
 
-  MOZ_ASSERT(aSize + sizeof(AsmJSCookieType) == file->FileSize());
-  MOZ_ASSERT(aMemory - sizeof(AsmJSCookieType) == file->MappedMemory());
+  MOZ_ASSERT(aSize + sizeof(AsmJSCookieType) == childRunnable->FileSize());
+  MOZ_ASSERT(aMemory - sizeof(AsmJSCookieType) ==
+               childRunnable->MappedMemory());
 
   // Flush to disk before writing the cookie (see OpenEntryForRead).
-  if (PR_SyncMemMap(file->FileDesc(),
-                    file->MappedMemory(),
-                    file->FileSize()) == PR_SUCCESS) {
-    *(AsmJSCookieType*)file->MappedMemory() = sAsmJSCookie;
+  if (PR_SyncMemMap(childRunnable->FileDesc(),
+                    childRunnable->MappedMemory(),
+                    childRunnable->FileSize()) == PR_SUCCESS) {
+    *(AsmJSCookieType*)childRunnable->MappedMemory() = sAsmJSCookie;
   }
 }
 
 bool
 GetBuildId(JS::BuildIdCharVector* aBuildID)
 {
   nsCOMPtr<nsIXULAppInfo> info = do_GetService("@mozilla.org/xre/app-info;1");
   if (!info) {