Bug 1529780 - Compute ImageCacheKey's hash number lazily; r=tnikkel

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Ehsan Akhgari 2019-02-22 14:25:04 +00:00
parent 106797bead
commit 1737eb4583
3 changed files with 61 additions and 40 deletions

View File

@ -7,6 +7,7 @@
#include "mozilla/HashFunctions.h"
#include "mozilla/Move.h"
#include "mozilla/Unused.h"
#include "nsContentUtils.h"
#include "nsICookieService.h"
#include "nsLayoutUtils.h"
@ -38,37 +39,16 @@ static Maybe<uint64_t> BlobSerial(nsIURI* aURI) {
}
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)
@ -100,6 +80,12 @@ bool ImageCacheKey::operator==(const ImageCacheKey& aOther) const {
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;
@ -111,6 +97,40 @@ bool ImageCacheKey::operator==(const ImageCacheKey& aOther) const {
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;

View File

@ -31,13 +31,18 @@ namespace image {
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; }
@ -61,12 +66,15 @@ class ImageCacheKey final {
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;
};

View File

@ -1407,9 +1407,8 @@ imgLoader::RemoveEntry(nsIURI* aURI, Document* aDoc) {
}
}
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;
}
}
@ -1429,9 +1428,7 @@ imgLoader::FindEntryProperties(nsIURI* uri, Document* aDoc,
}
}
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;
@ -2144,8 +2141,7 @@ nsresult imgLoader::LoadImage(
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) {
@ -2374,9 +2370,7 @@ nsresult imgLoader::LoadImageWithChannel(nsIChannel* channel,
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);
@ -2475,7 +2469,7 @@ nsresult imgLoader::LoadImageWithChannel(nsIChannel* channel,
// 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
@ -2496,8 +2490,7 @@ nsresult imgLoader::LoadImageWithChannel(nsIChannel* channel,
// 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