Bug 1627075 - Build Omnijar file list from startup cache r=froydnj

We would like to be able to defer opening the omnijar files until after startup
if the StartupCache has already been populated. Opening the omnijar files takes
a nontrivial time, at least on Windows, and almost everything in the omnijar
should be fairly compressible, and thus makes sense to live in the StartupCache.
See the last patch in this series for a little more discussion on numbers, but
tl;dr: we saw a 12% improvement in time to about:home being finished on reference
hardware with these changes together with the changes from the descendant patches.

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

modules/libjar/nsZipArchive.cpp

@@ -341,11 +341,19 @@ nsZipHandle::~nsZipHandle() {
 //---------------------------------------------
 //  nsZipArchive::OpenArchive
 //---------------------------------------------
-nsresult nsZipArchive::OpenArchive(nsZipHandle* aZipHandle, PRFileDesc* aFd) {
+nsresult nsZipArchive::OpenArchive(nsZipHandle* aZipHandle, PRFileDesc* aFd,
+                                   const uint8_t* aCachedCentral,
+                                   size_t aCachedCentralSize) {
   mFd = aZipHandle;
 
   //-- get table of contents for archive
-  nsresult rv = BuildFileList(aFd);
+  nsresult rv;
+  if (aCachedCentral) {
+    rv = BuildFileListFromBuffer(aCachedCentral,
+                                 aCachedCentral + aCachedCentralSize);
+  } else {
+    rv = BuildFileList(aFd);
+  }
   if (NS_SUCCEEDED(rv)) {
     if (aZipHandle->mFile && XRE_IsParentProcess()) {
       static char* env = PR_GetEnv("MOZ_JAR_LOG_FILE");
@@ -399,7 +407,9 @@ nsresult nsZipArchive::OpenArchive(nsZipHandle* aZipHandle, PRFileDesc* aFd) {
   return rv;
 }
 
-nsresult nsZipArchive::OpenArchive(nsIFile* aFile) {
+nsresult nsZipArchive::OpenArchive(nsIFile* aFile,
+                                   const uint8_t* aCachedCentral,
+                                   size_t aCachedCentralSize) {
   RefPtr<nsZipHandle> handle;
 #if defined(XP_WIN)
   mozilla::AutoFDClose fd;
@@ -410,9 +420,9 @@ nsresult nsZipArchive::OpenArchive(nsIFile* aFile) {
   if (NS_FAILED(rv)) return rv;
 
 #if defined(XP_WIN)
-  return OpenArchive(handle, fd.get());
+  return OpenArchive(handle, fd.get(), aCachedCentral, aCachedCentralSize);
 #else
-  return OpenArchive(handle);
+  return OpenArchive(handle, nullptr, aCachedCentral, aCachedCentralSize);
 #endif
 }
 
@@ -653,6 +663,8 @@ nsresult nsZipArchive::BuildFileList(PRFileDesc* aFd) {
   const uint8_t* buf;
   const uint8_t* startp = mFd->mFileData;
   const uint8_t* endp = startp + mFd->mLen;
+
+  nsresult rv;
   MMAP_FAULT_HANDLER_BEGIN_HANDLE(mFd)
   uint32_t centralOffset = 4;
   // Only perform readahead in the parent process. Children processes
@@ -677,20 +689,62 @@ nsresult nsZipArchive::BuildFileList(PRFileDesc* aFd) {
     return NS_ERROR_FILE_CORRUPTED;
   }
 
-  buf = startp + centralOffset;
-
-  // avoid overflow of startp + centralOffset.
-  if (buf < startp) {
+  uintptr_t startpInt = (uintptr_t)startp;
+  if (startpInt + centralOffset < startpInt || centralOffset > mFd->mLen) {
     return NS_ERROR_FILE_CORRUPTED;
   }
 
+  buf = startp + centralOffset;
+
+  mZipCentralOffset = centralOffset;
+  rv = BuildFileListFromBuffer(buf, endp);
+
+  MMAP_FAULT_HANDLER_CATCH(NS_ERROR_FAILURE)
+  return rv;
+}
+
+UniquePtr<uint8_t[]> nsZipArchive::CopyCentralDirectoryBuffer(size_t* aSize) {
+  *aSize = 0;
+
+  // mZipCentralOffset could in theory be 0. In practice though, we likely
+  // won't ever see this. If the end result is that we can't cache the buffer
+  // in these cases, that's fine.
+  if (!mZipCentralOffset || !mZipCentralSize) {
+    return nullptr;
+  }
+
+  const uint8_t* buf;
+  const uint8_t* startp = mFd->mFileData;
+  buf = startp + mZipCentralOffset;
+
+  // Just a sanity check to make sure these values haven't overflowed the
+  // buffer mapped to our file. Technically the pointer could overflow the max
+  // pointer value, but that could only happen with this check succeeding if
+  // mFd->mLen is incorrect, which we will here assume is impossible.
+  if (mZipCentralOffset + mZipCentralSize > mFd->mLen) {
+    return nullptr;
+  }
+
+  auto resultBuf = MakeUnique<uint8_t[]>(mZipCentralSize);
+
+  MMAP_FAULT_HANDLER_BEGIN_HANDLE(mFd)
+  memcpy(resultBuf.get(), buf, mZipCentralSize);
+  MMAP_FAULT_HANDLER_CATCH(nullptr)
+
+  *aSize = mZipCentralSize;
+  return resultBuf;
+}
+
+nsresult nsZipArchive::BuildFileListFromBuffer(const uint8_t* aBuf,
+                                               const uint8_t* aEnd) {
+  const uint8_t* buf = aBuf;
   //-- Read the central directory headers
   uint32_t sig = 0;
   while ((buf + int32_t(sizeof(uint32_t)) > buf) &&
-         (buf + int32_t(sizeof(uint32_t)) <= endp) &&
+         (buf + int32_t(sizeof(uint32_t)) <= aEnd) &&
          ((sig = xtolong(buf)) == CENTRALSIG)) {
     // Make sure there is enough data available.
-    if ((buf > endp) || (endp - buf < ZIPCENTRAL_SIZE)) {
+    if ((buf > aEnd) || (aEnd - buf < ZIPCENTRAL_SIZE)) {
       return NS_ERROR_FILE_CORRUPTED;
     }
 
@@ -708,7 +762,7 @@ nsresult nsZipArchive::BuildFileList(PRFileDesc* aFd) {
       return NS_ERROR_FILE_CORRUPTED;
     }
     if (buf >= buf + diff ||  // No overflow
-        buf >= endp - diff) {
+        buf >= aEnd - diff) {
       return NS_ERROR_FILE_CORRUPTED;
     }
 
@@ -735,18 +789,18 @@ nsresult nsZipArchive::BuildFileList(PRFileDesc* aFd) {
   }
 
   // Make the comment available for consumers.
-  if ((endp >= buf) && (endp - buf >= ZIPEND_SIZE)) {
+  if ((aEnd >= buf) && (aEnd - buf >= ZIPEND_SIZE)) {
     ZipEnd* zipend = (ZipEnd*)buf;
 
     buf += ZIPEND_SIZE;
     uint16_t commentlen = xtoint(zipend->commentfield_len);
-    if (endp - buf >= commentlen) {
-      mCommentPtr = (const char*)buf;
+    if (aEnd - buf >= commentlen) {
+      mCommentPtr = (const char*)aBuf;
       mCommentLen = commentlen;
     }
   }
 
-  MMAP_FAULT_HANDLER_CATCH(NS_ERROR_FAILURE)
+  mZipCentralSize = buf - aBuf;
   return NS_OK;
 }
 
@@ -889,6 +943,8 @@ int64_t nsZipArchive::SizeOfMapping() { return mFd ? mFd->SizeOfMapping() : 0; }
 nsZipArchive::nsZipArchive()
     : mRefCnt(0),
       mCommentPtr(nullptr),
+      mZipCentralOffset(0),
+      mZipCentralSize(0),
       mCommentLen(0),
       mBuiltSynthetics(false),
       mUseZipLog(false) {

modules/libjar/nsZipArchive.h

@@ -97,11 +97,17 @@ class nsZipArchive final {
    * object. If we were allowed to use exceptions this would have been
    * part of the constructor
    *
-   * @param   aZipHandle  The nsZipHandle used to access the zip
-   * @param   aFd         Optional PRFileDesc for Windows readahead optimization
+   * @param   aZipHandle      The nsZipHandle used to access the zip
+   * @param   aFd             Optional PRFileDesc for Windows readahead
+                              optimization
+   * @param   aCachedCentral  Optional cached buffer containing the zip central
+                              for this zip.
+   * @param   aCachedCentralSize  Optional size of aCachedCentral.
    * @return  status code
    */
-  nsresult OpenArchive(nsZipHandle* aZipHandle, PRFileDesc* aFd = nullptr);
+  nsresult OpenArchive(nsZipHandle* aZipHandle, PRFileDesc* aFd = nullptr,
+                       const uint8_t* aCachedCentral = nullptr,
+                       size_t aCachedCentralSize = 0);
 
   /**
    * OpenArchive
@@ -109,9 +115,13 @@ class nsZipArchive final {
    * Convenience function that generates nsZipHandle
    *
    * @param   aFile         The file used to access the zip
+   * @param   aCachedCentral  Optional cached buffer containing the zip central
+                              for this zip.
+   * @param   aCachedCentralSize  Optional size of aCachedCentral.
    * @return  status code
    */
-  nsresult OpenArchive(nsIFile* aFile);
+  nsresult OpenArchive(nsIFile* aFile, const uint8_t* aCachedCentral = nullptr,
+                       size_t aCachedCentralSize = 0);
 
   /**
    * Test the integrity of items in this archive by running
@@ -181,6 +191,17 @@ class nsZipArchive final {
    */
   const uint8_t* GetData(nsZipItem* aItem);
 
+  /**
+   * Copies the contents of the zip central directory, and returns it to the
+   * caller to take ownership. This is useful for caching the contents of the
+   * central directory, which can be compressed and stored elsewhere, and
+   * passed back into OpenArchive when this archive is opened in the future.
+   *
+   * @param   aSize       size_t pointer to be filled with the size of the
+                          returned buffer.
+   */
+  mozilla::UniquePtr<uint8_t[]> CopyCentralDirectoryBuffer(size_t* aSize);
+
   bool GetComment(nsACString& aComment);
 
   /**
@@ -204,6 +225,8 @@ class nsZipArchive final {
   mozilla::ArenaAllocator<1024, sizeof(void*)> mArena;
 
   const char* mCommentPtr;
+  size_t mZipCentralOffset;
+  size_t mZipCentralSize;
   uint16_t mCommentLen;
 
   // Whether we synthesized the directory entries
@@ -223,6 +246,7 @@ class nsZipArchive final {
   //--- private methods ---
   nsZipItem* CreateZipItem();
   nsresult BuildFileList(PRFileDesc* aFd = nullptr);
+  nsresult BuildFileListFromBuffer(const uint8_t* aBuf, const uint8_t* aEnd);
   nsresult BuildSynthetics();
 
   nsZipArchive& operator=(const nsZipArchive& rhs) = delete;

startupcache/StartupCache.cpp

@@ -439,7 +439,7 @@ nsresult StartupCache::GetBuffer(const char* id, const char** outbuf,
   return NS_OK;
 }
 
-// Makes a copy of the buffer, client retains ownership of inbuf.
+// Takes ownership of the input buffer
 nsresult StartupCache::PutBuffer(const char* id, UniquePtr<char[]>&& inbuf,
                                  uint32_t len) {
   NS_ASSERTION(NS_IsMainThread(),

xpcom/build/Omnijar.cpp

@@ -11,6 +11,7 @@
 #include "nsIFile.h"
 #include "nsZipArchive.h"
 #include "nsNetUtil.h"
+#include "mozilla/scache/StartupCache.h"
 
 namespace mozilla {
 
@@ -21,6 +22,7 @@ bool Omnijar::sInitialized = false;
 bool Omnijar::sIsUnified = false;
 
 static const char* sProp[2] = {NS_GRE_DIR, NS_XPCOM_CURRENT_PROCESS_DIR};
+static const char* sCachePrefixes[2] = {"GreOmnijar:", "AppOmnijar:"};
 
 #define SPROP(Type) ((Type == mozilla::Omnijar::GRE) ? sProp[GRE] : sProp[APP])
 
@@ -78,10 +80,36 @@ void Omnijar::InitOne(nsIFile* aPath, Type aType) {
   }
 
   RefPtr<nsZipArchive> zipReader = new nsZipArchive();
-  if (NS_FAILED(zipReader->OpenArchive(file))) {
+  auto* cache = scache::StartupCache::GetSingleton();
+  const uint8_t* centralBuf = nullptr;
+  uint32_t centralBufLength = 0;
+  nsPrintfCString startupCacheKey("::%s:OmnijarCentral", sCachePrefixes[aType]);
+  if (cache) {
+    nsresult rv = cache->GetBuffer(startupCacheKey.get(),
+                                   reinterpret_cast<const char**>(&centralBuf),
+                                   &centralBufLength);
+    if (NS_FAILED(rv)) {
+      centralBuf = nullptr;
+      centralBufLength = 0;
+    }
+  }
+  if (NS_FAILED(zipReader->OpenArchive(file, centralBuf, centralBufLength))) {
     return;
   }
 
+  if (cache && !centralBuf) {
+    size_t bufSize;
+
+    // Annoyingly, nsZipArchive and the startupcache use different types to
+    // represent bytes (uint8_t vs char), so we have to do a little dance to
+    // convert the UniquePtr over.
+    UniquePtr<char[]> centralBuf(reinterpret_cast<char*>(
+        zipReader->CopyCentralDirectoryBuffer(&bufSize).release()));
+    if (centralBuf) {
+      cache->PutBuffer(startupCacheKey.get(), std::move(centralBuf), bufSize);
+    }
+  }
+
   RefPtr<nsZipArchive> outerReader;
   RefPtr<nsZipHandle> handle;
   if (NS_SUCCEEDED(nsZipHandle::Init(zipReader, MOZ_STRINGIFY(OMNIJAR_NAME),