Bug 624549, Don't call GC so aggressively in nsJSContext::CC, r=gal, a=jst
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Sat, 29 Jan 2011 22:55:39 +0200
changeset 61596 3ea2b5a7c9c8eaa346875fe644c444df5f388d38
parent 61595 ba84db82eed5945f6f1cf0ebef383d3e905c8ae7
child 61597 0ab93abd23cb40f5918e16b0e1b5b7706fd18a2e
push id18404
push useropettay@mozilla.com
push dateSat, 29 Jan 2011 21:10:51 +0000
treeherdermozilla-central@3ea2b5a7c9c8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgal, jst
bugs624549
milestone2.0b11pre
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 624549, Don't call GC so aggressively in nsJSContext::CC, r=gal, a=jst
dom/base/nsDOMWindowUtils.cpp
dom/base/nsJSEnvironment.cpp
dom/base/nsJSEnvironment.h
toolkit/components/places/tests/chrome/Makefile.in
--- a/dom/base/nsDOMWindowUtils.cpp
+++ b/dom/base/nsDOMWindowUtils.cpp
@@ -650,17 +650,17 @@ nsDOMWindowUtils::GarbageCollect(nsICycl
 {
   // Always permit this in debug builds.
 #ifndef DEBUG
   if (!IsUniversalXPConnectCapable()) {
     return NS_ERROR_DOM_SECURITY_ERR;
   }
 #endif
 
-  nsJSContext::CC(aListener);
+  nsJSContext::CC(aListener, PR_TRUE);
 
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsDOMWindowUtils::ProcessUpdates()
 {
--- a/dom/base/nsJSEnvironment.cpp
+++ b/dom/base/nsJSEnvironment.cpp
@@ -156,24 +156,26 @@ static PRLogModuleInfo* gJSDiagnostics;
 // is multiplied with this number.
 #define NS_PROBABILITY_MULTIPLIER   3
 // Cycle collector is never called more often than every NS_MIN_CC_INTERVAL
 // milliseconds. Exceptions are low memory situation and memory pressure
 // notification.
 #define NS_MIN_CC_INTERVAL          10000 // ms
 // If previous cycle collection collected more than this number of objects,
 // the next collection will happen somewhat soon.
+// Also, if there are more than this number suspected objects, GC will be called
+// right before CC, if it wasn't called after last CC.
 #define NS_COLLECTED_OBJECTS_LIMIT  5000
 // CC will be called if GC has been called at least this number of times and
 // there are at least NS_MIN_SUSPECT_CHANGES new suspected objects.
 #define NS_MAX_GC_COUNT             5
-#define NS_MIN_SUSPECT_CHANGES      10
+#define NS_MIN_SUSPECT_CHANGES      100
 // CC will be called if there are at least NS_MAX_SUSPECT_CHANGES new suspected
 // objects.
-#define NS_MAX_SUSPECT_CHANGES      100
+#define NS_MAX_SUSPECT_CHANGES      1000
 
 // if you add statics here, add them to the list in nsJSRuntime::Startup
 
 static PRUint32 sDelayedCCollectCount;
 static PRUint32 sCCollectCount;
 static PRBool sUserIsActive;
 static PRTime sPreviousCCTime;
 static PRUint32 sCollectedObjectsCounts;
@@ -254,17 +256,17 @@ nsUserActivityObserver::Observe(nsISuppo
   ++mUserActivityCounter;
   if (!strcmp(aTopic, "user-interaction-inactive")) {
 #ifdef DEBUG_smaug
     printf("user-interaction-inactive\n");
 #endif
     if (sUserIsActive) {
       sUserIsActive = PR_FALSE;
       if (!sGCTimer) {
-        nsJSContext::IntervalCC();
+        nsJSContext::MaybeCC(PR_FALSE);
         return NS_OK;
       }
     }
     higherProbability = (mUserActivityCounter > NS_CC_SOFT_LIMIT_INACTIVE);
   } else if (!strcmp(aTopic, "user-interaction-active")) {
 #ifdef DEBUG_smaug
     printf("user-interaction-active\n");
 #endif
@@ -294,17 +296,17 @@ public:
 };
 
 NS_IMPL_ISUPPORTS1(nsCCMemoryPressureObserver, nsIObserver)
 
 NS_IMETHODIMP
 nsCCMemoryPressureObserver::Observe(nsISupports* aSubject, const char* aTopic,
                                     const PRUnichar* aData)
 {
-  nsJSContext::CC(nsnull);
+  nsJSContext::CC(nsnull, PR_TRUE);
   return NS_OK;
 }
 
 /****************************************************************
  ************************** AutoFree ****************************
  ****************************************************************/
 
 class AutoFree {
@@ -3361,62 +3363,65 @@ nsJSContext::SetGCOnDestruction(PRBool a
 NS_IMETHODIMP
 nsJSContext::ScriptExecuted()
 {
   ScriptEvaluated(!::JS_IsRunning(mContext));
 
   return NS_OK;
 }
 
+static inline uint32
+GetGCRunsSinceLastCC()
+{
+    // To avoid crash if nsJSRuntime is not properly initialized.
+    // See the bug 474586
+    if (!nsJSRuntime::sRuntime)
+        return 0;
+
+    // Since JS_GetGCParameter() and sSavedGCCount are unsigned, the following
+    // gives the correct result even when the GC counter wraps around
+    // UINT32_MAX since the last call to JS_GetGCParameter(). 
+    return JS_GetGCParameter(nsJSRuntime::sRuntime, JSGC_NUMBER) -
+           sSavedGCCount;
+}
+
 //static
 void
-nsJSContext::CC(nsICycleCollectorListener *aListener)
+nsJSContext::CC(nsICycleCollectorListener *aListener, PRBool aForceGC)
 {
   NS_TIME_FUNCTION_MIN(1.0);
 
   ++sCCollectCount;
 #ifdef DEBUG_smaug
   printf("Will run cycle collector (%i), %lldms since previous.\n",
          sCCollectCount, (PR_Now() - sPreviousCCTime) / PR_USEC_PER_MSEC);
 #endif
   sPreviousCCTime = PR_Now();
   sDelayedCCollectCount = 0;
   sCCSuspectChanges = 0;
   // nsCycleCollector_collect() no longer forces a JS garbage collection,
   // so we have to do it ourselves here.
-  if (nsContentUtils::XPConnect()) {
+  if (nsContentUtils::XPConnect() &&
+      (aForceGC ||
+       (!GetGCRunsSinceLastCC() &&
+        sCCSuspectedCount > NS_COLLECTED_OBJECTS_LIMIT))) {
     nsContentUtils::XPConnect()->GarbageCollect();
   }
   sCollectedObjectsCounts = nsCycleCollector_collect(aListener);
   sCCSuspectedCount = nsCycleCollector_suspectedCount();
   if (nsJSRuntime::sRuntime) {
     sSavedGCCount = JS_GetGCParameter(nsJSRuntime::sRuntime, JSGC_NUMBER);
   }
 #ifdef DEBUG_smaug
   printf("Collected %u objects, %u suspected objects, took %lldms\n",
          sCollectedObjectsCounts, sCCSuspectedCount,
          (PR_Now() - sPreviousCCTime) / PR_USEC_PER_MSEC);
 #endif
 }
 
-static inline uint32
-GetGCRunsSinceLastCC()
-{
-    // To avoid crash if nsJSRuntime is not properly initialized.
-    // See the bug 474586
-    if (!nsJSRuntime::sRuntime)
-        return 0;
-
-    // Since JS_GetGCParameter() and sSavedGCCount are unsigned, the following
-    // gives the correct result even when the GC counter wraps around
-    // UINT32_MAX since the last call to JS_GetGCParameter(). 
-    return JS_GetGCParameter(nsJSRuntime::sRuntime, JSGC_NUMBER) -
-           sSavedGCCount;
-}
-
 //static
 PRBool
 nsJSContext::MaybeCC(PRBool aHigherProbability)
 {
   ++sDelayedCCollectCount;
 
   // Don't check suspected count if CC will be called anyway.
   if (sCCSuspectChanges <= NS_MIN_SUSPECT_CHANGES ||
@@ -3464,36 +3469,36 @@ nsJSContext::MaybeCC(PRBool aHigherProba
 
 //static
 void
 nsJSContext::CCIfUserInactive()
 {
   if (sUserIsActive) {
     MaybeCC(PR_TRUE);
   } else {
-    IntervalCC();
+    IntervalCC(PR_TRUE);
   }
 }
 
 //static
 void
 nsJSContext::MaybeCCIfUserInactive()
 {
   if (!sUserIsActive) {
     MaybeCC(PR_FALSE);
   }
 }
 
 //static
 PRBool
-nsJSContext::IntervalCC()
+nsJSContext::IntervalCC(PRBool aForceGC)
 {
   if ((PR_Now() - sPreviousCCTime) >=
       PRTime(NS_MIN_CC_INTERVAL * PR_USEC_PER_MSEC)) {
-    nsJSContext::CC(nsnull);
+    nsJSContext::CC(nsnull, aForceGC);
     return PR_TRUE;
   }
 #ifdef DEBUG_smaug
   printf("Running CC was delayed because of NS_MIN_CC_INTERVAL.\n");
 #endif
   return PR_FALSE;
 }
 
--- a/dom/base/nsJSEnvironment.h
+++ b/dom/base/nsJSEnvironment.h
@@ -184,17 +184,18 @@ public:
 
   NS_DECL_NSIXPCSCRIPTNOTIFY
 
   static void LoadStart();
   static void LoadEnd();
 
   // CC does always call cycle collector and it also updates the counters
   // that MaybeCC uses.
-  static void CC(nsICycleCollectorListener *aListener);
+  static void CC(nsICycleCollectorListener *aListener,
+                 PRBool aForceGC = PR_FALSE);
 
   // MaybeCC calls cycle collector if certain conditions are fulfilled.
   // The conditions are:
   // - The timer related to page load (sGCTimer) must not be active.
   // - At least NS_MIN_CC_INTERVAL milliseconds must have elapsed since the
   //   previous cycle collector call.
   // - Certain number of MaybeCC calls have occurred.
   //   The number of needed MaybeCC calls depends on the aHigherProbability
@@ -203,19 +204,20 @@ public:
   //   at least NS_MAX_DELAYED_CCOLLECT MaybeCC calls are needed.
   //   If the previous call to cycle collector did collect something,
   //   MaybeCC works effectively as if aHigherProbability was true.
   // @return PR_TRUE if cycle collector was called.
   static PRBool MaybeCC(PRBool aHigherProbability);
 
   // IntervalCC() calls CC() if at least NS_MIN_CC_INTERVAL milliseconds have
   // elapsed since the previous cycle collector call.
-  static PRBool IntervalCC();
+  static PRBool IntervalCC(PRBool aForceGC = PR_FALSE);
 
-  // Calls IntervalCC() if user is currently inactive, otherwise MaybeCC(PR_TRUE)
+  // Calls IntervalCC(PR_TRUE) if user is currently inactive,
+  // otherwise MaybeCC(PR_TRUE)
   static void CCIfUserInactive();
 
   static void MaybeCCIfUserInactive();
 
   static void FireGCTimer(PRBool aLoadInProgress);
 
 protected:
   nsresult InitializeExternalClasses();
--- a/toolkit/components/places/tests/chrome/Makefile.in
+++ b/toolkit/components/places/tests/chrome/Makefile.in
@@ -51,18 +51,19 @@ include $(topsrcdir)/config/rules.mk
 		rss_as_html.rss^headers^ \
 		$(NULL)
 
 _CHROME_FILES	= \
 		test_329534.xul \
 		test_371798.xul \
 		test_342484.xul \
 		test_341972a.xul \
-		test_381357.xul \
 		test_favicon_annotations.xul \
 		$(NULL)
 
+# test_381357.xul is disabled for now because of bug 561007. See also bug 624549. 
+
 libs:: $(_HTTP_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
 
 libs:: $(_CHROME_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/chrome/$(relativesrcdir)