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 idunknown
push userunknown
push dateunknown
reviewersgal, jst
bugs624549
milestone2.0b11pre
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)