Bug 938186 - introduce FORCE_ASYNC_CALLBACK for opening cache entries, r=michal
authorHonza Bambas <honzab.moz@firemni.cz>
Thu, 05 Jun 2014 18:27:38 +0200
changeset 206076 940d041073c7a9bb2b1ec6bff600ada76ac94444
parent 206075 927b20a21b19a6309697803c2ebf70b4fc8ccf2b
child 206077 d405928cb93469b7559d9599a719dae9ba670586
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmichal
bugs938186
milestone32.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 938186 - introduce FORCE_ASYNC_CALLBACK for opening cache entries, r=michal
netwerk/cache2/CacheEntry.cpp
netwerk/cache2/CacheEntry.h
netwerk/cache2/nsICacheStorage.idl
netwerk/test/unit/test_cache2-01f-basic-async.js
netwerk/test/unit/test_cache2-24-force-async.js
netwerk/test/unit/xpcshell.ini
--- a/netwerk/cache2/CacheEntry.cpp
+++ b/netwerk/cache2/CacheEntry.cpp
@@ -71,24 +71,25 @@ CacheEntryHandle::~CacheEntryHandle()
 
   MOZ_COUNT_DTOR(CacheEntryHandle);
 }
 
 // CacheEntry::Callback
 
 CacheEntry::Callback::Callback(CacheEntry* aEntry,
                                nsICacheEntryOpenCallback *aCallback,
-                               bool aReadOnly, bool aCheckOnAnyThread)
+                               bool aReadOnly, bool aCheckOnAnyThread, bool aForceAsync)
 : mEntry(aEntry)
 , mCallback(aCallback)
 , mTargetThread(do_GetCurrentThread())
 , mReadOnly(aReadOnly)
 , mCheckOnAnyThread(aCheckOnAnyThread)
 , mRecheckAfterWrite(false)
 , mNotWanted(false)
+, mForceAsync(aForceAsync)
 {
   MOZ_COUNT_CTOR(CacheEntry::Callback);
 
   // The counter may go from zero to non-null only under the service lock
   // but here we expect it to be already positive.
   MOZ_ASSERT(mEntry->HandlesCount());
   mEntry->AddHandleRef();
 }
@@ -96,16 +97,17 @@ CacheEntry::Callback::Callback(CacheEntr
 CacheEntry::Callback::Callback(CacheEntry::Callback const &aThat)
 : mEntry(aThat.mEntry)
 , mCallback(aThat.mCallback)
 , mTargetThread(aThat.mTargetThread)
 , mReadOnly(aThat.mReadOnly)
 , mCheckOnAnyThread(aThat.mCheckOnAnyThread)
 , mRecheckAfterWrite(aThat.mRecheckAfterWrite)
 , mNotWanted(aThat.mNotWanted)
+, mForceAsync(aThat.mForceAsync)
 {
   MOZ_COUNT_CTOR(CacheEntry::Callback);
 
   // The counter may go from zero to non-null only under the service lock
   // but here we expect it to be already positive.
   MOZ_ASSERT(mEntry->HandlesCount());
   mEntry->AddHandleRef();
 }
@@ -126,30 +128,58 @@ void CacheEntry::Callback::ExchangeEntry
   // The counter may go from zero to non-null only under the service lock
   // but here we expect it to be already positive.
   MOZ_ASSERT(aEntry->HandlesCount());
   aEntry->AddHandleRef();
   mEntry->ReleaseHandleRef();
   mEntry = aEntry;
 }
 
-nsresult CacheEntry::Callback::OnCheckThread(bool *aOnCheckThread) const
+bool CacheEntry::Callback::ForceAsync()
 {
+  if (!mForceAsync) {
+    return false;
+  }
+
+  // Drop the flag now.  First time we must claim we are not on the proper thread
+  // what will simply force a post.  But, the post does the check again and that
+  // time we must already tell the true we are on the proper thread otherwise we
+  // just loop indefinitely.  Also, we need to post only once the first callback
+  // of OnCheck or OnAvail.  OnAvail after OnCheck can already go directly.
+  // Note on thread safety: when called from OnCheckThread we are definitely under
+  // the lock, when called from OnAvailThread we don't anymore need to be under
+  // the lock since all concurrency risks are over by that time.
+  mForceAsync = false;
+  return true;
+}
+
+nsresult CacheEntry::Callback::OnCheckThread(bool *aOnCheckThread)
+{
+  if (ForceAsync()) {
+    *aOnCheckThread = false;
+    return NS_OK;
+  }
+
   if (!mCheckOnAnyThread) {
     // Check we are on the target
     return mTargetThread->IsOnCurrentThread(aOnCheckThread);
   }
 
   // We can invoke check anywhere
   *aOnCheckThread = true;
   return NS_OK;
 }
 
-nsresult CacheEntry::Callback::OnAvailThread(bool *aOnAvailThread) const
+nsresult CacheEntry::Callback::OnAvailThread(bool *aOnAvailThread)
 {
+  if (ForceAsync()) {
+    *aOnAvailThread = false;
+    return NS_OK;
+  }
+
   return mTargetThread->IsOnCurrentThread(aOnAvailThread);
 }
 
 // CacheEntry
 
 NS_IMPL_ISUPPORTS(CacheEntry,
                   nsICacheEntry,
                   nsIRunnable,
@@ -263,21 +293,22 @@ void CacheEntry::AsyncOpen(nsICacheEntry
   LOG(("CacheEntry::AsyncOpen [this=%p, state=%s, flags=%d, callback=%p]",
     this, StateString(mState), aFlags, aCallback));
 
   bool readonly = aFlags & nsICacheStorage::OPEN_READONLY;
   bool bypassIfBusy = aFlags & nsICacheStorage::OPEN_BYPASS_IF_BUSY;
   bool truncate = aFlags & nsICacheStorage::OPEN_TRUNCATE;
   bool priority = aFlags & nsICacheStorage::OPEN_PRIORITY;
   bool multithread = aFlags & nsICacheStorage::CHECK_MULTITHREADED;
+  bool async = aFlags & nsICacheStorage::FORCE_ASYNC_CALLBACK;
 
   MOZ_ASSERT(!readonly || !truncate, "Bad flags combination");
   MOZ_ASSERT(!(truncate && mState > LOADING), "Must not call truncate on already loaded entry");
 
-  Callback callback(this, aCallback, readonly, multithread);
+  Callback callback(this, aCallback, readonly, multithread, async);
 
   mozilla::MutexAutoLock lock(mLock);
 
   RememberCallback(callback, bypassIfBusy);
 
   // Load() opens the lock
   if (Load(truncate, priority)) {
     // Loading is in progress...
@@ -693,17 +724,17 @@ bool CacheEntry::InvokeCallback(Callback
 
     mozilla::MutexAutoUnlock unlock(mLock);
     InvokeAvailableCallback(aCallback);
   }
 
   return true;
 }
 
-void CacheEntry::InvokeAvailableCallback(Callback const & aCallback)
+void CacheEntry::InvokeAvailableCallback(Callback & aCallback)
 {
   LOG(("CacheEntry::InvokeAvailableCallback [this=%p, state=%s, cb=%p, r/o=%d, n/w=%d]",
     this, StateString(mState), aCallback.mCallback.get(), aCallback.mReadOnly, aCallback.mNotWanted));
 
   nsresult rv;
 
   uint32_t const state = mState;
 
--- a/netwerk/cache2/CacheEntry.h
+++ b/netwerk/cache2/CacheEntry.h
@@ -130,17 +130,17 @@ private:
   // We must monitor when a cache entry whose consumer is responsible
   // for writing it the first time gets released.  We must then invoke
   // waiting callbacks to not break the chain.
   class Callback
   {
   public:
     Callback(CacheEntry* aEntry,
              nsICacheEntryOpenCallback *aCallback,
-             bool aReadOnly, bool aCheckOnAnyThread);
+             bool aReadOnly, bool aCheckOnAnyThread, bool aForceAsync);
     Callback(Callback const &aThat);
     ~Callback();
 
     // Called when this callback record changes it's owning entry,
     // mainly during recreation.
     void ExchangeEntry(CacheEntry* aEntry);
 
     // We are raising reference count here to take into account the pending
@@ -148,19 +148,21 @@ private:
     // it's pointer).
     nsRefPtr<CacheEntry> mEntry;
     nsCOMPtr<nsICacheEntryOpenCallback> mCallback;
     nsCOMPtr<nsIThread> mTargetThread;
     bool mReadOnly : 1;
     bool mCheckOnAnyThread : 1;
     bool mRecheckAfterWrite : 1;
     bool mNotWanted : 1;
+    bool mForceAsync : 1;
 
-    nsresult OnCheckThread(bool *aOnCheckThread) const;
-    nsresult OnAvailThread(bool *aOnAvailThread) const;
+    bool ForceAsync();
+    nsresult OnCheckThread(bool *aOnCheckThread);
+    nsresult OnAvailThread(bool *aOnAvailThread);
   };
 
   // Since OnCacheEntryAvailable must be invoked on the main thread
   // we need a runnable for it...
   class AvailableCallbackRunnable : public nsRunnable
   {
   public:
     AvailableCallbackRunnable(CacheEntry* aEntry,
@@ -210,17 +212,17 @@ private:
   bool Load(bool aTruncate, bool aPriority);
   void OnLoaded();
 
   void RememberCallback(Callback & aCallback, bool aBypassIfBusy);
   void InvokeCallbacksLock();
   void InvokeCallbacks();
   bool InvokeCallbacks(bool aReadOnly);
   bool InvokeCallback(Callback & aCallback);
-  void InvokeAvailableCallback(Callback const & aCallback);
+  void InvokeAvailableCallback(Callback & aCallback);
 
   nsresult OpenOutputStreamInternal(int64_t offset, nsIOutputStream * *_retval);
 
   // When this entry is new and recreated w/o a callback, we need to wrap it
   // with a handle to detect writing consumer is gone.
   CacheEntryHandle* NewWriteHandle();
   void OnHandleClosed(CacheEntryHandle const* aHandle);
 
--- a/netwerk/cache2/nsICacheStorage.idl
+++ b/netwerk/cache2/nsICacheStorage.idl
@@ -46,16 +46,23 @@ interface nsICacheStorage : nsISupports
    * Perform the cache entry check (onCacheEntryCheck invocation) on any thread 
    * for optimal perfomance optimization.  If this flag is not specified it is
    * ensured that onCacheEntryCheck is called on the same thread as respective 
    * asyncOpen has been called.
    */
   const uint32_t CHECK_MULTITHREADED = 1 << 4;
 
   /**
+   * Forces the callback to be invoked always only asynchronously regardless
+   * we have all the information to invoke it directly from inside asyncOpenURI
+   * method.
+   */
+  const uint32_t FORCE_ASYNC_CALLBACK = 1 << 5;
+
+  /**
    * Asynchronously opens a cache entry for the specified URI.
    * Result is fetched asynchronously via the callback.
    *
    * @param aURI
    *    The URI to search in cache or to open for writting.
    * @param aIdExtension
    *    Any string that will extend (distinguish) the entry.  Two entries
    *    with the same aURI but different aIdExtension will be comletely
@@ -64,16 +71,17 @@ interface nsICacheStorage : nsISupports
    * @param aFlags
    *    OPEN_NORMALLY - open cache entry normally for read and write
    *    OPEN_TRUNCATE - delete any existing entry before opening it
    *    OPEN_READONLY - don't create an entry if there is none
    *    OPEN_PRIORITY - give this request a priority over others
    *    OPEN_BYPASS_IF_BUSY - backward compatibility only, LOAD_BYPASS_LOCAL_CACHE_IF_BUSY
    *    CHECK_MULTITHREADED - onCacheEntryCheck may be called on any thread, consumer 
    *                          implementation is thread-safe
+   *    FORCE_ASYNC_CALLBACK - always call the callback asynchronously
    * @param aCallback
    *    The consumer that receives the result.
    *    IMPORTANT: The callback may be called sooner the method returns.
    */
   void asyncOpenURI(in nsIURI aURI, in ACString aIdExtension,
                     in uint32_t aFlags,
                     in nsICacheEntryOpenCallback aCallback);
 
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit/test_cache2-01f-basic-async.js
@@ -0,0 +1,28 @@
+function run_test()
+{
+  do_get_profile();
+
+  // Open for write, write
+  asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+    new OpenCallback(NEW, "a1m", "a1d", function(entry) {
+      // Open for read and check
+      asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+        new OpenCallback(NORMAL, "a1m", "a1d", function(entry) {
+          // Open for rewrite (truncate), write different meta and data
+          asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_TRUNCATE | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+            new OpenCallback(NEW, "a2m", "a2d", function(entry) {
+              // Open for read and check
+              asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+                new OpenCallback(NORMAL, "a2m", "a2d", function(entry) {
+                  finish_cache2_test();
+                })
+              );
+            })
+          );
+        })
+      );
+    })
+  );
+
+  do_test_pending();
+}
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit/test_cache2-24-force-async.js
@@ -0,0 +1,42 @@
+function run_test()
+{
+  do_get_profile();
+
+  // Open for write, write, and wait for finishing it before notification to avoid concurrent write
+  // since we want to get as much as possible the scenario when an entry is left in the pool
+  // and a new consumer comes to open it later.
+  var outOfAsyncOpen0 = false;
+  asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+    new OpenCallback(NEW|WAITFORWRITE, "a1m", "a1d", function(entry) {
+      do_check_true(outOfAsyncOpen0);
+      // Open for read, expect callback happen from inside asyncOpenURI
+      var outOfAsyncOpen1 = false;
+      asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY, null,
+        function(entry) {
+          do_check_false(outOfAsyncOpen1);
+          var outOfAsyncOpen2 = false;
+          // Open for read, again should be sync
+          asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY, null,
+            function(entry) {
+              do_check_false(outOfAsyncOpen2);
+              var outOfAsyncOpen3 = false;
+              // Open for read, expect callback happen from outside of asyncOpenURI
+              asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+                function(entry) {
+                  do_check_true(outOfAsyncOpen3);
+                  finish_cache2_test();
+                }
+              );
+              outOfAsyncOpen3 = true;
+            }
+          );
+          outOfAsyncOpen2 = true;
+        }
+      );
+      outOfAsyncOpen1 = true;
+    })
+  );
+  outOfAsyncOpen0 = true;
+
+  do_test_pending();
+}
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -20,16 +20,17 @@ support-files =
 
 [test_nsIBufferedOutputStream_writeFrom_block.js]
 [test_cache2-01-basic.js]
 [test_cache2-01a-basic-readonly.js]
 [test_cache2-01b-basic-datasize.js]
 [test_cache2-01c-basic-hasmeta-only.js]
 [test_cache2-01d-basic-not-wanted.js]
 [test_cache2-01e-basic-bypass-if-busy.js]
+[test_cache2-01f-basic-async.js]
 [test_cache2-02-open-non-existing.js]
 [test_cache2-03-oncacheentryavail-throws.js]
 [test_cache2-04-oncacheentryavail-throws2x.js]
 [test_cache2-05-visit.js]
 [test_cache2-06-pb-mode.js]
 # Bug 675039, comment 6: "The difference is that the memory cache is disabled in Armv6 builds."
 skip-if = os == "android"
 [test_cache2-07-visit-memory.js]
@@ -52,16 +53,17 @@ skip-if = os == "android"
 [test_cache2-16-conditional-200.js]
 [test_cache2-17-evict-all.js]
 [test_cache2-18-not-valid.js]
 [test_cache2-19-range-206.js]
 [test_cache2-20-range-200.js]
 [test_cache2-21-anon-storage.js]
 [test_cache2-22-anon-visit.js]
 [test_cache2-23-read-over-chunk.js]
+[test_cache2-24-force-async.js]
 [test_304_responses.js]
 # Bug 675039: test hangs on Android-armv6 
 skip-if = os == "android"
 [test_cacheForOfflineUse_no-store.js]
 [test_307_redirect.js]
 [test_NetUtil.js]
 [test_URIs.js]
 [test_aboutblank.js]