Bug 1162751 - Part 3: Wrap expensive calls in PR_LOG_TEST. r=seth
authorEric Rahm <erahm@mozilla.com>
Mon, 11 May 2015 13:42:30 -0700
changeset 243399 f9af70b6e6de1ce8e1c823fbdad417c9e084ab79
parent 243398 37f361e463b3ea5246eb313cb0a4c065a2a05937
child 243400 520a0ded5a2fcec9820f5b1de89ee12f8b7c1e91
push id28738
push usercbook@mozilla.com
push dateTue, 12 May 2015 14:11:31 +0000
treeherdermozilla-central@bedce1b405a3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth
bugs1162751
milestone40.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 1162751 - Part 3: Wrap expensive calls in PR_LOG_TEST. r=seth Check that logging is enabled before performing potentially expensive operations.
image/src/ProgressTracker.cpp
image/src/imgLoader.cpp
image/src/imgRequest.cpp
image/src/imgRequestProxy.cpp
--- a/image/src/ProgressTracker.cpp
+++ b/image/src/ProgressTracker.cpp
@@ -159,26 +159,28 @@ class AsyncNotifyRunnable : public nsRun
     nsTArray<nsRefPtr<IProgressObserver>> mObservers;
 };
 
 void
 ProgressTracker::Notify(IProgressObserver* aObserver)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  nsRefPtr<Image> image = GetImage();
-  if (image && image->GetURI()) {
-    nsRefPtr<ImageURL> uri(image->GetURI());
-    nsAutoCString spec;
-    uri->GetSpec(spec);
-    LOG_FUNC_WITH_PARAM(GetImgLog(),
-                        "ProgressTracker::Notify async", "uri", spec.get());
-  } else {
-    LOG_FUNC_WITH_PARAM(GetImgLog(),
-                        "ProgressTracker::Notify async", "uri", "<unknown>");
+  if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+    nsRefPtr<Image> image = GetImage();
+    if (image && image->GetURI()) {
+      nsRefPtr<ImageURL> uri(image->GetURI());
+      nsAutoCString spec;
+      uri->GetSpec(spec);
+      LOG_FUNC_WITH_PARAM(GetImgLog(),
+                          "ProgressTracker::Notify async", "uri", spec.get());
+    } else {
+      LOG_FUNC_WITH_PARAM(GetImgLog(),
+                          "ProgressTracker::Notify async", "uri", "<unknown>");
+    }
   }
 
   aObserver->SetNotificationsDeferred(true);
 
   // If we have an existing runnable that we can use, we just append this
   // observer to its list of observers to be notified. This ensures we don't
   // unnecessarily delay onload.
   AsyncNotifyRunnable* runnable =
@@ -226,23 +228,25 @@ class AsyncNotifyCurrentStateRunnable : 
     nsRefPtr<Image> mImage;
 };
 
 void
 ProgressTracker::NotifyCurrentState(IProgressObserver* aObserver)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  nsRefPtr<Image> image = GetImage();
-  nsAutoCString spec;
-  if (image && image->GetURI()) {
-    image->GetURI()->GetSpec(spec);
+  if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+    nsRefPtr<Image> image = GetImage();
+    nsAutoCString spec;
+    if (image && image->GetURI()) {
+      image->GetURI()->GetSpec(spec);
+    }
+    LOG_FUNC_WITH_PARAM(GetImgLog(),
+                        "ProgressTracker::NotifyCurrentState", "uri", spec.get());
   }
-  LOG_FUNC_WITH_PARAM(GetImgLog(),
-                      "ProgressTracker::NotifyCurrentState", "uri", spec.get());
 
   aObserver->SetNotificationsDeferred(true);
 
   nsCOMPtr<nsIRunnable> ev = new AsyncNotifyCurrentStateRunnable(this,
                                                                  aObserver);
   NS_DispatchToCurrentThread(ev);
 }
 
--- a/image/src/imgLoader.cpp
+++ b/image/src/imgLoader.cpp
@@ -888,28 +888,30 @@ imgCacheEntry::UpdateCache(int32_t diff 
   if (!Evicted() && HasNoProxies()) {
     mLoader->CacheEntriesChanged(mRequest->IsChrome(), diff);
   }
 }
 
 void
 imgCacheEntry::SetHasNoProxies(bool hasNoProxies)
 {
-  nsRefPtr<ImageURL> uri;
-  mRequest->GetURI(getter_AddRefs(uri));
-  nsAutoCString spec;
-  if (uri) {
-    uri->GetSpec(spec);
-  }
-  if (hasNoProxies) {
-    LOG_FUNC_WITH_PARAM(GetImgLog(), "imgCacheEntry::SetHasNoProxies true",
-                        "uri", spec.get());
-  } else {
-    LOG_FUNC_WITH_PARAM(GetImgLog(), "imgCacheEntry::SetHasNoProxies false",
-                        "uri", spec.get());
+  if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+    nsRefPtr<ImageURL> uri;
+    mRequest->GetURI(getter_AddRefs(uri));
+    nsAutoCString spec;
+    if (uri) {
+      uri->GetSpec(spec);
+    }
+    if (hasNoProxies) {
+      LOG_FUNC_WITH_PARAM(GetImgLog(), "imgCacheEntry::SetHasNoProxies true",
+                          "uri", spec.get());
+    } else {
+      LOG_FUNC_WITH_PARAM(GetImgLog(), "imgCacheEntry::SetHasNoProxies false",
+                          "uri", spec.get());
+    }
   }
 
   mHasNoProxies = hasNoProxies;
 }
 
 imgCacheQueue::imgCacheQueue()
  : mDirty(false),
    mSize(0)
@@ -1070,25 +1072,27 @@ imgCacheExpirationTracker::imgCacheExpir
 
 void
 imgCacheExpirationTracker::NotifyExpired(imgCacheEntry* entry)
 {
   // Hold on to a reference to this entry, because the expiration tracker
   // mechanism doesn't.
   nsRefPtr<imgCacheEntry> kungFuDeathGrip(entry);
 
-  nsRefPtr<imgRequest> req(entry->GetRequest());
-  if (req) {
-    nsRefPtr<ImageURL> uri;
-    req->GetURI(getter_AddRefs(uri));
-    nsAutoCString spec;
-    uri->GetSpec(spec);
-    LOG_FUNC_WITH_PARAM(GetImgLog(),
-                       "imgCacheExpirationTracker::NotifyExpired",
-                       "entry", spec.get());
+  if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+    nsRefPtr<imgRequest> req(entry->GetRequest());
+    if (req) {
+      nsRefPtr<ImageURL> uri;
+      req->GetURI(getter_AddRefs(uri));
+      nsAutoCString spec;
+      uri->GetSpec(spec);
+      LOG_FUNC_WITH_PARAM(GetImgLog(),
+                         "imgCacheExpirationTracker::NotifyExpired",
+                         "entry", spec.get());
+    }
   }
 
   // We can be called multiple times on the same entry. Don't do work multiple
   // times.
   if (!entry->Evicted()) {
     entry->Loader()->RemoveFromCache(entry);
   }
 
@@ -1504,23 +1508,25 @@ imgLoader::PutIntoCache(const ImageCache
   RemoveFromUncachedImages(request);
 
   return true;
 }
 
 bool
 imgLoader::SetHasNoProxies(imgRequest* aRequest, imgCacheEntry* aEntry)
 {
-  nsRefPtr<ImageURL> uri;
-  aRequest->GetURI(getter_AddRefs(uri));
-  nsAutoCString spec;
-  uri->GetSpec(spec);
-
-  LOG_STATIC_FUNC_WITH_PARAM(GetImgLog(),
-                             "imgLoader::SetHasNoProxies", "uri", spec.get());
+  if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+    nsRefPtr<ImageURL> uri;
+    aRequest->GetURI(getter_AddRefs(uri));
+    nsAutoCString spec;
+    uri->GetSpec(spec);
+
+    LOG_STATIC_FUNC_WITH_PARAM(GetImgLog(),
+                               "imgLoader::SetHasNoProxies", "uri", spec.get());
+  }
 
   aEntry->SetHasNoProxies(true);
 
   if (aEntry->Evicted()) {
     return false;
   }
 
   imgCacheQueue& queue = GetCacheQueue(aRequest->IsChrome());
@@ -1594,25 +1600,27 @@ imgLoader::CheckCacheLimits(imgCacheTabl
 
   // Remove entries from the cache until we're back at our desired max size.
   while (queue.GetSize() > sCacheMaxSize) {
     // Remove the first entry in the queue.
     nsRefPtr<imgCacheEntry> entry(queue.Pop());
 
     NS_ASSERTION(entry, "imgLoader::CheckCacheLimits -- NULL entry pointer");
 
-    nsRefPtr<imgRequest> req(entry->GetRequest());
-    if (req) {
-      nsRefPtr<ImageURL> uri;
-      req->GetURI(getter_AddRefs(uri));
-      nsAutoCString spec;
-      uri->GetSpec(spec);
-      LOG_STATIC_FUNC_WITH_PARAM(GetImgLog(),
-                                 "imgLoader::CheckCacheLimits",
-                                 "entry", spec.get());
+    if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+      nsRefPtr<imgRequest> req(entry->GetRequest());
+      if (req) {
+        nsRefPtr<ImageURL> uri;
+        req->GetURI(getter_AddRefs(uri));
+        nsAutoCString spec;
+        uri->GetSpec(spec);
+        LOG_STATIC_FUNC_WITH_PARAM(GetImgLog(),
+                                   "imgLoader::CheckCacheLimits",
+                                   "entry", spec.get());
+      }
     }
 
     if (entry) {
       RemoveFromCache(entry);
     }
   }
 }
 
@@ -1832,17 +1840,17 @@ imgLoader::ValidateEntry(imgCacheEntry* 
     }
 
     // Determine whether the cache aEntry must be revalidated...
     validateRequest = ShouldRevalidateEntry(aEntry, aLoadFlags, hasExpired);
 
     PR_LOG(GetImgLog(), PR_LOG_DEBUG,
            ("imgLoader::ValidateEntry validating cache entry. "
             "validateRequest = %d", validateRequest));
-  } else if (!key) {
+  } else if (!key && PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
     nsAutoCString spec;
     aURI->GetSpec(spec);
 
     PR_LOG(GetImgLog(), PR_LOG_DEBUG,
            ("imgLoader::ValidateEntry BYPASSING cache validation for %s "
             "because of NULL LoadID", spec.get()));
   }
 
@@ -2859,21 +2867,23 @@ imgCacheValidator::OnStartRequest(nsIReq
   // data that's coming in off the channel.
   nsCOMPtr<nsIURI> uri;
   {
     nsRefPtr<ImageURL> imageURL;
     mRequest->GetURI(getter_AddRefs(imageURL));
     uri = imageURL->ToIURI();
   }
 
-  nsAutoCString spec;
-  uri->GetSpec(spec);
-  LOG_MSG_WITH_PARAM(GetImgLog(),
-                     "imgCacheValidator::OnStartRequest creating new request",
-                     "uri", spec.get());
+  if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+    nsAutoCString spec;
+    uri->GetSpec(spec);
+    LOG_MSG_WITH_PARAM(GetImgLog(),
+                       "imgCacheValidator::OnStartRequest creating new request",
+                       "uri", spec.get());
+  }
 
   int32_t corsmode = mRequest->GetCORSMode();
   ReferrerPolicy refpol = mRequest->GetReferrerPolicy();
   nsCOMPtr<nsIPrincipal> loadingPrincipal = mRequest->GetLoadingPrincipal();
 
   // Doom the old request's cache entry
   mRequest->RemoveFromCache();
 
--- a/image/src/imgRequest.cpp
+++ b/image/src/imgRequest.cpp
@@ -253,18 +253,17 @@ imgRequest::RemoveProxy(imgRequestProxy*
     // been cancelled and thus removed from the cache, tell the image loader so
     // we can be evicted from the cache.
     if (mCacheEntry) {
       MOZ_ASSERT(mURI, "Removing last observer without key uri.");
 
       if (mLoader) {
         mLoader->SetHasNoProxies(this, mCacheEntry);
       }
-    }
-    else {
+    } else if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
       nsAutoCString spec;
       mURI->GetSpec(spec);
       LOG_MSG_WITH_PARAM(GetImgLog(),
                          "imgRequest::RemoveProxy no cache entry",
                          "uri", spec.get());
     }
 
     /* If |aStatus| is a failure code, then cancel the load if it is still in
--- a/image/src/imgRequestProxy.cpp
+++ b/image/src/imgRequestProxy.cpp
@@ -856,20 +856,23 @@ imgRequestProxy::Notify(int32_t aType, c
   nsCOMPtr<imgINotificationObserver> listener(mListener);
 
   mListener->Notify(this, aType, aRect);
 }
 
 void
 imgRequestProxy::OnLoadComplete(bool aLastPart)
 {
-  nsAutoCString name;
-  GetName(name);
-  LOG_FUNC_WITH_PARAM(GetImgLog(), "imgRequestProxy::OnLoadComplete",
-                      "name", name.get());
+  if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+    nsAutoCString name;
+    GetName(name);
+    LOG_FUNC_WITH_PARAM(GetImgLog(), "imgRequestProxy::OnLoadComplete",
+                        "name", name.get());
+  }
+
   // There's all sorts of stuff here that could kill us (the OnStopRequest call
   // on the listener, the removal from the loadgroup, the release of the
   // listener, etc).  Don't let them do it.
   nsCOMPtr<imgIRequest> kungFuDeathGrip(this);
 
   if (mListener && !mCanceled) {
     // Hold a ref to the listener while we call it, just in case.
     nsCOMPtr<imgINotificationObserver> kungFuDeathGrip(mListener);
@@ -899,34 +902,38 @@ imgRequestProxy::OnLoadComplete(bool aLa
     mListenerIsStrongRef = false;
     NS_RELEASE(obs);
   }
 }
 
 void
 imgRequestProxy::BlockOnload()
 {
-  nsAutoCString name;
-  GetName(name);
-  LOG_FUNC_WITH_PARAM(GetImgLog(), "imgRequestProxy::BlockOnload",
-                      "name", name.get());
+  if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+    nsAutoCString name;
+    GetName(name);
+    LOG_FUNC_WITH_PARAM(GetImgLog(), "imgRequestProxy::BlockOnload",
+                        "name", name.get());
+  }
 
   nsCOMPtr<imgIOnloadBlocker> blocker = do_QueryInterface(mListener);
   if (blocker) {
     blocker->BlockOnload(this);
   }
 }
 
 void
 imgRequestProxy::UnblockOnload()
 {
-  nsAutoCString name;
-  GetName(name);
-  LOG_FUNC_WITH_PARAM(GetImgLog(), "imgRequestProxy::UnblockOnload",
-                      "name", name.get());
+  if (PR_LOG_TEST(GetImgLog(), PR_LOG_DEBUG)) {
+    nsAutoCString name;
+    GetName(name);
+    LOG_FUNC_WITH_PARAM(GetImgLog(), "imgRequestProxy::UnblockOnload",
+                        "name", name.get());
+  }
 
   nsCOMPtr<imgIOnloadBlocker> blocker = do_QueryInterface(mListener);
   if (blocker) {
     blocker->UnblockOnload(this);
   }
 }
 
 void