Bug 956801 - save some locking in CacheEntry::BackgroundOp, r=michal
☠☠ backed out by 8c81fc70045c ☠ ☠
authorHonza Bambas <honzab.moz@firemni.cz>
Sat, 21 Jun 2014 21:41:01 +0200
changeset 189948 374f713017038d8f52624207c0552c02c6e0e500
parent 189947 53bfc7d490291a8d79b7a0df01920faabb0cbde6
child 189949 d1c1aec942281cc2e50c9d1907f2e44d931cdba7
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersmichal
bugs956801
milestone33.0a1
Bug 956801 - save some locking in CacheEntry::BackgroundOp, r=michal
netwerk/cache2/CacheEntry.cpp
--- a/netwerk/cache2/CacheEntry.cpp
+++ b/netwerk/cache2/CacheEntry.cpp
@@ -1485,65 +1485,66 @@ void CacheEntry::BackgroundOp(uint32_t a
   if (!CacheStorageService::IsOnManagementThread() || aForceAsync) {
     if (mBackgroundOperations.Set(aOperations))
       CacheStorageService::Self()->Dispatch(this);
 
     LOG(("CacheEntry::BackgroundOp this=%p dipatch of %x", this, aOperations));
     return;
   }
 
-  mozilla::MutexAutoUnlock unlock(mLock);
+  {
+    mozilla::MutexAutoUnlock unlock(mLock);
 
-  MOZ_ASSERT(CacheStorageService::IsOnManagementThread());
+    MOZ_ASSERT(CacheStorageService::IsOnManagementThread());
 
-  if (aOperations & Ops::FRECENCYUPDATE) {
-    #ifndef M_LN2
-    #define M_LN2 0.69314718055994530942
-    #endif
+    if (aOperations & Ops::FRECENCYUPDATE) {
+      #ifndef M_LN2
+      #define M_LN2 0.69314718055994530942
+      #endif
+
+      // Half-life is dynamic, in seconds.
+      static double half_life = CacheObserver::HalfLifeSeconds();
+      // Must convert from seconds to milliseconds since PR_Now() gives usecs.
+      static double const decay = (M_LN2 / half_life) / static_cast<double>(PR_USEC_PER_SEC);
 
-    // Half-life is dynamic, in seconds.
-    static double half_life = CacheObserver::HalfLifeSeconds();
-    // Must convert from seconds to milliseconds since PR_Now() gives usecs.
-    static double const decay = (M_LN2 / half_life) / static_cast<double>(PR_USEC_PER_SEC);
+      double now_decay = static_cast<double>(PR_Now()) * decay;
 
-    double now_decay = static_cast<double>(PR_Now()) * decay;
+      if (mFrecency == 0) {
+        mFrecency = now_decay;
+      }
+      else {
+        // TODO: when C++11 enabled, use std::log1p(n) which is equal to log(n + 1) but
+        // more precise.
+        mFrecency = log(exp(mFrecency - now_decay) + 1) + now_decay;
+      }
+      LOG(("CacheEntry FRECENCYUPDATE [this=%p, frecency=%1.10f]", this, mFrecency));
 
-    if (mFrecency == 0) {
-      mFrecency = now_decay;
+      // Because CacheFile::Set*() are not thread-safe to use (uses WeakReference that
+      // is not thread-safe) we must post to the main thread...
+      nsRefPtr<nsRunnableMethod<CacheEntry> > event =
+        NS_NewRunnableMethod(this, &CacheEntry::StoreFrecency);
+      NS_DispatchToMainThread(event);
     }
-    else {
-      // TODO: when C++11 enabled, use std::log1p(n) which is equal to log(n + 1) but
-      // more precise.
-      mFrecency = log(exp(mFrecency - now_decay) + 1) + now_decay;
+
+    if (aOperations & Ops::REGISTER) {
+      LOG(("CacheEntry REGISTER [this=%p]", this));
+
+      CacheStorageService::Self()->RegisterEntry(this);
     }
-    LOG(("CacheEntry FRECENCYUPDATE [this=%p, frecency=%1.10f]", this, mFrecency));
 
-    // Because CacheFile::Set*() are not thread-safe to use (uses WeakReference that
-    // is not thread-safe) we must post to the main thread...
-    nsRefPtr<nsRunnableMethod<CacheEntry> > event =
-      NS_NewRunnableMethod(this, &CacheEntry::StoreFrecency);
-    NS_DispatchToMainThread(event);
-  }
+    if (aOperations & Ops::UNREGISTER) {
+      LOG(("CacheEntry UNREGISTER [this=%p]", this));
 
-  if (aOperations & Ops::REGISTER) {
-    LOG(("CacheEntry REGISTER [this=%p]", this));
-
-    CacheStorageService::Self()->RegisterEntry(this);
-  }
-
-  if (aOperations & Ops::UNREGISTER) {
-    LOG(("CacheEntry UNREGISTER [this=%p]", this));
-
-    CacheStorageService::Self()->UnregisterEntry(this);
-  }
+      CacheStorageService::Self()->UnregisterEntry(this);
+    }
+  } // unlock
 
   if (aOperations & Ops::CALLBACKS) {
     LOG(("CacheEntry CALLBACKS (invoke) [this=%p]", this));
 
-    mozilla::MutexAutoLock lock(mLock);
     InvokeCallbacks();
   }
 }
 
 void CacheEntry::StoreFrecency()
 {
   // No need for thread safety over mFrecency, it will be rewriten
   // correctly on following invocation if broken by concurrency.