Bug 1529780 - Compute ImageCacheKey's hash number lazily; r=tnikkel
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 22 Feb 2019 14:25:04 +0000
changeset 460581 045d790ca417fc6e827b57eef8ec5aea2beef81a
parent 460580 67b131d1aac58cabaea7ce66abebb8bfee142bc9
child 460582 b171bfdb308f94aea2193530ef670f2c5a350a84
push id35596
push userrmaries@mozilla.com
push dateSat, 23 Feb 2019 04:13:22 +0000
treeherdermozilla-central@fdd04819e350 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1529780
milestone67.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 1529780 - Compute ImageCacheKey's hash number lazily; r=tnikkel Differential Revision: https://phabricator.services.mozilla.com/D20746
image/ImageCacheKey.cpp
image/ImageCacheKey.h
image/imgLoader.cpp
--- a/image/ImageCacheKey.cpp
+++ b/image/ImageCacheKey.cpp
@@ -2,16 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ImageCacheKey.h"
 
 #include "mozilla/HashFunctions.h"
 #include "mozilla/Move.h"
+#include "mozilla/Unused.h"
 #include "nsContentUtils.h"
 #include "nsICookieService.h"
 #include "nsLayoutUtils.h"
 #include "nsString.h"
 #include "mozilla/AntiTrackingCommon.h"
 #include "mozilla/dom/BlobURLProtocolHandler.h"
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/ServiceWorkerManager.h"
@@ -33,47 +34,26 @@ static Maybe<uint64_t> BlobSerial(nsIURI
       blob) {
     return Some(blob->GetSerialNumber());
   }
 
   return Nothing();
 }
 
 ImageCacheKey::ImageCacheKey(nsIURI* aURI, const OriginAttributes& aAttrs,
-                             Document* aDocument, nsresult& aRv)
+                             Document* aDocument)
     : mURI(aURI),
       mOriginAttributes(aAttrs),
       mControlledDocument(GetSpecialCaseDocumentToken(aDocument, aURI)),
-      mHash(0),
       mIsChrome(false) {
   if (SchemeIs("blob")) {
     mBlobSerial = BlobSerial(mURI);
   } else if (SchemeIs("chrome")) {
     mIsChrome = true;
   }
-
-  // Since we frequently call Hash() several times in a row on the same
-  // ImageCacheKey, as an optimization we compute our hash once and store it.
-
-  nsPrintfCString ptr("%p", mControlledDocument);
-  nsAutoCString suffix;
-  mOriginAttributes.CreateSuffix(suffix);
-
-  if (mBlobSerial) {
-    aRv = mURI->GetRef(mBlobRef);
-    NS_ENSURE_SUCCESS_VOID(aRv);
-    mHash = HashGeneric(*mBlobSerial, HashString(mBlobRef));
-  } else {
-    nsAutoCString spec;
-    aRv = mURI->GetSpec(spec);
-    NS_ENSURE_SUCCESS_VOID(aRv);
-    mHash = HashString(spec);
-  }
-
-  mHash = AddToHash(mHash, HashString(suffix), HashString(ptr));
 }
 
 ImageCacheKey::ImageCacheKey(const ImageCacheKey& aOther)
     : mURI(aOther.mURI),
       mBlobSerial(aOther.mBlobSerial),
       mBlobRef(aOther.mBlobRef),
       mOriginAttributes(aOther.mOriginAttributes),
       mControlledDocument(aOther.mControlledDocument),
@@ -95,27 +75,67 @@ bool ImageCacheKey::operator==(const Ima
   if (mControlledDocument != aOther.mControlledDocument) {
     return false;
   }
   // The origin attributes always have to match.
   if (mOriginAttributes != aOther.mOriginAttributes) {
     return false;
   }
   if (mBlobSerial || aOther.mBlobSerial) {
+    if (mBlobSerial && mBlobRef.IsEmpty()) {
+      EnsureBlobRef();
+    }
+    if (aOther.mBlobSerial && aOther.mBlobRef.IsEmpty()) {
+      aOther.EnsureBlobRef();
+    }
     // If at least one of us has a blob serial, just compare the blob serial and
     // the ref portion of the URIs.
     return mBlobSerial == aOther.mBlobSerial && mBlobRef == aOther.mBlobRef;
   }
 
   // For non-blob URIs, compare the URIs.
   bool equals = false;
   nsresult rv = mURI->Equals(aOther.mURI, &equals);
   return NS_SUCCEEDED(rv) && equals;
 }
 
+void ImageCacheKey::EnsureBlobRef() const {
+  MOZ_ASSERT(mBlobSerial);
+  MOZ_ASSERT(mBlobRef.IsEmpty());
+
+  nsresult rv = mURI->GetRef(mBlobRef);
+  NS_ENSURE_SUCCESS_VOID(rv);
+}
+
+void ImageCacheKey::EnsureHash() const {
+  MOZ_ASSERT(mHash.isNothing());
+  PLDHashNumber hash = 0;
+
+  // Since we frequently call Hash() several times in a row on the same
+  // ImageCacheKey, as an optimization we compute our hash once and store it.
+
+  nsPrintfCString ptr("%p", mControlledDocument);
+  nsAutoCString suffix;
+  mOriginAttributes.CreateSuffix(suffix);
+
+  if (mBlobSerial) {
+    if (mBlobRef.IsEmpty()) {
+      EnsureBlobRef();
+    }
+    hash = HashGeneric(*mBlobSerial, HashString(mBlobRef));
+  } else {
+    nsAutoCString spec;
+    Unused << mURI->GetSpec(spec);
+    hash = HashString(spec);
+  }
+
+  hash = AddToHash(hash, HashString(suffix), HashString(ptr));
+  mHash.emplace(hash);
+}
+
 bool ImageCacheKey::SchemeIs(const char* aScheme) {
   bool matches = false;
   return NS_SUCCEEDED(mURI->SchemeIs(aScheme, &matches)) && matches;
 }
 
 /* static */ void* ImageCacheKey::GetSpecialCaseDocumentToken(
     Document* aDocument, nsIURI* aURI) {
   // Cookie-averse documents can never have storage granted to them.  Since they
--- a/image/ImageCacheKey.h
+++ b/image/ImageCacheKey.h
@@ -26,23 +26,28 @@ namespace image {
  * We key the cache on the initial URI (before any redirects), with some
  * canonicalization applied. See ComputeHash() for the details.
  * Controlled documents do not share their cache entries with
  * non-controlled documents, or other controlled documents.
  */
 class ImageCacheKey final {
  public:
   ImageCacheKey(nsIURI* aURI, const OriginAttributes& aAttrs,
-                dom::Document* aDocument, nsresult& aRv);
+                dom::Document* aDocument);
 
   ImageCacheKey(const ImageCacheKey& aOther);
   ImageCacheKey(ImageCacheKey&& aOther);
 
   bool operator==(const ImageCacheKey& aOther) const;
-  PLDHashNumber Hash() const { return mHash; }
+  PLDHashNumber Hash() const {
+    if (MOZ_UNLIKELY(mHash.isNothing())) {
+      EnsureHash();
+    }
+    return mHash.value();
+  }
 
   /// A weak pointer to the URI.
   nsIURI* URI() const { return mURI; }
 
   const OriginAttributes& OriginAttributesRef() const {
     return mOriginAttributes;
   }
 
@@ -56,21 +61,24 @@ class ImageCacheKey final {
  private:
   bool SchemeIs(const char* aScheme);
 
   // For ServiceWorker and for anti-tracking we need to use the document as
   // token for the key. All those exceptions are handled by this method.
   static void* GetSpecialCaseDocumentToken(dom::Document* aDocument,
                                            nsIURI* aURI);
 
+  void EnsureHash() const;
+  void EnsureBlobRef() const;
+
   nsCOMPtr<nsIURI> mURI;
   Maybe<uint64_t> mBlobSerial;
-  nsCString mBlobRef;
+  mutable nsCString mBlobRef;
   OriginAttributes mOriginAttributes;
   void* mControlledDocument;
-  PLDHashNumber mHash;
+  mutable Maybe<PLDHashNumber> mHash;
   bool mIsChrome;
 };
 
 }  // namespace image
 }  // namespace mozilla
 
 #endif  // mozilla_image_src_ImageCacheKey_h
--- a/image/imgLoader.cpp
+++ b/image/imgLoader.cpp
@@ -1402,19 +1402,18 @@ imgLoader::RemoveEntry(nsIURI* aURI, Doc
     OriginAttributes attrs;
     if (aDoc) {
       nsCOMPtr<nsIPrincipal> principal = aDoc->NodePrincipal();
       if (principal) {
         attrs = principal->OriginAttributesRef();
       }
     }
 
-    nsresult rv = NS_OK;
-    ImageCacheKey key(aURI, attrs, aDoc, rv);
-    if (NS_SUCCEEDED(rv) && RemoveFromCache(key)) {
+    ImageCacheKey key(aURI, attrs, aDoc);
+    if (RemoveFromCache(key)) {
       return NS_OK;
     }
   }
   return NS_ERROR_NOT_AVAILABLE;
 }
 
 NS_IMETHODIMP
 imgLoader::FindEntryProperties(nsIURI* uri, Document* aDoc,
@@ -1424,19 +1423,17 @@ imgLoader::FindEntryProperties(nsIURI* u
   OriginAttributes attrs;
   if (aDoc) {
     nsCOMPtr<nsIPrincipal> principal = aDoc->NodePrincipal();
     if (principal) {
       attrs = principal->OriginAttributesRef();
     }
   }
 
-  nsresult rv;
-  ImageCacheKey key(uri, attrs, aDoc, rv);
-  NS_ENSURE_SUCCESS(rv, rv);
+  ImageCacheKey key(uri, attrs, aDoc);
   imgCacheTable& cache = GetCache(key);
 
   RefPtr<imgCacheEntry> entry;
   if (cache.Get(key, getter_AddRefs(entry)) && entry) {
     if (mCacheTracker && entry->HasNoProxies()) {
       mCacheTracker->MarkUsed(entry);
     }
 
@@ -2139,18 +2136,17 @@ nsresult imgLoader::LoadImage(
   // Look in the cache for our URI, and then validate it.
   // XXX For now ignore aCacheKey. We will need it in the future
   // for correctly dealing with image load requests that are a result
   // of post data.
   OriginAttributes attrs;
   if (aTriggeringPrincipal) {
     attrs = aTriggeringPrincipal->OriginAttributesRef();
   }
-  ImageCacheKey key(aURI, attrs, aLoadingDocument, rv);
-  NS_ENSURE_SUCCESS(rv, rv);
+  ImageCacheKey key(aURI, attrs, aLoadingDocument);
   imgCacheTable& cache = GetCache(key);
 
   if (cache.Get(key, getter_AddRefs(entry)) && entry) {
     bool newChannelCreated = false;
     if (ValidateEntry(
             entry, aURI, aInitialDocumentURI, aReferrerURI, aReferrerPolicy,
             aLoadGroup, aObserver, ToSupports(aLoadingDocument),
             aLoadingDocument, requestFlags, aContentPolicyType, true,
@@ -2369,19 +2365,17 @@ nsresult imgLoader::LoadImageWithChannel
   channel->GetURI(getter_AddRefs(uri));
   nsCOMPtr<Document> doc = do_QueryInterface(aCX);
 
   NS_ENSURE_TRUE(channel, NS_ERROR_FAILURE);
   nsCOMPtr<nsILoadInfo> loadInfo = channel->LoadInfo();
 
   OriginAttributes attrs = loadInfo->GetOriginAttributes();
 
-  nsresult rv;
-  ImageCacheKey key(uri, attrs, doc, rv);
-  NS_ENSURE_SUCCESS(rv, rv);
+  ImageCacheKey key(uri, attrs, doc);
 
   nsLoadFlags requestFlags = nsIRequest::LOAD_NORMAL;
   channel->GetLoadFlags(&requestFlags);
 
   // If we are trying to load a thumbnail via the moz-page-thumb:// protocol,
   // load it directly from the cache to prevent re-decoding the image. See Bug
   // 1373258
   // TODO: Bug 1406134
@@ -2470,17 +2464,17 @@ nsresult imgLoader::LoadImageWithChannel
     nsCOMPtr<nsILoadGroup> docLoadGroup = doc->GetDocumentLoadGroup();
     MOZ_ASSERT(docLoadGroup == loadGroup);
   }
 #endif
 
   // Filter out any load flags not from nsIRequest
   requestFlags &= nsIRequest::LOAD_REQUESTMASK;
 
-  rv = NS_OK;
+  nsresult rv = NS_OK;
   if (request) {
     // we have this in our cache already.. cancel the current (document) load
 
     // this should fire an OnStopRequest
     channel->Cancel(NS_ERROR_PARSED_DATA_CACHED);
 
     *listener = nullptr;  // give them back a null nsIStreamListener
 
@@ -2491,18 +2485,17 @@ nsresult imgLoader::LoadImageWithChannel
     // We use originalURI here to fulfil the imgIRequest contract on GetURI.
     nsCOMPtr<nsIURI> originalURI;
     channel->GetOriginalURI(getter_AddRefs(originalURI));
 
     // XXX(seth): We should be able to just use |key| here, except that |key| is
     // constructed above with the *current URI* and not the *original URI*. I'm
     // pretty sure this is a bug, and it's preventing us from ever getting a
     // cache hit in LoadImageWithChannel when redirects are involved.
-    ImageCacheKey originalURIKey(originalURI, attrs, doc, rv);
-    NS_ENSURE_SUCCESS(rv, rv);
+    ImageCacheKey originalURIKey(originalURI, attrs, doc);
 
     // Default to doing a principal check because we don't know who
     // started that load and whether their principal ended up being
     // inherited on the channel.
     NewRequestAndEntry(/* aForcePrincipalCheckForCacheEntry = */ true, this,
                        originalURIKey, getter_AddRefs(request),
                        getter_AddRefs(entry));