Bug 1325341 - Query the cache index to get if the entry has alt data and its fileSize r=michal draft
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 03 Apr 2017 14:41:40 +0200
changeset 554999 272e4d8c0bd73a9ee9f1a8b5a08f7eec59b8ab39
parent 554998 9bc10faca1e29e2221b179819ad359e6cfbd346e
child 622492 bf4aca837d1ad73930d7c38a71590acc8a6ea87f
push id52110
push uservalentin.gosu@gmail.com
push dateMon, 03 Apr 2017 12:42:26 +0000
reviewersmichal
bugs1325341
milestone55.0a1
Bug 1325341 - Query the cache index to get if the entry has alt data and its fileSize r=michal MozReview-Commit-ID: 6wqsexqJq3u
modules/libpref/init/all.js
netwerk/cache2/CacheFileContextEvictor.cpp
netwerk/cache2/CacheIndex.cpp
netwerk/cache2/CacheIndex.h
netwerk/cache2/CacheStorage.cpp
netwerk/cache2/CacheStorage.h
netwerk/cache2/CacheStorageService.cpp
netwerk/cache2/CacheStorageService.h
netwerk/cache2/nsICacheStorage.idl
netwerk/protocol/http/nsHttpChannel.cpp
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1623,16 +1623,19 @@ pref("network.http.keep_empty_response_h
 
 // Max size, in bytes, for received HTTP response header.
 pref("network.http.max_response_header_size", 393216);
 
 // If we should attempt to race the cache and network
 pref("network.http.rcwn.enabled", false);
 pref("network.http.rcwn.cache_queue_normal_threshold", 50);
 pref("network.http.rcwn.cache_queue_priority_threshold", 10);
+// We might attempt to race the cache with the network only if a resource
+// is smaller than this size.
+pref("network.http.rcwn.small_resource_size_kb", 256);
 
 // default values for FTP
 // in a DSCP environment this should be 40 (0x28, or AF11), per RFC-4594,
 // Section 4.8 "High-Throughput Data Service Class", and 80 (0x50, or AF22)
 // per Section 4.7 "Low-Latency Data Service Class".
 pref("network.ftp.data.qos", 0);
 pref("network.ftp.control.qos", 0);
 
--- a/netwerk/cache2/CacheFileContextEvictor.cpp
+++ b/netwerk/cache2/CacheFileContextEvictor.cpp
@@ -608,18 +608,21 @@ CacheFileContextEvictor::EvictEntries()
       // We doom any active handle in CacheFileIOManager::EvictByContext(), so
       // this must be a new one. Skip it.
       LOG(("CacheFileContextEvictor::EvictEntries() - Skipping entry since we "
            "found an active handle. [handle=%p]", handle.get()));
       continue;
     }
 
     CacheIndex::EntryStatus status;
-    bool pinned;
-    rv = CacheIndex::HasEntry(hash, &status, &pinned);
+    bool pinned = false;
+    auto callback = [&pinned](const CacheIndexEntry * aEntry) {
+      pinned = aEntry->IsPinned();
+    };
+    rv = CacheIndex::HasEntry(hash, &status, callback);
     // This must never fail, since eviction (this code) happens only when the index
     // is up-to-date and thus the informatin is known.
     MOZ_ASSERT(NS_SUCCEEDED(rv));
 
     if (pinned != mEntries[0]->mPinned) {
       LOG(("CacheFileContextEvictor::EvictEntries() - Skipping entry since pinning "
            "doesn't match [evicting pinned=%d, entry pinned=%d]",
            mEntries[0]->mPinned, pinned));
--- a/netwerk/cache2/CacheIndex.cpp
+++ b/netwerk/cache2/CacheIndex.cpp
@@ -1109,48 +1109,46 @@ CacheIndex::RemoveAll()
     file->Remove(false);
   }
 
   return NS_OK;
 }
 
 // static
 nsresult
-CacheIndex::HasEntry(const nsACString &aKey, EntryStatus *_retval, bool *_pinned)
+CacheIndex::HasEntry(const nsACString &aKey, EntryStatus *_retval,
+                     const std::function<void(const CacheIndexEntry*)> &aCB)
 {
   LOG(("CacheIndex::HasEntry() [key=%s]", PromiseFlatCString(aKey).get()));
 
   SHA1Sum sum;
   SHA1Sum::Hash hash;
   sum.update(aKey.BeginReading(), aKey.Length());
   sum.finish(hash);
 
-  return HasEntry(hash, _retval, _pinned);
+  return HasEntry(hash, _retval, aCB);
 }
 
 // static
 nsresult
-CacheIndex::HasEntry(const SHA1Sum::Hash &hash, EntryStatus *_retval, bool *_pinned)
+CacheIndex::HasEntry(const SHA1Sum::Hash &hash, EntryStatus *_retval,
+                     const std::function<void(const CacheIndexEntry*)> &aCB)
 {
   StaticMutexAutoLock lock(sLock);
 
   RefPtr<CacheIndex> index = gInstance;
 
   if (!index) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   if (!index->IsIndexUsable()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  if (_pinned) {
-    *_pinned = false;
-  }
-
   const CacheIndexEntry *entry = nullptr;
 
   switch (index->mState) {
     case READING:
     case WRITING:
       entry = index->mPendingUpdates.GetEntry(hash);
       MOZ_FALLTHROUGH;
     case BUILDING:
@@ -1175,18 +1173,18 @@ CacheIndex::HasEntry(const SHA1Sum::Hash
     if (entry->IsRemoved()) {
       if (entry->IsFresh()) {
         *_retval = DOES_NOT_EXIST;
       } else {
         *_retval = DO_NOT_KNOW;
       }
     } else {
       *_retval = EXISTS;
-      if (_pinned && entry->IsPinned()) {
-        *_pinned = true;
+      if (aCB) {
+        aCB(entry);
       }
     }
   }
 
   LOG(("CacheIndex::HasEntry() - result is %u", *_retval));
   return NS_OK;
 }
 
--- a/netwerk/cache2/CacheIndex.h
+++ b/netwerk/cache2/CacheIndex.h
@@ -666,21 +666,22 @@ public:
   enum EntryStatus {
     EXISTS         = 0,
     DOES_NOT_EXIST = 1,
     DO_NOT_KNOW    = 2
   };
 
   // Returns status of the entry in index for the given key. It can be called
   // on any thread.
-  // If _pinned is non-null, it's filled with pinning status of the entry.
+  // If the optional aCB callback is given, the it will be called with a
+  // CacheIndexEntry only if _retval is EXISTS when the method returns.
   static nsresult HasEntry(const nsACString &aKey, EntryStatus *_retval,
-                           bool *_pinned = nullptr);
+                           const std::function<void(const CacheIndexEntry*)> &aCB = nullptr);
   static nsresult HasEntry(const SHA1Sum::Hash &hash, EntryStatus *_retval,
-                           bool *_pinned = nullptr);
+                           const std::function<void(const CacheIndexEntry*)> &aCB = nullptr);
 
   // Returns a hash of the least important entry that should be evicted if the
   // cache size is over limit and also returns a total number of all entries in
   // the index minus the number of forced valid entries and unpinned entries
   // that we encounter when searching (see below)
   static nsresult GetEntryForEviction(bool aIgnoreEmptyEntries, SHA1Sum::Hash *aHash, uint32_t *aCnt);
 
   // Checks if a cache entry is currently forced valid. Used to prevent an entry
--- a/netwerk/cache2/CacheStorage.cpp
+++ b/netwerk/cache2/CacheStorage.cpp
@@ -166,16 +166,43 @@ NS_IMETHODIMP CacheStorage::Exists(nsIUR
   nsAutoCString asciiSpec;
   rv = noRefURI->GetAsciiSpec(asciiSpec);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return CacheStorageService::Self()->CheckStorageEntry(
     this, asciiSpec, aIdExtension, aResult);
 }
 
+nsresult
+CacheStorage::GetCacheIndexEntryAttrs(nsIURI *aURI,
+                                      const nsACString &aIdExtension,
+                                      bool *aHasAltData,
+                                      uint32_t *aSizeInKB)
+{
+  NS_ENSURE_ARG(aURI);
+  NS_ENSURE_ARG(aHasAltData);
+  NS_ENSURE_ARG(aSizeInKB);
+  if (!CacheStorageService::Self()) {
+    return NS_ERROR_NOT_INITIALIZED;
+  }
+
+  nsresult rv;
+
+  nsCOMPtr<nsIURI> noRefURI;
+  rv = aURI->CloneIgnoringRef(getter_AddRefs(noRefURI));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsAutoCString asciiSpec;
+  rv = noRefURI->GetAsciiSpec(asciiSpec);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return CacheStorageService::Self()->GetCacheIndexEntryAttrs(
+    this, asciiSpec, aIdExtension, aHasAltData, aSizeInKB);
+}
+
 NS_IMETHODIMP CacheStorage::AsyncDoomURI(nsIURI *aURI, const nsACString & aIdExtension,
                                          nsICacheEntryDoomCallback* aCallback)
 {
   if (!CacheStorageService::Self())
     return NS_ERROR_NOT_INITIALIZED;
 
   nsresult rv;
 
--- a/netwerk/cache2/CacheStorage.h
+++ b/netwerk/cache2/CacheStorage.h
@@ -68,14 +68,17 @@ protected:
   bool mPinning : 1;
 
 public:
   nsILoadContextInfo* LoadInfo() const { return mLoadContextInfo; }
   bool WriteToDisk() const { return mWriteToDisk && !mLoadContextInfo->IsPrivate(); }
   bool LookupAppCache() const { return mLookupAppCache; }
   bool SkipSizeCheck() const { return mSkipSizeCheck; }
   bool Pinning() const { return mPinning; }
+  virtual nsresult GetCacheIndexEntryAttrs(
+          nsIURI *aURI, const nsACString &aIdExtension,
+          bool *aHasAltData, uint32_t *aSizeInKB) override;
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif
--- a/netwerk/cache2/CacheStorageService.cpp
+++ b/netwerk/cache2/CacheStorageService.cpp
@@ -1630,16 +1630,58 @@ CacheStorageService::CheckStorageEntry(C
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   *aResult = status == CacheIndex::EXISTS;
   LOG(("  %sfound in index", *aResult ? "" : "not "));
   return NS_OK;
 }
 
+nsresult
+CacheStorageService::GetCacheIndexEntryAttrs(CacheStorage const* aStorage,
+                                             const nsACString &aURI,
+                                             const nsACString &aIdExtension,
+                                             bool *aHasAltData,
+                                             uint32_t *aFileSizeKb)
+{
+  nsresult rv;
+
+  nsAutoCString contextKey;
+  CacheFileUtils::AppendKeyPrefix(aStorage->LoadInfo(), contextKey);
+
+  LOG(("CacheStorageService::GetCacheIndexEntryAttrs [uri=%s, eid=%s, contextKey=%s]",
+    aURI.BeginReading(), aIdExtension.BeginReading(), contextKey.get()));
+
+  nsAutoCString fileKey;
+  rv = CacheEntry::HashingKey(contextKey, aIdExtension, aURI, fileKey);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  *aHasAltData = false;
+  *aFileSizeKb = 0;
+  auto closure = [&aHasAltData, &aFileSizeKb](const CacheIndexEntry *entry) {
+    *aHasAltData = entry->GetHasAltData();
+    *aFileSizeKb = entry->GetFileSize();
+  };
+
+  CacheIndex::EntryStatus status;
+  rv = CacheIndex::HasEntry(fileKey, &status, closure);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  if (status != CacheIndex::EXISTS) {
+    return NS_ERROR_CACHE_KEY_NOT_FOUND;
+  }
+
+  return NS_OK;
+}
+
+
 namespace {
 
 class CacheEntryDoomByKeyCallback : public CacheFileIOListener
                                   , public nsIRunnable
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIRUNNABLE
--- a/netwerk/cache2/CacheStorageService.h
+++ b/netwerk/cache2/CacheStorageService.h
@@ -106,16 +106,22 @@ public:
                              int64_t aDataSize, int32_t aFetchCount,
                              uint32_t aLastModifiedTime, uint32_t aExpirationTime,
                              bool aPinned, nsILoadContextInfo* aInfo) = 0;
   };
 
   // Invokes OnEntryInfo for the given aEntry, synchronously.
   static void GetCacheEntryInfo(CacheEntry* aEntry, EntryInfoCallback *aVisitor);
 
+  nsresult GetCacheIndexEntryAttrs(CacheStorage const* aStorage,
+                                   const nsACString &aURI,
+                                   const nsACString &aIdExtension,
+                                   bool *aHasAltData,
+                                   uint32_t *aFileSizeKb);
+
   static uint32_t CacheQueueSize(bool highPriority);
 
   // Memory reporting
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
   MOZ_DEFINE_MALLOC_SIZE_OF(MallocSizeOf)
 
 private:
--- a/netwerk/cache2/nsICacheStorage.idl
+++ b/netwerk/cache2/nsICacheStorage.idl
@@ -126,9 +126,17 @@ interface nsICacheStorage : nsISupports
   void asyncEvictStorage(in nsICacheEntryDoomCallback aCallback);
 
   /**
    * Visits the storage and its entries.
    * NOTE: Disk storage also visits memory storage.
    */
   void asyncVisitStorage(in nsICacheStorageVisitor aVisitor,
                          in boolean aVisitEntries);
+
+  %{C++
+    virtual nsresult GetCacheIndexEntryAttrs(
+          nsIURI *aURI, const nsACString &aIdExtension,
+          bool *aHasAltData, uint32_t *aSizeInKB) {
+      return NS_ERROR_NOT_IMPLEMENTED;
+    }
+  %}
 };
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -117,16 +117,17 @@ namespace mozilla { namespace net {
 namespace {
 
 // Monotonically increasing ID for generating unique cache entries per
 // intercepted channel.
 static uint64_t gNumIntercepted = 0;
 static bool sRCWNEnabled = false;
 static uint32_t sRCWNQueueSizeNormal = 50;
 static uint32_t sRCWNQueueSizePriority = 10;
+static uint32_t sRCWNSmallResourceSizeKB = 256;
 
 // True if the local cache should be bypassed when processing a request.
 #define BYPASS_LOCAL_CACHE(loadFlags) \
         (loadFlags & (nsIRequest::LOAD_BYPASS_CACHE | \
                       nsICachingChannel::LOAD_BYPASS_LOCAL_CACHE))
 
 #define RECOVER_FROM_CACHE_FILE_ERROR(result) \
         ((result) == NS_ERROR_FILE_NOT_FOUND || \
@@ -3606,17 +3607,26 @@ nsHttpChannel::OpenCacheEntry(bool isHtt
             DebugOnly<bool> exists;
             MOZ_ASSERT(NS_SUCCEEDED(cacheStorage->Exists(openURI, extension, &exists)) && exists,
                        "The entry must exist in the cache after we create it here");
         }
 
         mCacheOpenWithPriority = cacheEntryOpenFlags & nsICacheStorage::OPEN_PRIORITY;
         mCacheQueueSizeWhenOpen = CacheStorageService::CacheQueueSize(mCacheOpenWithPriority);
 
-        if (sRCWNEnabled && mInterceptCache != INTERCEPTED) {
+        bool hasAltData = false;
+        uint32_t sizeInKb = 0;
+        rv = cacheStorage->GetCacheIndexEntryAttrs(openURI, extension,
+                                                   &hasAltData, &sizeInKb);
+
+        // We will attempt to race the network vs the cache if we've found this
+        // entry in the cache index, and it has appropriate
+        // attributes (doesn't have alt-data, and has a small size)
+        if (sRCWNEnabled && mInterceptCache != INTERCEPTED &&
+            NS_SUCCEEDED(rv) && !hasAltData && sizeInKb < sRCWNSmallResourceSizeKB) {
             MaybeRaceNetworkWithCache();
         }
 
         if (!mCacheOpenDelay) {
             rv = cacheStorage->AsyncOpenURI(openURI, extension, cacheEntryOpenFlags, this);
         } else {
             // We pass `this` explicitly as a parameter due to the raw pointer
             // to refcounted object in lambda analysis.
@@ -5816,16 +5826,17 @@ nsHttpChannel::AsyncOpen(nsIStreamListen
     }
 
     static bool sRCWNInited = false;
     if (!sRCWNInited) {
         sRCWNInited = true;
         Preferences::AddBoolVarCache(&sRCWNEnabled, "network.http.rcwn.enabled");
         Preferences::AddUintVarCache(&sRCWNQueueSizeNormal, "network.http.rcwn.cache_queue_normal_threshold");
         Preferences::AddUintVarCache(&sRCWNQueueSizePriority, "network.http.rcwn.cache_queue_priority_threshold");
+        Preferences::AddUintVarCache(&sRCWNSmallResourceSizeKB, "network.http.rcwn.small_resource_size_kb");
     }
 
     rv = NS_CheckPortSafety(mURI);
     if (NS_FAILED(rv)) {
         ReleaseListeners();
         return rv;
     }